Description of problem: *Every* SearchQuery is improperly evaluated as unsafe expression. Version-Release number of selected component (if applicable): 3.5 How reproducible: 100% Steps to Reproduce: 1. call GET on http://localhost:8080/api/vms or any other such 'getter' method and follow through mentioned methods. Additional info: in method org.ovirt.engine.core.bll.SearchQuery#initQueryData is this line: isSafe = SearchObjects.isSafeExpression(searchText); this line is currently always false, because internally it consists of searching through Map which is filled with <some_uppercase_constant> transformed to lowercase and then concatenated with colon. Example "vms:", "macpool:". The searching code looks like this: <map>.contains(<input>.toLowerCase()); but that input is calculated here: org.ovirt.engine.api.restapi.resource.AbstractBackendCollectionResource#getBackendCollection(org.ovirt.engine.core.common.interfaces.SearchType) using code: QueryHelper.getConstraint(getUriInfo(), "", modelType) which instead of previously mentioned examples produces "vms : ", "macpool : ". Reason is in incompatibilities among: org.ovirt.engine.api.common.util.QueryHelper#RETURN_TYPE_SEPARTOR and org/ovirt/engine/core/searchbackend/SearchObjects.java:63 //another definition of separator. Comment: trying to solve sql injection is very improper, related code is extremely inferior and all of this should be thrown away as a whole.
in http://gerrit.ovirt.org/#/c/28343/ I've selected " : " to be the right suffix — it seemed to me, that this value is more important for the system, since ':' was only used in determining 'query safeness'. So I've changed ':' to " : ". If it should be the other case, please let me know.
Is there any way how to test it? Increasing log level or something like that? I'm looking for a way how to confirm the change is in.
I did a quick scan through code and I do not think this is testable in a black-box way. Internals seems too complex to me, so we cannot prove it's ok just upon changing logging level or some other externally observable change like this. Which is, on top of that, not present in code. I.e. I did not find such 'property', but even if it exist somewhere in code, based on it's complexity I do not think it's safe to pronounce it "ok" based just on such observation. Bug was actually caused by two duplicate constants which weren't in sync, so fix was rather easy.
rhev 3.5.0 was released. closing.