Opened 4 years ago

Closed 4 years ago

#1037 closed defect (fixed)

NGiNX drops the body of a HTTP proxy response on Linux if proxy doesn't read all content sent to it.

Reported by: bbain.atlassian.com@… Owned by:
Priority: minor Milestone:
Component: nginx-module Version: 1.6.x
Keywords: http proxy Cc:
uname -a: Linux bbainfedora 4.6.4-301.fc24.x86_64 #1 SMP Tue Jul 12 11:50:00 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
nginx -V: nginx version: nginx/1.11.3
built by gcc 6.1.1 20160621 (Red Hat 6.1.1-3) (GCC)
built with OpenSSL 1.0.2h-fips 3 May 2016
TLS SNI support enabled
configure arguments: --with-http_ssl_module --with-http_realip_module --with-http_addition_module --with-http_sub_module --with-http_dav_module --with-http_flv_module --with-http_mp4_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_random_index_module --with-http_secure_link_module --with-http_stub_status_module --with-http_auth_request_module --with-http_geoip_module --with-pcre-jit --with-mail --with-mail_ssl_module --with-file-aio --with-ipv6 --with-cc-opt='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic' --prefix=/home/bbain/nginx/1.11.3 --with-debug

Description

We have NGiNX sitting in front of gnuicorn on Linux. We have a REST resource that completely ignores the body of the request and returns a response and closes the connection. NGiNX returns the body of this resource if the size of proxy request sent (i.e. the bytes written to the socket) to gunicorn is <= 8k. If the request is >8k we end up with a HTTP response without the body.

I am pretty sure I have tracked this down. Linux will report a ECONNRESET error you try and read data on a socket that has been closed but still has outgoing data pending http://stackoverflow.com/questions/2974021/what-does-econnreset-mean-in-the-context-of-an-af-local-socket. So basically:

  1. Client makes connection to NGiNX.
  2. NGiNX send >8k worth of data to Gunicorn.
  3. Gunicorn reads in 8k to parse the headers. This leaves data on the socket.
  4. Gunicorn sends response and closes the socket leaving some data still unread.
  5. NGiNX gets all the response from Gunicorn.
  6. NGiNX gets an ECONNRESET on the next recv call.
  7. NGiNX assumes an error even though it has a complete response from Gunicorn and decides to only send part of the response.

The request works if in step 2 <= 8k is sent to gunicorn. In this case gunicorn will actually read all the data on the socket in its attempt to parse the headers.

I tested on my Linux VM with both 1.6.2 and 1.11.3 (which I compiled with --with-debug). This is also occurring on our production servers where we have 1.6.2 deployed.

Our setup has a unix socket between NGiNX and Gunicorn (though it also happens when using TCP sockets too). I have attached a simple test case to reproduce the problem.

Attachments (1)

bug.tar (75.5 KB ) - added by bbain.atlassian.com@… 4 years ago.
Example test case

Download all attachments as: .zip

Change History (4)

by bbain.atlassian.com@…, 4 years ago

Attachment: bug.tar added

Example test case

comment:1 by Maxim Dounin, 4 years ago

Quoting RFC 7230, 6.6. Tear-down:

   If a server performs an immediate close of a TCP connection, there is
   a significant risk that the client will not be able to read the last
   HTTP response.  If the server receives additional data from the
   client on a fully closed connection, such as another request that was
   sent by the client before receiving the server's response, the
   server's TCP stack will send a reset packet to the client;
   unfortunately, the reset packet might erase the client's
   unacknowledged input buffers before they can be read and interpreted
   by the client's HTTP parser.

   To avoid the TCP reset problem, servers typically close a connection
   in stages.  First, the server performs a half-close by closing only
   the write side of the read/write connection.  The server then
   continues to read from the connection until it receives a
   corresponding close by the client, or until the server is reasonably
   certain that its own TCP stack has received the client's
   acknowledgement of the packet(s) containing the server's last
   response.  Finally, the server fully closes the connection.

A proper solution for this can be only implemented at Gunicorn side. And it looks like Gunicorn already has a ticket about this, see https://github.com/benoitc/gunicorn/issues/1025.

On the other hand, logs you've provided suggest that all the response bytes were already read by nginx at the time it got ECONNRESET, so it should be possible to recover the whole response. Try the following patch:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1470268806 -10800
#      Thu Aug 04 03:00:06 2016 +0300
# Node ID 21de9751d14207e63af9c922e250db1d2b328392
# Parent  b894097d43d3e810f3fef648db4b4eb65bef9530
Event pipe: process data after recv_chain() errors.

When c->recv_chain() returns an error, it is possible that we already
have some data previously read, e.g., in preread buffer.  And in some
cases it may be even a complete response.  The patch changes c->recv_chain()
error handling to process the data, much like it is already done if
kevent reports about an error.

This change, in particular, fixes processing of small responses
when an upstream fails to properly close a connection with lingering and
therefore the connection is reset, but the response is already fully
obtained by nginx (see ticket #1037).

diff --git a/src/event/ngx_event_pipe.c b/src/event/ngx_event_pipe.c
--- a/src/event/ngx_event_pipe.c
+++ b/src/event/ngx_event_pipe.c
@@ -300,7 +300,7 @@ ngx_event_pipe_read_upstream(ngx_event_p
 
             if (n == NGX_ERROR) {
                 p->upstream_error = 1;
-                return NGX_ERROR;
+                break;
             }
 
             if (n == NGX_AGAIN) {

Note though that this will only cover some specific cases. Proper solution should be implemented at the backend side, see above.

comment:2 by bbain.atlassian.com@…, 4 years ago

Thank you very much for the info. The patch fixes the problem for us but as you point out starts hitting problems with large bodies (on my box > 11,000 bytes).

Last edited 4 years ago by bbain.atlassian.com@… (previous) (diff)

comment:3 by Maxim Dounin, 4 years ago

Resolution: fixed
Status: newclosed

The patch above committed as 0fa883e92895.

Note: See TracTickets for help on using tickets.