[PATCH] always process short preread body

[John Fremlin]

Hi Maxim,

Sorry for the delay

Maxim Dounin wrote:
>>>> 1. Backend, which hasn't closed connection after sending reply.
>>>
>>> Yes, the backend does not close.
>>
>> Ok, so I was right and backend isn't http-complaint.
>>
>>>> 2. The fact that nginx waits for connection close before it
>>>> realizes that request was completed, even if got 'Content-Length'
>>>> header and appropriate number of body bytes.
>>>
>>> And 3. nginx never forwards the body of the message after the time out,
>>> (even though it has read it) sending only the header
>>> with content-length!
>>
>> Yep, thats another problem in nginx with handling such buggy backends.
> 
> [...]
> 
>> The problem is that patch penalizes configurations with non-buggy  
>> backends in an attempt to solve problem with buggy ones.
>>
>> I'm not SPEWS^W^W not Igor, but it looks for me that the chances the 
>> patch will be accepted is rather small.
>>
>> On the other hand I've seen at least two similar reports in last month 
>> or two, and it will be good for nginx to accept such backend replies.  
>> And handling of this situation correctly is anyway required to support 
>> persistent connections in future (if any).
>>
>> I believe I will be able to reproduce the problem and I'll try to  
>> produce more correct patch as time permits.
> 
> I've finally build two patches for this, attached.
> 
> The patch-nginx-proxy-flush.txt patch adds flushing of partially filled 
> buffers in pipe if upstream times out.  This makes nginx replies as 
> close as possible to upstream's ones.
> 
> The patch-nginx-proxy-length.txt allows nginx to use Content-Length it 
> got from upstream as a hint that there is no need to wait for upstream's 
> connection close.
> 
> Patches are independent and apply cleanly in any order.
> 
> Igor, could you please take a look?


These patches definitely fix our problem. Thanks

To reproduce easily see this toy Ruby script webserver which 
deliberately exposes the problem

require 'socket'

server = TCPServer.new("127.0.0.1", 3000)

text = "All my base are belong to who?\n"

loop do
   session = server.accept
   request = session.gets

   header = session.gets until header =~ /^\s*$/

   puts "#{Time.now} #{session.peeraddr[3]} #{request}"

   session.puts "HTTP/1.0 200 OK"
   session.puts "Connection: close"
   session.puts "Content-Length: #{text.size}"
   session.puts
   session.write text
   session.flush
end

This server does not close the connection, only flushes. Without the 
patch, nginx will timeout and never send the body. With the patch, it 
works fine.

The real problem occurs sporadically because of the real mongrel 
webserver's borked HTTP handling. I appreciate that it is out of spec 
but it would be nice if nginx handled this case.