Skip to content

Commit d89262e

Browse files
authored
Fix test sending body as url parameter (backport of #65779) (#65969)
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
1 parent 4ff939c commit d89262e

File tree

2 files changed

+32
-8
lines changed

2 files changed

+32
-8
lines changed

rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ setup:
100100
---
101101
"Regexp length limit":
102102
- skip:
103-
version: all
104-
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
103+
version: " - 6.99.99"
104+
reason: "The regex length limit was introduced in 7.0.0"
105105

106106
- do:
107107
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\./

test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,15 @@
1919
package org.elasticsearch.test.rest.yaml;
2020

2121
import com.carrotsearch.randomizedtesting.RandomizedTest;
22+
2223
import org.apache.http.HttpEntity;
2324
import org.apache.http.HttpHost;
25+
import org.apache.http.NameValuePair;
26+
import org.apache.http.ParseException;
2427
import org.apache.http.client.methods.HttpGet;
28+
import org.apache.http.client.utils.URLEncodedUtils;
2529
import org.apache.http.entity.ContentType;
30+
import org.apache.http.message.BasicNameValuePair;
2631
import org.apache.http.util.EntityUtils;
2732
import org.apache.logging.log4j.LogManager;
2833
import org.apache.logging.log4j.Logger;
@@ -44,6 +49,7 @@
4449
import java.io.UncheckedIOException;
4550
import java.net.URI;
4651
import java.net.URISyntaxException;
52+
import java.nio.charset.StandardCharsets;
4753
import java.util.Arrays;
4854
import java.util.HashMap;
4955
import java.util.List;
@@ -52,6 +58,8 @@
5258
import java.util.Set;
5359
import java.util.stream.Collectors;
5460

61+
import static com.carrotsearch.randomizedtesting.RandomizedTest.frequently;
62+
5563
/**
5664
* Used by {@link ESClientYamlSuiteTestCase} to execute REST requests according to the tests written in yaml suite files. Wraps a
5765
* {@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<String, String> params
158166
}
159167
String contentType = entity.getContentType().getValue();
160168
//randomly test the GET with source param instead of GET/POST with body
161-
if (sendBodyAsSourceParam(supportedMethods, contentType, entity.getContentLength())) {
169+
if (sendBodyAsSourceParam(supportedMethods, contentType, entity)) {
162170
logger.debug("sending the request body as source param with GET method");
163171
queryStringParams.put("source", EntityUtils.toString(entity));
164172
queryStringParams.put("source_content_type", contentType);
@@ -215,25 +223,41 @@ protected static void setOptions(Request request, Map<String, String> headers) {
215223
request.setOptions(options);
216224
}
217225

218-
private static boolean sendBodyAsSourceParam(List<String> supportedMethods, String contentType, long contentLength) {
226+
private static boolean sendBodyAsSourceParam(List<String> supportedMethods, String contentType, HttpEntity entity)
227+
throws ParseException, IOException {
219228
if (false == supportedMethods.contains(HttpGet.METHOD_NAME)) {
220229
// The API doesn't claim to support GET anyway
221230
return false;
222231
}
223-
if (contentLength < 0) {
232+
if (entity.getContentLength() < 0) {
224233
// Negative length means "unknown" or "huge" in this case. Either way we can't send it as a parameter
225234
return false;
226235
}
227-
if (contentLength > 2000) {
228-
// Long bodies won't fit in the parameter and will cause a too_long_frame_exception
236+
if (entity.getContentLength() > 2000) {
237+
/*
238+
* HTTP lines longer than 4096 bytes will cause a too_long_frame_exception
239+
* so we chop at 2000 just to give us some room for extra parameters and
240+
* url encoding.
241+
*/
229242
return false;
230243
}
231244
if (false == contentType.startsWith(ContentType.APPLICATION_JSON.getMimeType())
232245
&& false == contentType.startsWith(YAML_CONTENT_TYPE.getMimeType())) {
233246
// We can only encode JSON or YAML this way.
234247
return false;
235248
}
236-
return RandomizedTest.rarely();
249+
if (frequently()) {
250+
return false;
251+
}
252+
/*
253+
* Now, the last (expensive) test: make sure the *url encoded* size
254+
* isn't too big. We limit ourselves to 3000 bytes for the source of
255+
* the request out of 4096 so we can use the rest for other parameters
256+
* and the url and stuff.
257+
*/
258+
NameValuePair param = new BasicNameValuePair("source", EntityUtils.toString(entity));
259+
String encoded = URLEncodedUtils.format(org.elasticsearch.common.collect.List.of(param), StandardCharsets.UTF_8);
260+
return encoded.length() < 3000;
237261
}
238262

239263
private ClientYamlSuiteRestApi restApi(String apiName) {

0 commit comments

Comments
 (0)