<div dir="ltr"><div><div><div><div dir="auto">Hi, Vadim.</div></div></div><div dir="auto"><span style="word-spacing:1px;border-color:rgb(0,0,0)"><br></span></div><div dir="auto"><span style="word-spacing:1px;border-color:rgb(0,0,0)">Thanks for your cooperation!</span><br></div></div><div><div dir="auto"><br></div><div dir="auto">> <span style="word-spacing:1px;color:rgb(49,49,49)">@@ -5067,6 +5091,7 @@</span></div><span style="word-spacing:1px;color:rgb(49,49,49)">> || (h->value.len == 1 && h->value.data[0] == '*'))</span><br style="color:rgb(49,49,49);word-spacing:1px"><span style="word-spacing:1px;color:rgb(49,49,49)">> {</span><br style="color:rgb(49,49,49);word-spacing:1px"></div><div><span style="word-spacing:1px;color:rgb(49,49,49)">> u->cacheable = 0;</span><br style="color:rgb(49,49,49);word-spacing:1px"><span style="word-spacing:1px;color:rgb(49,49,49)">> + u->cacheable_reason |= NGX_HTTP_CACHEABLE_VARY;</span><br style="color:rgb(49,49,49);word-spacing:1px"><span style="word-spacing:1px;color:rgb(49,49,49)">> }</span><div dir="auto"><font style="color:rgb(49,49,49)"><span style="word-spacing:1px"><br></span></font></div><div dir="auto"><font style="color:rgb(49,49,49)"><span style="word-spacing:1px">> </span></font><span style="color:rgb(0,0,0)">I don't see any reasons to store information </span>about<span style="color:rgb(0,0,0)"> "Vary" header.</span></div><div dir="auto"><span style="color:rgb(0,0,0)"><br></span></div><div dir="auto"><font style="color:rgb(49,49,49)"><span style="word-spacing:1px"><div dir="auto">It should not cache only the Vary asterisk. it doesn't need to store this information so we consider that your approach (bitmask) is correct.<br></div><div dir="auto"><br></div><div dir="auto">> <span style="word-spacing:0px;color:rgb(0,0,0)">And we can call</span><span style="word-spacing:0px;color:rgb(0,0,0)"> </span></div><span style="word-spacing:0px;border-color:rgb(0,0,0) rgb(0,0,0) rgb(0,0,0) rgb(204,204,204);color:rgb(0,0,0)">> "ngx_http_upstream_cache_</span><span style="word-spacing:0px;border-color:rgb(0,0,0) rgb(0,0,0) rgb(0,0,0) rgb(204,204,204);color:rgb(0,0,0)">validate_regardless_order" only when cacheable is still</span><br style="color:rgb(0,0,0);font-family:-apple-system,HelveticaNeue;word-spacing:0px"><span style="word-spacing:0px;border-color:rgb(0,0,0) rgb(0,0,0) rgb(0,0,0) rgb(204,204,204);color:rgb(0,0,0)">> enabled after parsing all other headers.</span><div dir="auto"><br></div><div dir="auto">Our approach is a bit different, u->cacheable should be determined after completing all headers parsing. So we consider that it could be better not to set here and there during parsing headers particularly in case of u->cacheable reverts 1 to 0 if there is, it could be misread easily.<br></div></span></font></div></div><div style="background-color:rgba(0,0,0,0)!important;border-color:rgb(0,0,0)!important;color:rgb(0,0,0)!important"><div dir="auto" style="background-color:rgba(0,0,0,0)!important;border-color:rgb(0,0,0)!important;color:rgb(0,0,0)!important"><div dir="auto" style="background-color:rgba(0,0,0,0)!important;border-color:rgb(0,0,0)!important;color:rgb(0,0,0)!important"><div dir="auto"><div dir="auto"><br></div><div dir="auto">> <span>I will prepare an i</span><span style="border-color:rgb(0,0,0) rgb(0,0,0) rgb(0,0,0) rgb(204,204,204)">mproved version if you don't mind.</span></div></div><div style="background-color:rgba(0,0,0,0);border-color:rgb(0,0,0)" dir="auto"><font style="border-color:rgb(0,0,0) rgb(0,0,0) rgb(0,0,0) rgb(204,204,204);color:rgb(0,0,0)"><br></font></div><div style="background-color:rgba(0,0,0,0);border-color:rgb(0,0,0)" dir="auto"><font style="border-color:rgb(0,0,0) rgb(0,0,0) rgb(0,0,0) rgb(204,204,204);color:rgb(0,0,0)">Sure! Can we pick your brain for this issue?</font></div><div style="background-color:rgba(0,0,0,0);border-color:rgb(0,0,0)" dir="auto"><font style="border-color:rgb(0,0,0) rgb(0,0,0) rgb(0,0,0) rgb(204,204,204);color:rgb(0,0,0)"><br></font></div><div style="background-color:rgba(0,0,0,0);border-color:rgb(0,0,0)"><font style="border-color:rgb(0,0,0) rgb(0,0,0) rgb(0,0,0) rgb(204,204,204);color:rgb(0,0,0)">Thanks, <br>Yugo Horie</font></div><div style="background-color:rgba(0,0,0,0);border-color:rgb(0,0,0)" dir="auto"><font style="border-color:rgb(0,0,0) rgb(0,0,0) rgb(0,0,0) rgb(204,204,204);color:rgb(0,0,0)"><br></font><div dir="auto"><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 18, 2022 at 19:19 Vadim Fedorenko <<a href="mailto:vadim.fedorenko@cdnnow.ru" target="_blank">vadim.fedorenko@cdnnow.ru</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;padding-left:1ex;border-left-color:rgb(204,204,204)">Hi Yugo!<br>
<br>
Looks like another solution with the same concept - store additional information<br>
about several headers. The second patch you proposed looks a bit overcomplicated<br>
inside this additional function. I don't see any reasons to store information<br>
about "Vary" header. AFAIU, weed special information about 3 headers: "Expires",<br>
"Cache-Control" and "X-Accel-Expires". And we can call <br>
"ngx_http_upstream_cache_validate_regardless_order" only when cacheable is still<br>
enabled after parsing all other headers. That approach will simplify the logic<br>
inside ngx_http_upstream_cache_validate_regardless_order. I will prepare an<br>
improved version if you don't mind.<br>
<br>
Thanks,<br>
Vadim<br>
<br>
On 18.04.2022 02:14, Yugo Horie wrote:<br>
> We're also interested in this issue because the CDN provider like us is <br>
> concerned about the problems with the headers that determine the cache freshness.<br>
> It is often a serious problem used in Nginx as the origin server or the <br>
> intermediate cache server that has such problems.<br>
> In those days, we also proposed a patch to resolve it.<br>
> <a href="https://mailman.nginx.org/archives/list/nginx-devel@nginx.org/thread/ZQVPGEXAAHVL23TBBNMXLP4KDGDJCVTW/" rel="noreferrer" target="_blank">https://mailman.nginx.org/archives/list/nginx-devel@nginx.org/thread/ZQVPGEXAAHVL23TBBNMXLP4KDGDJCVTW/</a> <br>
> <<a href="https://mailman.nginx.org/archives/list/nginx-devel@nginx.org/thread/ZQVPGEXAAHVL23TBBNMXLP4KDGDJCVTW/" rel="noreferrer" target="_blank">https://mailman.nginx.org/archives/list/nginx-devel@nginx.org/thread/ZQVPGEXAAHVL23TBBNMXLP4KDGDJCVTW/</a>><br>
> <br>
> But it seems to have declined. We sincerely wish to resolve it and intend to do <br>
> anything.<br>
> <br>
> Thanks,<br>
> Yugo Horie<br>
> <br>
> 2022年4月18日(月) 4:51 Vadim Fedorenko via nginx-devel <<a href="mailto:nginx-devel@nginx.org" target="_blank">nginx-devel@nginx.org</a> <br>
> <mailto:<a href="mailto:nginx-devel@nginx.org" target="_blank">nginx-devel@nginx.org</a>>>:<br>
> <br>
> Hi Maxim!<br>
> <br>
> On 17.04.2022 02:55, Maxim Dounin wrote:<br>
> > Hello!<br>
> ><br>
> ><br>
> > [...]<br>
> ><br>
> > Thanks for the patch.<br>
> ><br>
> > I'm quite sceptical about attempts to fix this by introducing<br>
> > various flags and reverting cacheable status back to 1. This is<br>
> > not how it should be fixed.<br>
> ><br>
> Yeah, the solution might be a bit complicated, but I couldn't find<br>
> another way without breaking concept of independent header parsing.<br>
> Could you please suggest something if you think that current approach<br>
> is wrong? The ticket that this patch tries to fix is 6 years old and<br>
> still has discussions going on without any solution.<br>
> <br>
> Thanks,<br>
> Vadim<br>
> _______________________________________________<br>
> nginx-devel mailing list -- <a href="mailto:nginx-devel@nginx.org" target="_blank">nginx-devel@nginx.org</a> <mailto:<a href="mailto:nginx-devel@nginx.org" target="_blank">nginx-devel@nginx.org</a>><br>
> To unsubscribe send an email to <a href="mailto:nginx-devel-leave@nginx.org" target="_blank">nginx-devel-leave@nginx.org</a><br>
> <mailto:<a href="mailto:nginx-devel-leave@nginx.org" target="_blank">nginx-devel-leave@nginx.org</a>><br>
> <br>
</blockquote></div></div>
</div>
</div></div>
</div>
</div>