Improper handling of requests containing both Content-Length and Transfer-Encoding headers

[kenballus]

From RFC 7230:

If a message is received with both a Transfer-Encoding and a
Content-Length header field, the Transfer-Encoding overrides the
Content-Length. Such a message might indicate an attempt to
perform request smuggling (Section 9.5) or response splitting
(Section 9.4) and ought to be handled as an error. A sender MUST
remove the received Content-Length field prior to forwarding such
a message downstream.

This language was tightened up in RFC 9112:

If a message is received with both a Transfer-Encoding and a Content-Length header field, the Transfer-Encoding overrides the Content-Length. Such a message might indicate an attempt to perform request smuggling (Section 11.2) or response splitting (Section 11.1) and ought to be handled as an error. An intermediary that chooses to forward the message MUST first remove the received Content-Length field and process the Transfer-Encoding (as described below) prior to forwarding the message downstream.

In short, there are two appropriate responses to a request containing both a Content-Length and Transfer-Encoding header:

1. Ignore the Content-Length header. (Recommended by RFC 7230) (this is what Apache, Caddy, and IIS do)
2. Reject the message. (Recommended by RFC 9112) (this is what Nginx, Node, and LiteSpeed do)

Right now, thin has the Content-Length header override the Transfer-Encoding header. This behavior should be updated to match the standards.

[ioquatix]

Feel free to submit a PR! :)

[kenballus]

I'd be happy to.

I spent a little time looking through the parser, and it seems like it doesn't support the chunked transfer encoding at all. Thin's behavior seems to confirm this. (I could be wrong, though.)

If that's the case, then that functionality should definitely be added, and I might be out of my depth handling that on my own as a newcomer to the project. I would be open to collaborating, if you (or anyone) would like.