Jetty.project: Request with `Content-Encoding: gzip` should not perform parameter extraction

[joakime]

Since Jetty does not support Request decompression, if a request shows up with Content-Encoding: gzip and the servlet does a HttpServletRequest.getParameter*() method call, then this should automatically fail, as its not possible for Jetty to satisfy this kind of request.

[joakime]

Perhaps access to HttpServletRequest.getInputStream() should also be protected.

[sbordet]

@joakime HttpServletRequest.getInputStream() should be accessible from applications so that they can ungzip the content.

[joakime]

@sbordet i'm not saying that HttpServletRequest.getInputStream() should be disabled, just that in the specific case of ...

Client:

GET /foo?key=value
Content-Type: application/x-www-form-urlencoded; charset=utf-8
Content-Encoding: gzip
Content-Length: 4000001
Host: server

<lots of binary. compressed gzip. containing form urlencoded data>

arrives at to a Jetty Server which ..

• Does not have GzipHandler specified.
• Is dispatched to a ServletContext
• Is dispatched to a user HttpServlet
• User HttpServlet does HttpServletRequest.getParameter("key")

In this scenario, the part of Jetty's Request.getParameters() that performs Request body content parsing for application/x-www-form-urlencoded cannot work, and should be skipped.
This just means that the Request.getParameters() method should not perform content extraction if the Content-Encoding exists and is one of the compressed formats.

Now, if GzipHandler is present, it will wire up the HttpInput Interceptor and automatically decompress the request body, which also (should be) removes the Content-Encoding header from the request.

[gregw]

I think the HttpServletRequest.getParameter("key") and any other attempt to extract the parameters should throw an IllegalStateException or perhaps BadMessageException(HttpStatus.NOT_IMPLEMENTED_501)

[slonopotamus]

This suddenly broke our software. I think you're violating documentation contract of javax.servlet.ServletRequest#getParameterMap by throwing exception from it. Documentation doesn't say that getParameterMap is ever supposed to throw an exception. It even isn't actually clear how user is supposed to work with getParameterMap. Should I check the same condition that you check before calling getParameterMap? Why you cannot simply return an empty map?

UPD: Okay, there is a bug in our client, it erroneously sends "Content-Type: UTF-8" instead of "Accept-Charset: UTF-8". But this doesn't sound like a good excuse to throw exceptions from random servlet api methods on server.

[joakime]

@slonopotamus if you send a Content-Encoding and there's nothing configured on the server to understand that Content-Encoding then you'll at best wind up with incredibly bad data with HttpServletRequest.getParameterMap(), or a BadMessageException when it cannot parse the unparseble body content.

An Empty Map has proven to be problematic as it hides issues, and is just as bad a Map with bad data, quietly continuing with bad or incomplete data. Which will cause problems with RequestDispatching, JSPs, JSFs, and even other libraries like Spring.

The correct technique is to fail the request, but with the Servlet spec we don't pre-parse the request body content before dispatching the request. We can only fail when the APIs are used to access the data (during a dispatch) in the Servlet spec defined way. There's nothing preventing a Servlet from skipping and not actually using .getParameterMap(), choosing to ignore Content-Encoding and Content-Type and reading the request body content its own way. (a generally questionable approach, but not forbidden).

The .getParameterMap() (and associated APIs) have always thrown Exceptions, even from the earliest days of the Servlet spec.
eg:

• You have an IOException, that's wrapped in a RuntimeIOException and causes the API to fail.
• You have improper URL encoding in your query string (boom a encoding exception, illegal state exception, illegal argument exception)
• You have declared Content-Type: application/x-www-form-urlencoded and are not sending url encoded form content, or are using unencoded UTF-8, or are not using UTF-8 (boom, encoding exception, illegal state exception, or illegal argument exception)
• You are sending too many key/value pairs (illegal state exception)
• You have declared Content-Type: multipart/form-data and have bad body content (encoding exceptions, illegal state exceptions, spec violation exceptions, illegal argument exceptions, base 64 exceptions, quoted-printable exceptions, incomplete parts, filesystem exceptions, io exceptions etc)

[SebasPalm92]

There is really no reason to deny the a request when it contains a Content Encoding: identity.
identity: Indicates the identity function (i.e., no compression or modification). This token, except if explicitly specified, is always deemed acceptable.
(https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Encoding)

[joakime]

@SebasPalm92 developer.mozilla.org is not the HTTP spec.

If you have Content-Encoding: identity then you have an invalid header per RFC7231.

Why?
Because the identity Content Coding token is only for Accept-Encoding per the RFC spec, and also the registered at the IANA Content Codings as reserved (not used) for "no encoding" on the Accept-Encoding header.

• https://tools.ietf.org/html/rfc7231#section-3.1.2.1 - Content Codings
• https://tools.ietf.org/html/rfc7231#section-5.3.4 - Accept-Encoding Header
• http://www.iana.org/assignments/http-parameters/http-parameters.xhtml#content-coding - IANA Registerd Content Codings

Even in the older (now obsolete) RFC2616, the identity Content-Coding value has the following text ...
https://tools.ietf.org/html/rfc2616#section-3.5

    identity
        The default (identity) encoding; the use of no transformation
        whatsoever. This content-coding is used only in the Accept-
        Encoding header, and SHOULD NOT be used in the Content-Encoding
        header.