Opened 3 years ago

Closed 3 years ago

#1442 closed defect (invalid)

May be caused bad request when received with both a Transfer-Encoding and Content-Length header

Reported by: wangfakang@… Owned by:
Priority: minor Milestone:
Component: nginx-core Version: 1.13.x
Keywords: Cc:
uname -a:
nginx -V: before 1.13.x

Description

May be caused bad request when received with both a Transfer-Encoding and Content-Length header.

For example:

$curl nginx.org -H "Transfer-Encoding:111" -H "Content-Length: -1" -I
HTTP/1.1 400 Bad Request
Server: nginx/1.13.3
Date: Mon, 04 Dec 2017 08:45:35 GMT
Content-Type: text/html; charset=utf-8
Content-Length: 173
Connection: close

https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4

As described in the document above. if a Transfer-Encoding header field is present). If a message is received with both a Transfer-Encoding header field and a Content-Length header field, the latter MUST be ignored.

Fixed:

$hg diff

diff -r fc0d06224eda src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c	Tue Nov 28 13:09:54 2017 +0300
+++ b/src/http/ngx_http_request.c	Mon Dec 04 17:10:20 2017 +0800
@@ -1816,7 +1816,7 @@
         return NGX_ERROR;
     }
 
-    if (r->headers_in.content_length) {
+    if (!r->headers_in.transfer_encoding && r->headers_in.content_length) {
         r->headers_in.content_length_n =
                             ngx_atoof(r->headers_in.content_length->value.data,
                                       r->headers_in.content_length->value.len);

Change History (4)

comment:1 by Maxim Dounin, 3 years ago

Resolution: invalid
Status: newclosed

The 400 (Bad Request) response is returned because Content-Length: -1 is invalid. See the ngx_http_process_request_header() function for Transfer-Encoding handling.

in reply to:  1 comment:2 by wangfakang@…, 3 years ago

Replying to mdounin:

The 400 (Bad Request) response is returned because Content-Length: -1 is invalid. See the ngx_http_process_request_header() function for Transfer-Encoding handling.


https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.4

As described in the document above. If a message is received with both a Transfer-Encoding header field and a Content-Length header field, the latter MUST be ignored.

So, I don't think that in this scenario should be checking the validity of content-length. what do you think?

Version 0, edited 3 years ago by wangfakang@… (next)

comment:3 by wangfakang@…, 3 years ago

Resolution: invalid
Status: closedreopened

comment:4 by Maxim Dounin, 3 years ago

Resolution: invalid
Status: reopenedclosed

The same paragraph also says

     ... The Content-Length header field MUST NOT be sent
     if these two lengths are different ...

your test is clearly violating this requirement. Additionally, the provided Content-Length: -1 does not match they Content-Length header syntax,

       Content-Length    = "Content-Length" ":" 1*DIGIT

These are clearly enough to reject such a request as a bad one, and that's what nginx does. The MUST be ignored claim is to avoid accidental use of such misleading Content-Length.

Note well that the claim is not present in the newer RFC 7230, see https://tools.ietf.org/html/rfc7230#section-3.3.3. Instead, it says:

       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.
Note: See TracTickets for help on using tickets.