From 97a3829fa0433b5825e136a13907e7a49c58e394 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 7 Dec 2020 12:26:34 -0500 Subject: [PATCH 1/3] Fix test sending body as url parameter (backport of #65779) Our test framework randomly sends the body of requests over the `source` parameter. We never send the body if it is more than 2000 bytes becuse our HTTP receiver can't handle lines more the 4096 bytes. The thing is, when we do that 2000 bytes check we do it against the reported length of body, not the body after it has been url encoded. Than url encoding strings can *vastly* increase their size. Which could cause us to send some request over the URL that are longer than 4096 bytes. This fixes that by checking the url encoded length as well. We keep the 2000 byte check of the unencoded length because it is a nice fast check, even if it is a bit inaccurate. Closes #65718 --- .../rest-api-spec/test/search/30_limits.yml | 6 ++-- .../test/rest/yaml/ClientYamlTestClient.java | 36 +++++++++++++++---- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml index 8c2a8fd01eaff..43c23189b942b 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml @@ -99,9 +99,9 @@ setup: --- "Regexp length limit": - - skip: - version: all - reason: Long regex breaks HTTP query length when request body is sent as a query param - https://github.com/elastic/elasticsearch/issues/65718 - change back to version - 6.99.99 afterwards + - skip: + version: " - 6.99.99" + reason: "The regex length limit was introduced in 7.0.0" - do: catch: /The length of regex \[1110\] used in the Regexp Query request has exceeded the allowed maximum of \[1000\]\. This maximum can be set by changing the \[index.max_regex_length\] index level setting\./ diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java index 64b61773461f6..da1828bb4f82e 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java @@ -19,10 +19,15 @@ package org.elasticsearch.test.rest.yaml; import com.carrotsearch.randomizedtesting.RandomizedTest; + import org.apache.http.HttpEntity; import org.apache.http.HttpHost; +import org.apache.http.NameValuePair; +import org.apache.http.ParseException; import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.utils.URLEncodedUtils; import org.apache.http.entity.ContentType; +import org.apache.http.message.BasicNameValuePair; import org.apache.http.util.EntityUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -44,6 +49,7 @@ import java.io.UncheckedIOException; import java.net.URI; import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -52,6 +58,8 @@ import java.util.Set; import java.util.stream.Collectors; +import static com.carrotsearch.randomizedtesting.RandomizedTest.frequently; + /** * Used by {@link ESClientYamlSuiteTestCase} to execute REST requests according to the tests written in yaml suite files. Wraps a * {@link RestClient} instance used to send the REST requests. Holds the {@link ClientYamlSuiteRestSpec} used to translate api calls into @@ -158,7 +166,7 @@ public ClientYamlTestResponse callApi(String apiName, Map params } String contentType = entity.getContentType().getValue(); //randomly test the GET with source param instead of GET/POST with body - if (sendBodyAsSourceParam(supportedMethods, contentType, entity.getContentLength())) { + if (sendBodyAsSourceParam(supportedMethods, contentType, entity)) { logger.debug("sending the request body as source param with GET method"); queryStringParams.put("source", EntityUtils.toString(entity)); queryStringParams.put("source_content_type", contentType); @@ -215,17 +223,22 @@ protected static void setOptions(Request request, Map headers) { request.setOptions(options); } - private static boolean sendBodyAsSourceParam(List supportedMethods, String contentType, long contentLength) { + private static boolean sendBodyAsSourceParam(List supportedMethods, String contentType, HttpEntity entity) + throws ParseException, IOException { if (false == supportedMethods.contains(HttpGet.METHOD_NAME)) { // The API doesn't claim to support GET anyway return false; } - if (contentLength < 0) { + if (entity.getContentLength() < 0) { // Negative length means "unknown" or "huge" in this case. Either way we can't send it as a parameter return false; } - if (contentLength > 2000) { - // Long bodies won't fit in the parameter and will cause a too_long_frame_exception + if (entity.getContentLength() > 2000) { + /* + * HTTP lines longer than 4096 bytes will cause a too_long_frame_exception + * so we chop at 2000 just to give us some room for extra parameters and + * url encoding. + */ return false; } if (false == contentType.startsWith(ContentType.APPLICATION_JSON.getMimeType()) @@ -233,7 +246,18 @@ private static boolean sendBodyAsSourceParam(List supportedMethods, Stri // We can only encode JSON or YAML this way. return false; } - return RandomizedTest.rarely(); + if (frequently()) { + return false; + } + /* + * Now, the last (expensive) test: make sure the *url encoded* size + * isn't too big. We limit ourselves to 3000 bytes for the source of + * the request out of 4096 so we can use the rest for other parameters + * and the url and stuff. + */ + NameValuePair param = new BasicNameValuePair("source", EntityUtils.toString(entity)); + String encoded = URLEncodedUtils.format(List.of(param), StandardCharsets.UTF_8); + return encoded.length() < 3000; } private ClientYamlSuiteRestApi restApi(String apiName) { From 3814d63bca24808ccba7d76a93ade984538359f9 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 7 Dec 2020 12:38:02 -0500 Subject: [PATCH 2/3] Fixup --- .../org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java index da1828bb4f82e..b4f0dc0e67f87 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java @@ -256,7 +256,7 @@ private static boolean sendBodyAsSourceParam(List supportedMethods, Stri * and the url and stuff. */ NameValuePair param = new BasicNameValuePair("source", EntityUtils.toString(entity)); - String encoded = URLEncodedUtils.format(List.of(param), StandardCharsets.UTF_8); + String encoded = URLEncodedUtils.format(org.elasticsearch.common.collect.List.of(param), StandardCharsets.UTF_8); return encoded.length() < 3000; } From 143c561d6ff2ef5fcfc0d7ed494c33ceef1ddc7e Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 7 Dec 2020 16:01:26 -0500 Subject: [PATCH 3/3] Fixup --- .../src/main/resources/rest-api-spec/test/search/30_limits.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml index 43c23189b942b..2818b07d6b578 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml @@ -99,7 +99,7 @@ setup: --- "Regexp length limit": - - skip: + - skip: version: " - 6.99.99" reason: "The regex length limit was introduced in 7.0.0"