<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Thank you for the feedback- <a href="http://trac.nginx.org/nginx/ticket/756" class="">http://trac.nginx.org/nginx/ticket/756</a><div class=""><div class=""><br class=""></div><div class="">Regarding the fix, I do not believe bytes are silently skipped- if you look further below in src/http/modules/ngx_http_image_filter_module.c:</div><div class=""><br class=""></div><div class="">switch (ctx->phase) {</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>case NGX_HTTP_IMAGE_START:</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>ctx->type = ngx_http_image_test(r, in);</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>….</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>if (ctx->type == NGX_HTTP_IMAGE_NONE) {</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>….</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>/* wait to consume more data, do not give up on the request right away */</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>return NGX_OK; /* as in the patch */</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>}</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>….</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>/* fall-through */</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>case NGX_HTTP_IMAGE_READ:</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>…</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>/* fall-through */</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>case NGX_HTTP_IMAGE_PROCESS:</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>…</div><div class="">}</div><div class=""><br class=""></div><div class="">i.e. that section of code is, correctly, designed to maintain state and be called repeatedly for the same image. In any case, we are no longer able to replicate the bug once we have applied the patch.</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">I think a better fix would be to "return NGX_OK" if we do not have enough data in “case NGX_HTTP_IMAGE_START", and "return NGX_HTTP_UNSUPPORTED_MEDIA_TYPE" (as per the original code) if enough data has been read, but it’s really not an image- but this exceeds the scope of the fix and my use case.</div><div class=""><br class=""></div><div class="">Thoughts ?</div><div class=""><br class=""></div><div class="">Thank you,</div><div class="">Dan</div><div class=""><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Apr 29, 2015, at 2:32 AM, Maxim Dounin <<a href="mailto:mdounin@mdounin.ru" class="">mdounin@mdounin.ru</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">Hello!<br class=""><br class="">On Tue, Apr 28, 2015 at 04:56:54PM +0700, Dan Podeanu wrote:<br class=""><br class=""><blockquote type="cite" class=""><br class="">I have encountered a bug in ngx_http_image_filter_module when used in conjunction with ngx_http_proxy_module ; the configuration is as following:<br class=""><br class="">location /img/ {<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>proxy_pass <a href="http://mybucket.s3.amazonaws.com" class="">http://mybucket.s3.amazonaws.com</a> <<a href="http://mybucket.s3.amazonaws.com/" class="">http://mybucket.s3.amazonaws.com/</a>>;<br class=""><span class="Apple-tab-span" style="white-space:pre"> </span>image_filter resize 150 100;<br class="">}<br class=""><br class="">The steps to reproduce are rather complicated as they depend on how TCP fragments the response coming from the proxy:<br class=""><br class="">- If <a href="http://mybucket.s3.amazonaws.com" class="">http://mybucket.s3.amazonaws.com</a> <<a href="http://mybucket.s3.amazonaws.com/" class="">http://mybucket.s3.amazonaws.com/</a>> returns, in the first TCP packet, a sizable amount of data (1-2k), the image is resized correctly.<br class=""><br class="">- If <a href="http://mybucket.s3.amazonaws.com" class="">http://mybucket.s3.amazonaws.com</a> <<a href="http://mybucket.s3.amazonaws.com/" class="">http://mybucket.s3.amazonaws.com/</a>> returns, in the first TCP packet, a small amount of data (HTTP header, or HTTP header + a few bytes), the content is marked as not an image and NGX_HTTP_UNSUPPORTED_MEDIA_TYPE is returned (disconnecting the client), irrespective on whether or not subsequent data would complete the response to a valid image.<br class=""><br class="">Nginx appears to give up right away on waiting for data if the contents of the first TCP packet received from the proxy does not contain a valid image header- i.e. ngx_http_image_test() will return NGX_HTTP_IMAGE_SIZE, etc.<br class=""><br class="">In my experience this was triggered by a subtle change in AWS S3 that fragments the TCP header more.<br class=""><br class="">Versions affected: 1.6.2, 1.6.3, 1.7.2, 1.8.0, etc.<br class=""><br class=""><br class=""><br class="">Attaching a patch which fixes it for my use case (1.6.2) below; the other versions can be fixed similarly:<br class=""><br class="">————————————————————————<br class="">diff -ruN nginx-1.6.2.orig/src/http/modules/ngx_http_image_filter_module.c nginx-1.6.2/src/http/modules/ngx_http_image_filter_module.c<br class="">--- nginx-1.6.2.orig/src/http/modules/ngx_http_image_filter_module.c<span class="Apple-tab-span" style="white-space:pre"> </span>2014-09-16 19:23:19.000000000 +0700<br class="">+++ nginx-1.6.2/src/http/modules/ngx_http_image_filter_module.c<span class="Apple-tab-span" style="white-space:pre"> </span>2015-04-28 16:25:47.000000000 +0700<br class="">@@ -317,9 +317,8 @@<br class=""> }<br class=""> }<br class=""><br class="">- return ngx_http_filter_finalize_request(r,<br class="">- &ngx_http_image_filter_module,<br class="">- NGX_HTTP_UNSUPPORTED_MEDIA_TYPE);<br class="">+ /* wait to consume more data, do not give up on the request right away */<br class="">+ return NGX_OK;<br class=""> }<br class=""><br class=""> /* override content type */<br class="">————————————————————————<br class=""><br class=""><br class="">Thoughts ? Is this the right fix ? Should I also post on the nginx Trac ?<br class=""></blockquote><br class="">Thanks, looks like a valid problem - and yes, please file a ticket <br class="">on trac. The fix doesn't looks correct for me though - with such <br class="">a change nginx will silently skip first bytes of a response.<br class=""><br class="">-- <br class="">Maxim Dounin<br class=""><a href="http://nginx.org/" class="">http://nginx.org/</a><br class=""><br class="">_______________________________________________<br class="">nginx-devel mailing list<br class="">nginx-devel@nginx.org<br class="">http://mailman.nginx.org/mailman/listinfo/nginx-devel</div></blockquote></div><br class=""></div></div></div></body></html>