Martin Peřina has posted comments on this change. Change subject: core : Support tokens with special characters ......................................................................
Patch Set 1: (6 comments) https://gerrit.ovirt.org/#/c/41832/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SearchQuery.java: Line 63: Line 64: public SearchQuery(P parameters) { Line 65: super(parameters); Line 66: } Line 67: What about using regex to indetify AD* query: private static final AD_SEARCH_TYPES = { SearchObjects.AD_USER_OBJ_NAME, SearchObjects.AD_USER_PLU_OBJ_NAME, SearchObjects.AD_GROUP_OBJ_NAME, SearchObjects.AD_GROUP_PLU_OBJ_NAME }; private static final Pattern adSearchPattern = Pattern.compile( String.format( "^((%s)@)(?<content>.*)", StringUtils.join(AD_SEARCH_TYPES, "|"))); Line 68: @Override Line 69: protected void executeQueryCommand() { Line 70: List<? extends IVdcQueryable> returnValue = new ArrayList<>(); Line 71: switch (getParameters().getSearchTypeValue()) { Line 372: searchText = searchText.replace(SearchObjects.AD_USER_PLU_OBJ_NAME + AT, StringUtils.EMPTY); Line 373: searchText = searchText.replace(SearchObjects.AD_USER_OBJ_NAME + AT, StringUtils.EMPTY); Line 374: searchText = searchText.replace(SearchObjects.AD_GROUP_PLU_OBJ_NAME + AT, StringUtils.EMPTY); Line 375: searchText = searchText.replace(SearchObjects.AD_GROUP_OBJ_NAME + AT, StringUtils.EMPTY); Line 376: Instead of lines 366-375 just use these: Matcher m = adSearchPattern.matcher(searchText); if (m.matches) { searchText = m.group("content"); Line 377: // get profile Line 378: List<String> profiles = getBackend().runInternalQuery(VdcQueryType.GetDomainList, Line 379: new VdcQueryParametersBase()).getReturnValue(); Line 380: for (String profile : profiles) { Line 382: queryAuthz = profile; Line 383: searchText = searchText.replace(profile + COLON, StringUtils.EMPTY); Line 384: break; Line 385: } Line 386: } I think we could use pattern matching also for profiles and namespaces, but these patterns should be compiled on the 1st use of SearchQuery as they are not initialized during static class initialization. Maybe even single pattern with 3 groups (PROFILE, NAMESPACE, QUERY) could be enough to handle this parsing. Line 387: // get namespace Line 388: HashMap<String, List<String>> namespacesMap = Line 389: getBackend().runInternalQuery(VdcQueryType.GetAvailableNamespaces, Line 390: new VdcQueryParametersBase()).getReturnValue(); Line 398: } Line 399: searchText = prefix + searchText; Line 400: curSyntaxChecker = SyntaxCheckerFactory.createADSyntaxChecker(Config Line 401: .<String>getValue(ConfigValues.AuthenticationMethod)); Line 402: Why adding empty line? Line 403: } else { Line 404: curSyntaxChecker = SyntaxCheckerFactory Line 405: .createBackendSyntaxChecker(Config.<String>getValue(ConfigValues.AuthenticationMethod)); Line 406: } Line 403: } else { Line 404: curSyntaxChecker = SyntaxCheckerFactory Line 405: .createBackendSyntaxChecker(Config.<String>getValue(ConfigValues.AuthenticationMethod)); Line 406: } Line 407: Why adding empty line? Line 408: SyntaxContainer searchObj = curSyntaxChecker.analyzeSyntaxState(searchText, true); Line 409: // set the case-sensitive flag Line 410: searchObj.setCaseSensitive(getParameters().getCaseSensitive()); Line 411: // If a number > maxValue is given then maxValue will be used https://gerrit.ovirt.org/#/c/41832/1/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java File backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java: Line 191: Regex > any reason not to allow additional characters? I think that if we allow for example '&' which is logical and when searching on LDAP, then cross site injection could be possible. Although I'm not sure if we don't already have a hole there as we allow brackets, so following query is possible: NAME = "*)(objectClass=*)" But I need to verify this. -- To view, visit https://gerrit.ovirt.org/41832 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2464227376eb2e6ee0b5ada2ced21278675b7572 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
