Hello!

On Thu, Jun 22, 2017 at 01:05:37AM +0800, 胡聪 (hucc) wrote:

> 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);

I don't think it is.

> 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);
> }

The goal of the image filter module is to process all resources
returned, including ones returned with error codes. Such a change
will make it useless at least for the use case the module was
written for.

--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel