diff --git a/core/src/main/java/io/undertow/server/Connectors.java b/core/src/main/java/io/undertow/server/Connectors.java index afb6e68d5e..7dea9b0295 100644 --- a/core/src/main/java/io/undertow/server/Connectors.java +++ b/core/src/main/java/io/undertow/server/Connectors.java @@ -477,12 +477,34 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin /** * Sets the request path and query parameters, decoding to the requested charset. * - * @param exchange The exchange - * @param encodedPath The encoded path - * @param charset The charset - * @throws BadRequestException + * @param exchange the exchange + * @param encodedPath the encoded path + * @param decode indicates if the request path should be decoded + * @param decodeSlashFlag indicates if slash characters contained in the encoded path should be decoded + * @param decodeBuffer the buffer used for decoding + * @param maxParameters maximum number of parameters allowed in the path + * @param charset the charset + * @throws BadRequestException if there is something wrong with the request, such as non-allowed characters */ public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, final boolean decodeSlashFlag, StringBuilder decodeBuffer, int maxParameters) throws ParameterLimitException, BadRequestException { + setExchangeRequestPath(exchange, encodedPath, charset, decode, decode, decodeSlashFlag, decodeBuffer, maxParameters); + } + + /** + * Sets the request path and query parameters, decoding to the requested charset. + * + * @param exchange the exchange + * @param encodedPath the encoded path + * @param decode indicates if the request path should be decoded, apart from the query string part of the + * request (see next parameter) + * @param decodeQueryString indicates if the query string of the path, when present, should be decoded + * @param decodeSlashFlag indicates if slash characters contained in the request path should be decoded + * @param decodeBuffer the buffer used for decoding + * @param maxParameters maximum number of parameters allowed in the path + * @param charset the charset + * @throws BadRequestException if there is something wrong with the request, such as non-allowed characters + */ + public static void setExchangeRequestPath(final HttpServerExchange exchange, final String encodedPath, final String charset, boolean decode, boolean decodeQueryString, final boolean decodeSlashFlag, StringBuilder decodeBuffer, int maxParameters) throws ParameterLimitException, BadRequestException { final OptionMap options = exchange.getConnection().getUndertowOptions(); final boolean allowUnescapedCharactersInUrl = options.get(UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, false); boolean requiresDecode = false; @@ -520,7 +542,7 @@ public static void setExchangeRequestPath(final HttpServerExchange exchange, fin exchange.setQueryString(qs); } - URLUtils.parseQueryString(qs, exchange, charset, decode, maxParameters); + URLUtils.parseQueryString(qs, exchange, charset, decodeQueryString, maxParameters); return; } else if(c == ';') { String part; diff --git a/core/src/main/java/io/undertow/server/protocol/http2/Http2ReceiveListener.java b/core/src/main/java/io/undertow/server/protocol/http2/Http2ReceiveListener.java index 2f60cdfc41..1eb8aa9896 100644 --- a/core/src/main/java/io/undertow/server/protocol/http2/Http2ReceiveListener.java +++ b/core/src/main/java/io/undertow/server/protocol/http2/Http2ReceiveListener.java @@ -218,15 +218,19 @@ public void handleEvent(Http2StreamSourceChannel channel) { * @param initial The initial upgrade request that started the HTTP2 connection */ void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte[] data) { - handleInitialRequest(initial, channel, data, this.decode); + handleInitialRequest(initial, channel, data, this.decode, this.decode); } /** - * Handles the initial request when the exchange was started by a HTTP upgrade. - * - * @param initial The initial upgrade request that started the HTTP2 connection - */ - void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte[] data, boolean decode) { + * Handles the initial request when the exchange was started by a HTTP upgrade. + * + * @param initial the initial upgrade request that started the HTTP2 connection + * @param channel the channel that received the request + * @param data any extra data read by the channel that has not been parsed yet + * @param decode indicates if the request path should be decoded, apart from the query string + * @param decodeQueryString indicates if the query string of the path, when present, should be decoded + */ + void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte[] data, boolean decode, boolean decodeQueryString) { //we have a request Http2HeadersStreamSinkChannel sink = channel.createInitialUpgradeResponseStream(); final Http2ServerConnection connection = new Http2ServerConnection(channel, sink, undertowOptions, bufferSize, rootHandler); @@ -253,7 +257,7 @@ void handleInitialRequest(HttpServerExchange initial, Http2Channel channel, byte Connectors.terminateRequest(exchange); String uri = exchange.getQueryString().isEmpty() ? initial.getRequestURI() : initial.getRequestURI() + '?' + exchange.getQueryString(); try { - Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, slashDecodingFlag, decodeBuffer, maxParameters); + Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, decodeQueryString, slashDecodingFlag, decodeBuffer, maxParameters); } catch (ParameterLimitException | BadRequestException e) { exchange.setStatusCode(StatusCodes.BAD_REQUEST); exchange.endExchange(); diff --git a/core/src/main/java/io/undertow/server/protocol/http2/Http2UpgradeHandler.java b/core/src/main/java/io/undertow/server/protocol/http2/Http2UpgradeHandler.java index f338ea940e..8e3fe320e8 100644 --- a/core/src/main/java/io/undertow/server/protocol/http2/Http2UpgradeHandler.java +++ b/core/src/main/java/io/undertow/server/protocol/http2/Http2UpgradeHandler.java @@ -176,7 +176,9 @@ public void handleRequest(HttpServerExchange exchange) throws Exception { }, undertowOptions, exchange.getConnection().getBufferSize(), null); channel.getReceiveSetter().set(receiveListener); // don't decode requests from upgrade, they are already decoded by the parser for protocol HTTP 1.1 (HttpRequestParser) - receiveListener.handleInitialRequest(exchange, channel, data, false); + // however, the queries have to be decoded, since this is decoded only once, when the connector parses the query string + // to fill in the query param collection of the HttpServerExchange (see Connectors invoking URLUtils.QUERY_STRING_PARSER) + receiveListener.handleInitialRequest(exchange, channel, data, false, undertowOptions.get(UndertowOptions.DECODE_URL, true)); channel.resumeReceives(); } }); diff --git a/core/src/test/java/io/undertow/server/handlers/LotsOfQueryParametersTestCase.java b/core/src/test/java/io/undertow/server/handlers/LotsOfQueryParametersTestCase.java index 0d2d5edffb..1918a7e498 100644 --- a/core/src/test/java/io/undertow/server/handlers/LotsOfQueryParametersTestCase.java +++ b/core/src/test/java/io/undertow/server/handlers/LotsOfQueryParametersTestCase.java @@ -51,6 +51,7 @@ public class LotsOfQueryParametersTestCase { private static final String QUERY = "QUERY"; private static final String MESSAGE = "Hello Query"; + private static final String NON_DECODED_MESSAGE = "Hello+Query"; private static final int DEFAULT_MAX_PARAMETERS = 1000; private static final int TEST_MAX_PARAMETERS = 10; @@ -142,7 +143,7 @@ public void testLotsOfQueryParameters_MaxParameters_Ok() throws IOException { } qs.deleteCharAt(qs.length()-1); // delete last useless '&' HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/path?" + qs.toString()); - DefaultServer.setUndertowOptions(OptionMap.create(UndertowOptions.MAX_PARAMETERS, TEST_MAX_PARAMETERS)); + DefaultServer.setUndertowOptions(OptionMap.create(UndertowOptions.MAX_PARAMETERS, TEST_MAX_PARAMETERS, UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, true)); HttpResponse result = client.execute(get); Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode()); for (int i = 0; i < TEST_MAX_PARAMETERS; ++i) { @@ -170,7 +171,7 @@ public void testLotsOfQueryParameters_MaxParameters_BadRequest() throws IOExcept } qs.deleteCharAt(qs.length()-1); // delete last useless '&' HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/path?" + qs.toString()); - DefaultServer.setUndertowOptions(OptionMap.create(UndertowOptions.MAX_PARAMETERS, TEST_MAX_PARAMETERS)); + DefaultServer.setUndertowOptions(OptionMap.create(UndertowOptions.MAX_PARAMETERS, TEST_MAX_PARAMETERS, UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, true)); HttpResponse result = client.execute(get); Assert.assertEquals(StatusCodes.BAD_REQUEST, result.getStatusLine().getStatusCode()); } finally { @@ -179,4 +180,30 @@ public void testLotsOfQueryParameters_MaxParameters_BadRequest() throws IOExcept } } + @Test @AjpIgnore + public void testLotsOfQueryParameters_MaxParameters_Ok_UnescapedCharactersNotAllowed() throws IOException { + OptionMap existing = DefaultServer.getUndertowOptions(); + TestHttpClient client = new TestHttpClient(); + try { + StringBuilder qs = new StringBuilder(); + for (int i = 0; i < TEST_MAX_PARAMETERS; ++i) { + qs.append(QUERY + i); + qs.append("="); + qs.append(URLEncoder.encode(MESSAGE + i, "UTF-8")); + qs.append("&"); + } + qs.deleteCharAt(qs.length()-1); // delete last useless '&' + HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + "/path?" + qs.toString()); + DefaultServer.setUndertowOptions(OptionMap.create(UndertowOptions.MAX_PARAMETERS, TEST_MAX_PARAMETERS)); + HttpResponse result = client.execute(get); + Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode()); + for (int i = 0; i < TEST_MAX_PARAMETERS; ++i) { + Header[] header = result.getHeaders(QUERY + i); + Assert.assertEquals(NON_DECODED_MESSAGE + i, header[0].getValue()); + } + } finally { + DefaultServer.setUndertowOptions(existing); + client.getConnectionManager().shutdown(); + } + } }