[patch] Http image_filter: return 405 when method is HEADandbodyis empty.

[胡聪 (hucc)]

Hi,

On Wednesday, Jun 21, 2017 1:38 AM +0300, Maxim Dounin wrote:

>When the response is read from a static file, not proxied, and/or
>proxied with "proxy_method GET;", an empty response means exacly
>that: an empty response, and returning 415 is perfectly correct.
>
>The ngx_buf_size() test will limit the incorrect behaviour to
>indeed empty responses (previous version of your patch affected
>all non-image responses), but it won't eliminate incorrect behaviour
>for empty responses.
>
>The root cause of the problem is that in the configuration you've
>provided proxy don't know about image filter, and uses the HEAD
>method in the request to upstream filter despite the fact image
>filter needs a body to return proper response.  This doesn't look
>like something easy to fix, as proper fix implies some knowledge
>to be passed between image filter and proxy.
>
>Most trivial solution, as suggested above, would be to use
>"proxy_method GET" explicitly in the configuration.  It might be
>actually a good enough solution, as image filter is a special
>module and it requires proper configuration anyway.

Thank you for the detailed explanation. After reading the code again,
I still have two questions. First, is the following change appropriate ?

--- a/ngx_http_image_filter_module.c
+++ b/ngx_http_image_filter_module.c
@@ -330,6 +330,15 @@ ngx_http_image_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
                 }
             }

+            if (r->method & NGX_HTTP_HEAD
+                && ctx->length
+                && ngx_buf_size(in->buf) == 0)
+            {
+                return ngx_http_filter_finalize_request(r,
+                                                  &ngx_http_image_filter_module,
+                                                  NGX_HTTP_NOT_ALLOWED);
+            }
+
             return ngx_http_filter_finalize_request(r,
                                               &ngx_http_image_filter_module,
                                               NGX_HTTP_UNSUPPORTED_MEDIA_TYPE);

Second, Why not restrict the use of the image filter in the following way ?

--- a/ngx_http_image_filter_module.c
+++ b/ngx_http_image_filter_module.c
@@ -224,7 +224,9 @@ ngx_http_image_header_filter(ngx_http_request_t *r)
     ngx_http_image_filter_ctx_t   *ctx;
     ngx_http_image_filter_conf_t  *conf;

-    if (r->headers_out.status == NGX_HTTP_NOT_MODIFIED) {
+    if (r->headers_out.status != NGX_HTTP_OK
+        && r->headers_out.status != NGX_HTTP_PARTIAL_CONTENT)
+    {
         return ngx_http_next_header_filter(r);
     }

Best wishes
-hucc