[UNDERTOW-2312] Fix ALLOW_UNESCAPED_CHARACTERS_IN_URL with query strings in Http2 upgrade

The  new code caused ALLOW_UNESCAPED_CHARACTERS_IN_URL to not work correctly in HTTP2 upgrade scenarios when the characters are in the query. The reason for that is that Http2UpgradeHandler passes
decode as false when asking Connectors to parse the request path, assuming that otherwise the decode would have been done twice. While that may the true for the parsing of the path, it is not the case when Connectors is parsing query params to set them one-by-one in the HttpServerExchange.This caused LotsOfQueryParametersTestCase to fail when run with HTTP2 Upgrade config.

Signed-off-by: Flavia Rainone <frainone@redhat.com>

Author: fl4via

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;

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();

core/src/main/java/io/undertow/server/protocol/http2/Http2UpgradeHandler.java

@@ -176,7 +176,12 @@ 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)
+                final boolean decodeURL = undertowOptions.get(UndertowOptions.DECODE_URL, true);
+                final boolean allowUnescapedCharactersInURL = undertowOptions.get(UndertowOptions.ALLOW_UNESCAPED_CHARACTERS_IN_URL, false);
+                // if allowUnescapedCharactersInURL is true, the decoding has already been done
+                receiveListener.handleInitialRequest(exchange, channel, data, decodeURL && !allowUnescapedCharactersInURL, decodeURL);
                 channel.resumeReceives();
             }
         });