diff options
Diffstat (limited to 'vespaclient-container-plugin')
3 files changed, 94 insertions, 16 deletions
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java index cb4d1b9feed..c0ce3e84232 100644 --- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java +++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/OperationHandlerImpl.java @@ -355,12 +355,6 @@ public class OperationHandlerImpl implements OperationHandler { clusterListToString(clusters) + " not " + wantedCluster.get() + ". Please select a valid vespa cluster.", RestUri.apiErrorCodes.MISSING_CLUSTER)); } - // Based on resolveClusterRoute in VdsVisit, protected for testability - // TODO remove in favor of resolveClusterDef - protected static String resolveClusterRoute(Optional<String> wantedCluster, List<ClusterDef> clusters) throws RestApiException { - return clusterDefToRoute(resolveClusterDef(wantedCluster, clusters)); - } - protected static String clusterDefToRoute(ClusterDef clusterDef) { return "[Storage:cluster=" + clusterDef.getName() + ";clusterconfigid=" + clusterDef.getConfigId() + "]"; } 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 bb9387dc6e0..8639d4fad87 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 @@ -278,27 +278,60 @@ public class RestApi extends LoggingRequestHandler { return optionsBuilder.build(); } - - private HttpResponse handleVisit(RestUri restUri, HttpRequest request) throws RestApiException { + + /** + * Escapes all single quotes in input string. + * @param original non-escaped string that may contain single quotes + * @return original if no quotes to escaped were found, otherwise a quote-escaped string + */ + private static String singleQuoteEscapedString(String original) { + if (original.indexOf('\'') == -1) { + return original; + } + StringBuilder builder = new StringBuilder(original.length() + 1); + for (int i = 0; i < original.length(); ++i) { + char c = original.charAt(i); + if (c != '\'') { + builder.append(c); + } else { + builder.append("\\'"); + } + } + return builder.toString(); + } + + private static long parseAndValidateVisitNumericId(String value) { + try { + return Long.parseLong(value); + } catch (NumberFormatException e) { + throw new BadRequestParameterException(SELECTION, "Failed to parse numeric part of selection URI"); + } + } + + private static String documentSelectionFromRequest(RestUri restUri, HttpRequest request) throws BadRequestParameterException { String documentSelection = Optional.ofNullable(request.getProperty(SELECTION)).orElse(""); if (restUri.getGroup().isPresent() && ! restUri.getGroup().get().value.isEmpty()) { if (! documentSelection.isEmpty()) { // TODO why is this restriction in place? Document selection allows composition of location predicate and other expressions - return Response.createErrorResponse( - 400, - "Visiting does not support setting value for group/value in combination with expression, try using only expression parameter instead.", - restUri, - RestUri.apiErrorCodes.GROUP_AND_EXPRESSION_ERROR); + throw new BadRequestParameterException(SELECTION, "Visiting does not support setting value for group/value in combination with expression, try using only expression parameter instead."); } RestUri.Group group = restUri.getGroup().get(); if (group.name == 'n') { - documentSelection = "id.user=" + group.value; + documentSelection = String.format("id.user==%d", parseAndValidateVisitNumericId(group.value)); } else { - documentSelection = "id.group='" + group.value + "'"; + // TODO first pass through Text.validateTextString? Cannot create doc IDs that don't match that anyway... + documentSelection = String.format("id.group=='%s'", singleQuoteEscapedString(group.value)); } } + // TODO try to preemptively parse sub expression to ensure it is complete + return documentSelection; + } + + private HttpResponse handleVisit(RestUri restUri, HttpRequest request) throws RestApiException { + String documentSelection; OperationHandler.VisitOptions options; try { + documentSelection = documentSelectionFromRequest(restUri, request); options = visitOptionsFromRequest(request); } catch (BadRequestParameterException e) { return createInvalidParameterResponse(e.getParameter(), e.getMessage()); 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 eb06af41348..be8915496ac 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 @@ -24,6 +24,9 @@ import org.junit.Ignore; import org.junit.Test; import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.nio.file.Paths; import static org.hamcrest.core.Is.is; @@ -291,7 +294,7 @@ public class RestApiTest { } String visit_test_uri_selection_rewrite = "/document/v1/namespace/document-type/group/abc?continuation=abc"; - String visit_test_response_selection_rewrite = "doc selection: 'id.group='abc''"; + String visit_test_response_selection_rewrite = "doc selection: 'id.group=='abc''"; @Test @@ -302,6 +305,54 @@ public class RestApiTest { assertThat(rest, containsString(visit_test_response_selection_rewrite)); } + private static String encoded(String original) { + try { + return URLEncoder.encode(original, StandardCharsets.UTF_8.name()); + } catch (UnsupportedEncodingException e) { + throw new RuntimeException(e); + } + } + + private String performV1RestCall(String pathSuffix) { + try { + Request request = new Request(String.format("http://localhost:%s/document/v1/namespace/document-type/%s", + getFirstListenPort(), pathSuffix)); + HttpGet get = new HttpGet(request.getUri()); + return doRest(get); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + private void assertResultingDocumentSelection(String suffix, String expected) { + String output = performV1RestCall(suffix); + assertThat(output, containsString(String.format("doc selection: '%s'", expected))); + } + + private void assertGroupDocumentSelection(String group, String expected) { + assertResultingDocumentSelection("group/" + encoded(group), expected); + } + + @Test + public void group_strings_are_escaped() { + assertGroupDocumentSelection("'", "id.group=='\\''"); + assertGroupDocumentSelection("hello 'world'", "id.group=='hello \\'world\\''"); + assertGroupDocumentSelection("' goodbye moon", "id.group=='\\' goodbye moon'"); + } + + private void assertNumericIdFailsParsing(String id) { + String output = performV1RestCall(String.format("number/%s", encoded(id))); + assertThat(output, containsString("Failed to parse numeric part of selection URI")); + } + + @Test + public void invalid_numeric_id_returns_error() { + assertNumericIdFailsParsing("123a"); + assertNumericIdFailsParsing("a123"); + assertNumericIdFailsParsing("0x1234"); + assertNumericIdFailsParsing("\u0000"); + } + @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())); |