patch for Transfer-Encoding: chunked and proxy_cache - check if the file is complete

[Anatoli Marinov]

Thanks Maxim,
You are right.
The following patch is enough:


--- src/http/ngx_http_upstream.c
+++ src/http/ngx_http_upstream.c
@@ -2687,7 +2687,8 @@

                  tf = u->pipe->temp_file;

-                if (u->headers_in.content_length_n == -1
+                if ((u->headers_in.content_length_n == -1
+                        && !u->headers_in.chunked)
                      || u->headers_in.content_length_n
                         == tf->offset - (off_t) r->cache->body_start)
                  {

For chunked responses p->upstream_done should be true even 
p->upstream_eof is true.


On 10/30/2012 06:25 PM, Maxim Dounin wrote:
> Hello!
>
> On Tue, Oct 30, 2012 at 03:39:58PM +0200, Anatoli Marinov wrote:
>
>> This patch is for 1.2.4. It adds some code that checks if the file
>> is complete when Transfer-Encoding: chunked is used.
>>
>> --- nginx-1.2.4/src/http/modules/ngx_http_proxy_module.c 2012-04-23
>> 13:40:01.000000000 +0300
>> +++ nginx-1.2.4_patched/src/http/modules/ngx_http_proxy_module.c
>> 2012-10-30 14:55:24.106188123 +0200
>> @@ -1867,7 +1867,7 @@
>>       return rc;
>>
>>   done:
>>
>> +    r->upstream->chunked_cacheable = 1;
>>       return NGX_DONE;
>>
>>   invalid:
>> --- nginx-1.2.4/src/http/ngx_http_upstream.c    2012-08-06
>> 20:34:08.000000000 +0300
>> +++ nginx-1.2.4_patched/src/http/ngx_http_upstream.c    2012-10-30
>> 15:36:23.126381731 +0200
>> @@ -2687,7 +2687,8 @@
>>
>>                   tf = u->pipe->temp_file;
>>
>> -                if (u->headers_in.content_length_n == -1
>> +                if (((u->headers_in.content_length_n == -1 &&
>> !u->headers_in.chunked) ||
>> +                        (u->headers_in.chunked && u->chunked_cacheable))
>>                       || u->headers_in.content_length_n
>>                          == tf->offset - (off_t) r->cache->body_start)
>>                   {
> Have you tried the u->length / p->length aproach I've already
> suggested at [1]?  In either case I don't think that introduction
> of a protocol-specific flag is a way to go.  It should be either
> length check, or a separate protocol independant flag.
>
> If you are ok with testing headers_in.chunked, you may just safely
> omit any additional checks, as p->upstream_eof case can't happen
> with complete chunked response (p->upstream_done will be set in
> this case).
>
> [1] http://mailman.nginx.org/pipermail/nginx-devel/2012-September/002699.html
>