use standards conformant algorithm to determine request length

[pyfisch]

With the old algorithm CORS preflight requests send by Firefox where considered invalid.

[tailhook]

I think there are a lot of HTTP/1.0 clients which assume than shutdown(SHUT_WR) is enough to know request length. If we treat them as having empty request body, we may treat a body of that request as a next request, which is security issue.

[tailhook]

AFAICS, the link that you cite is for responses not for requests.

Also:

If a message is received with both a Transfer-Encoding and a Content-Length header field, the Transfer-Encoding overrides the Content-Length

I believe that current HTTP spec forbids such entities:

A sender MUST NOT send a Content-Length header field in any message that contains a Transfer-Encoding header field.

[pyfisch]

The section titled "Message Body Length" is about both request and response messages. Some parts of the rules only apply for requests and other apply for all http messages. These rules are applicable for parsing requests at a server.

RFC7230 states in section 3.3.3 "Message Body Length" bullet point 3:

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.

The HTTPbis RFCs are often more strict in what senders are allowed to transmit than what servers are expected to accept.

To prevent request smuggling attacks the RFC added the precise rules I implemented for message framing. For HTTP/1.0 clients rotor-http could decide only to allow persistent connections for requests that clearly indicate the request size. I believe that the rules given in the RFC are sufficient to prevent most request smuggling attacks.

[tailhook]

@pyfisch sorry, for a late reply. Just noticed your response.

For HTTP/1.0 clients rotor-http could decide only to allow persistent connections for requests that clearly indicate the request size.

HTTP/1.0 with request size doesn't allow persistent connections by definition (i.e. it shutdowns writing end and end of request body). Just your pull request discards request body of such request entirely. I believe yes, we can drop the connection-close and method check, but still allow HTTP/1.0. I also believe that we have to at least disable persistent connections for requests that have both Transfer-Encoding and Content-Length.

Also, why do you remove LengthRequired response code? While I believe that BadRequest is perfectly okay, I think more specific error is a little bit better.

[tailhook]

Ah, sorry, just noticed in HTTP/1.0 spec:

Closing the connection cannot be used to indicate the end of a request body, since it leaves no possibility for the server to send back a response.

So your implementation seems valid.

[pyfisch]

The 411 length required response code may only be send if the request contains a body but it does not contain a Content-Length header field. But when the body type is determined we do not know if the request actually contains content after the header fields. There are other cases in which it is impossible to determine body length: invalid Transfer-Encoding or Content-Length header fields. For this reason 400 bad request is returned.

[tailhook]

Merged. Thanks!