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