Uploaded image for project: 'Undertow'
  1. Undertow
  2. UNDERTOW-2567

Decoding of query strings with unescaped characters does not work in HTTP2 upgrade

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Done
    • Icon: Major Major
    • 2.3.19.Final, 2.2.38.Final
    • None
    • None
    • None

      On HTTP2 upgrade, the request is parsed before the upgrade is done.

      When Http2ReceiveListener.handleInitialRequest is invoked, it receives the original exchange and parses the query string contained in that original exchange:

      exchange.setQueryString(initial.getQueryString()); if (data != null) { Connectors.ungetRequestBytes(exchange, new ImmediatePooledByteBuffer(ByteBuffer.wrap(data))); } Connectors.terminateRequest(exchange);
      String uri = exchange.getQueryString().isEmpty() ? initial.getRequestURI() : initial.getRequestURI() + '?' + exchange.getQueryString();
      try {
          Connectors.setExchangeRequestPath(exchange, uri, encoding, decode, decodeQueryString, slashDecodingFlag, decodeBuffer, maxParameters);
      } catch (ParameterLimitException | BadRequestException e) {
          exchange.setStatusCode(StatusCodes.BAD_REQUEST);
          exchange.endExchange();
          return;
      } 

      Connectors.setExchangeRequestPath is responsible for parsing again the info in the uri.

      However, depending on some configurations used in Undertow, such as ALLOW_UNENCODED_CHARACTERS_IN_URL, DECODE_SLASH, and DECODE_URL, you can get a decoded or unencoded query string, including even a partly decoded (supposed, for example, slashes are encoded, but other characters are decoded in exchange.getQueryString()).

      So, because Connectors cannot know what to expect, it is impossible to make sure that it will do the parsing correctly, as the only correct parsing result is the result obtained by parsing the original request prior to the HTTP2 upgrade.

      Notice that the original input to the request has been discarded after that first parsing. Reconstrucing the original input is possible, but you would need to look at all the configs above and process exchange.getQueryString() to discover what input resulted in that query string, an unnecessary and error prone step.

      The best to do here is to just reuse the original parsing result. Besides being straightforward, it saves us extra cicles, as there is no point in parsing twice the same request.

              flaviarnn Flavia Rainone
              flaviarnn Flavia Rainone
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

                Created:
                Updated:
                Resolved: