Move raw path into HttpPreRequest (#113231)

Currently, the raw path is only available from the RestRequest. This
makes the logic to determine if a handler supports streaming more
challenging to evaluate. This commit moves the raw path into pre request
to allow easier streaming support logic.
This commit is contained in:
Tim Brooks 2024-09-20 13:32:45 -06:00 committed by GitHub
parent b9855b8e4e
commit c5caf84e2d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 38 additions and 19 deletions

View file

@ -17,6 +17,7 @@ import io.netty.handler.codec.http.FullHttpRequest;
import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpHeaders;
import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.QueryStringDecoder;
import io.netty.handler.codec.http.cookie.Cookie; import io.netty.handler.codec.http.cookie.Cookie;
import io.netty.handler.codec.http.cookie.ServerCookieDecoder; import io.netty.handler.codec.http.cookie.ServerCookieDecoder;
import io.netty.handler.codec.http.cookie.ServerCookieEncoder; import io.netty.handler.codec.http.cookie.ServerCookieEncoder;
@ -48,6 +49,7 @@ public class Netty4HttpRequest implements HttpRequest {
private final Exception inboundException; private final Exception inboundException;
private final boolean pooled; private final boolean pooled;
private final int sequence; private final int sequence;
private final QueryStringDecoder queryStringDecoder;
Netty4HttpRequest(int sequence, io.netty.handler.codec.http.HttpRequest request, Netty4HttpRequestBodyStream contentStream) { Netty4HttpRequest(int sequence, io.netty.handler.codec.http.HttpRequest request, Netty4HttpRequestBodyStream contentStream) {
this( this(
@ -94,6 +96,7 @@ public class Netty4HttpRequest implements HttpRequest {
this.pooled = pooled; this.pooled = pooled;
this.released = released; this.released = released;
this.inboundException = inboundException; this.inboundException = inboundException;
this.queryStringDecoder = new QueryStringDecoder(request.uri());
} }
@Override @Override
@ -106,6 +109,11 @@ public class Netty4HttpRequest implements HttpRequest {
return request.uri(); return request.uri();
} }
@Override
public String rawPath() {
return queryStringDecoder.rawPath();
}
@Override @Override
public HttpBody body() { public HttpBody body() {
assert released.get() == false; assert released.get() == false;

View file

@ -374,10 +374,7 @@ public class Netty4HttpServerTransport extends AbstractHttpServerTransport {
// combines the HTTP message pieces into a single full HTTP request (with headers and body) // combines the HTTP message pieces into a single full HTTP request (with headers and body)
final HttpObjectAggregator aggregator = new Netty4HttpAggregator( final HttpObjectAggregator aggregator = new Netty4HttpAggregator(
handlingSettings.maxContentLength(), handlingSettings.maxContentLength(),
httpPreRequest -> enabled.get() == false httpPreRequest -> enabled.get() == false || (httpPreRequest.rawPath().endsWith("/_bulk") == false)
|| (httpPreRequest.uri().contains("_bulk") == false
|| httpPreRequest.uri().contains("_bulk_update")
|| httpPreRequest.uri().contains("/_xpack/monitoring/_bulk"))
); );
aggregator.setMaxCumulationBufferComponents(transport.maxCompositeBufferComponents); aggregator.setMaxCumulationBufferComponents(transport.maxCompositeBufferComponents);
ch.pipeline() ch.pipeline()

View file

@ -29,6 +29,12 @@ import static org.hamcrest.Matchers.equalTo;
@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 2, numClientNodes = 0) @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 2, numClientNodes = 0)
public class IncrementalBulkRestIT extends HttpSmokeTestCase { public class IncrementalBulkRestIT extends HttpSmokeTestCase {
public void testBulkUriMatchingDoesNotMatchBulkCapabilitiesApi() throws IOException {
Request request = new Request("GET", "/_capabilities?method=GET&path=%2F_bulk&capabilities=failure_store_status&pretty");
Response response = getRestClient().performRequest(request);
assertEquals(200, response.getStatusLine().getStatusCode());
}
public void testBulkMissingBody() throws IOException { public void testBulkMissingBody() throws IOException {
Request request = new Request(randomBoolean() ? "POST" : "PUT", "/_bulk"); Request request = new Request(randomBoolean() ? "POST" : "PUT", "/_bulk");
request.setJsonEntity(""); request.setJsonEntity("");

View file

@ -33,6 +33,24 @@ public interface HttpPreRequest {
*/ */
String uri(); String uri();
/**
* The uri without the query string.
*/
default String rawPath() {
String uri = uri();
final int index = uri.indexOf('?');
if (index >= 0) {
return uri.substring(0, index);
} else {
final int index2 = uri.indexOf('#');
if (index2 >= 0) {
return uri.substring(0, index2);
} else {
return uri;
}
}
}
/** /**
* Get all of the headers and values associated with the HTTP headers. * Get all of the headers and values associated with the HTTP headers.
* Modifications of this map are not supported. * Modifications of this map are not supported.

View file

@ -105,19 +105,19 @@ public class RestRequest implements ToXContent.Params, Traceable {
protected RestRequest( protected RestRequest(
XContentParserConfiguration parserConfig, XContentParserConfiguration parserConfig,
Map<String, String> params, Map<String, String> params,
String path, String rawPath,
Map<String, List<String>> headers, Map<String, List<String>> headers,
HttpRequest httpRequest, HttpRequest httpRequest,
HttpChannel httpChannel HttpChannel httpChannel
) { ) {
this(parserConfig, params, path, headers, httpRequest, httpChannel, requestIdGenerator.incrementAndGet()); this(parserConfig, params, rawPath, headers, httpRequest, httpChannel, requestIdGenerator.incrementAndGet());
} }
@SuppressWarnings("this-escape") @SuppressWarnings("this-escape")
private RestRequest( private RestRequest(
XContentParserConfiguration parserConfig, XContentParserConfiguration parserConfig,
Map<String, String> params, Map<String, String> params,
String path, String rawPath,
Map<String, List<String>> headers, Map<String, List<String>> headers,
HttpRequest httpRequest, HttpRequest httpRequest,
HttpChannel httpChannel, HttpChannel httpChannel,
@ -149,7 +149,7 @@ public class RestRequest implements ToXContent.Params, Traceable {
: parserConfig.withRestApiVersion(effectiveApiVersion); : parserConfig.withRestApiVersion(effectiveApiVersion);
this.httpChannel = httpChannel; this.httpChannel = httpChannel;
this.params = params; this.params = params;
this.rawPath = path; this.rawPath = rawPath;
this.headers = Collections.unmodifiableMap(headers); this.headers = Collections.unmodifiableMap(headers);
this.requestId = requestId; this.requestId = requestId;
} }
@ -204,11 +204,10 @@ public class RestRequest implements ToXContent.Params, Traceable {
*/ */
public static RestRequest request(XContentParserConfiguration parserConfig, HttpRequest httpRequest, HttpChannel httpChannel) { public static RestRequest request(XContentParserConfiguration parserConfig, HttpRequest httpRequest, HttpChannel httpChannel) {
Map<String, String> params = params(httpRequest.uri()); Map<String, String> params = params(httpRequest.uri());
String path = path(httpRequest.uri());
return new RestRequest( return new RestRequest(
parserConfig, parserConfig,
params, params,
path, httpRequest.rawPath(),
httpRequest.getHeaders(), httpRequest.getHeaders(),
httpRequest, httpRequest,
httpChannel, httpChannel,
@ -229,15 +228,6 @@ public class RestRequest implements ToXContent.Params, Traceable {
return params; return params;
} }
private static String path(final String uri) {
final int index = uri.indexOf('?');
if (index >= 0) {
return uri.substring(0, index);
} else {
return uri;
}
}
/** /**
* Creates a new REST request. The path is not decoded so this constructor will not throw a * Creates a new REST request. The path is not decoded so this constructor will not throw a
* {@link BadParameterException}. * {@link BadParameterException}.