diff options
2 files changed, 42 insertions, 2 deletions
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java index 1dedbd857c3..49b27eba613 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java @@ -21,10 +21,13 @@ import com.yahoo.document.restapi.OperationHandlerImpl; import com.yahoo.document.restapi.Response; import com.yahoo.document.restapi.RestApiException; import com.yahoo.document.restapi.RestUri; +import com.yahoo.document.select.DocumentSelector; +import com.yahoo.document.select.parser.ParseException; import com.yahoo.documentapi.messagebus.MessageBusDocumentAccess; import com.yahoo.documentapi.messagebus.MessageBusParams; import com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet; import com.yahoo.metrics.simple.MetricReceiver; +import com.yahoo.text.Text; import com.yahoo.vespa.config.content.LoadTypeConfig; import com.yahoo.vespaxmlparser.VespaXMLFeedReader; @@ -312,14 +315,31 @@ public class RestApi extends LoggingRequestHandler { if (group.name == 'n') { return String.format("id.user==%d", parseAndValidateVisitNumericId(group.value)); } else { - // TODO first pass through Text.validateTextString? Cannot create doc IDs that don't match that anyway... + // Cannot feed documents with groups that don't pass this test, so it makes sense + // to enforce this symmetry when trying to retrieve them as well. + Text.validateTextString(group.value).ifPresent(codepoint -> { + throw new BadRequestParameterException(SELECTION, String.format( + "Failed to parse group part of selection URI; contains invalid text code point U%04X", codepoint)); + }); return String.format("id.group=='%s'", singleQuoteEscapedString(group.value)); } } + private static void validateDocumentSelectionSyntax(String expression) { + try { + new DocumentSelector(expression); + } catch (ParseException e) { + throw new BadRequestParameterException(SELECTION, String.format("Failed to parse expression given in 'selection'" + + " parameter. Must be a complete and valid sub-expression. Error: %s", e.getMessage())); + } + } + private static String documentSelectionFromRequest(RestUri restUri, HttpRequest request) throws BadRequestParameterException { - // TODO try to preemptively parse sub expression to ensure it is complete String documentSelection = Optional.ofNullable(request.getProperty(SELECTION)).orElse(""); + if (!documentSelection.isEmpty()) { + // Ensure that the selection parameter sub-expression is complete and valid by itself. + validateDocumentSelectionSyntax(documentSelection); + } if (restUri.getGroup().isPresent() && ! restUri.getGroup().get().value.isEmpty()) { String locationSubExpression = validateAndBuildLocationSubExpression(restUri.getGroup().get()); if (documentSelection.isEmpty()) { diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/RestApiTest.java b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/RestApiTest.java index ed7935b698b..14d8239ba41 100644 --- a/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/RestApiTest.java +++ b/vespaclient-container-plugin/src/test/java/com/yahoo/document/restapi/resource/RestApiTest.java @@ -340,6 +340,12 @@ public class RestApiTest { } @Test + public void non_text_group_string_character_returns_error() { + String output = performV1RestCall(String.format("group/%s", encoded("\u001f"))); + assertThat(output, containsString("Failed to parse group part of selection URI; contains invalid text code point U001F")); + } + + @Test public void can_specify_numeric_id_without_explicit_selection() { assertResultingDocumentSelection("number/1234", "id.user==1234"); } @@ -361,6 +367,20 @@ public class RestApiTest { "id.group=='bar' and (3 != 4)"); } + private void assertDocumentSelectionFailsParsing(String expression) { + String output = performV1RestCall(String.format("number/1234?selection=%s", encoded(expression))); + assertThat(output, containsString("Failed to parse expression given in 'selection' parameter. Must be a complete and valid sub-expression.")); + } + + // Make sure that typoing the selection parameter doesn't corrupt the entire selection expression + @Test + public void explicit_selection_sub_expression_is_validated_for_completeness() { + assertDocumentSelectionFailsParsing("1 +"); + assertDocumentSelectionFailsParsing(") or true"); + assertDocumentSelectionFailsParsing("((1 + 2)"); + assertDocumentSelectionFailsParsing("true) or (true"); + } + @Test public void wanted_document_count_returned_parameter_is_propagated() throws IOException { Request request = new Request(String.format("http://localhost:%s/document/v1/namespace/document-type/docid/?wantedDocumentCount=321", getFirstListenPort())); |