> On Dec 11, 2020, at 6:28 AM, Ruediger Pluem <rpl...@apache.org> wrote: > On 12/11/20 1:13 PM, Yann Ylavic wrote: >> On Fri, Dec 11, 2020 at 12:49 PM Graham Leggett <minf...@sharp.fm> wrote: >>> >>> On 10 Dec 2020, at 18:04, yla...@apache.org wrote: >>> >>> Author: ylavic >>> Date: Thu Dec 10 16:04:34 2020 >>> New Revision: 1884280 >>> >>> URL: http://svn.apache.org/viewvc?rev=1884280&view=rev >>> Log: >>> Revert r1480058, -1'ed on dev@ and STATUS. >>> >>> Never backported (and never will supposedly), while often creating >>> merge conflicts. >>> >>> You’ve just reverted a fix to an RFC violation that was picked up by the >>> CoAdvisor test suite. >> >> Where is this test suite? >> Which RFC violation, a proxy socket connection error should return 504 >> Gateway Timeout?? >> I see that RFC2616 14.9.4 is about cache, why don't you fix this in >> mod_cache? >> >>> >>> “Creating merge conflicts” is not a sufficient technical justification for >>> a veto that results in the re-introduction of an RFC violation. >>> >>> Please back this out. >>> >>> See >>> https://lists.apache.org/thread.html/be0e7bdc3510fddd2dd80accece44917eba361ef4fcc713dd0f7f7fa%401367999236%40%3Cdev.httpd.apache.org%3E >>> and >>> https://lists.apache.org/thread.html/6e63271b308a2723285d288857318e7bb51b6756690514d9bc75a71b%401371148914%40%3Ccvs.httpd.apache.org%3E >>> >>> Please resolve the discussion above. > > Just as an update for restarting the discussion: > > Old RFC2616 Section New RFC and section Link > 14.9.4 RFC7234, 5.2.2.1 > https://tools.ietf.org/html/rfc7234#section-5.2.2 > <https://tools.ietf.org/html/rfc7234#section-5.2.2> > 10.5.3 RFC7231, 6.6.3 > https://tools.ietf.org/html/rfc7231#section-6.6.3 > <https://tools.ietf.org/html/rfc7231#section-6.6.3> > 10.5.5 RFC7231, 6.6.5 > https://tools.ietf.org/html/rfc7231#section-6.6.5 > <https://tools.ietf.org/html/rfc7231#section-6.6.5>
Heh, sorry, the current version is https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html#cache-response-directive.must-revalidate <https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html#cache-response-directive.must-revalidate> https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html#rfc.section.4.3.3 <https://httpwg.org/http-core/draft-ietf-httpbis-cache-latest.html#rfc.section.4.3.3> https://httpwg.org/http-core/draft-ietf-httpbis-semantics-latest.html#status.502 <https://httpwg.org/http-core/draft-ietf-httpbis-semantics-latest.html#status.502> and we have until Monday to fix it, if needed. > After rereading the sections on the new RFC's my opinion is still the same as > in > > https://lists.apache.org/thread.html/be0e7bdc3510fddd2dd80accece44917eba361ef4fcc713dd0f7f7fa%401367999236%40%3Cdev.httpd.apache.org%3E > > <https://lists.apache.org/thread.html/be0e7bdc3510fddd2dd80accece44917eba361ef4fcc713dd0f7f7fa@1367999236@%3Cdev.httpd.apache.org%3E> > > IMHO RFC7231, 6.6.3 and RFC7231, 6.6.5 apply for the proxy/gateway. In the > revalidate case (RFC7234, 5.2.2.1) the issue for the > cache may be even wider: What if the reply on the revalidation request from > the backend is not cachable for whatever reason (like > the 502 here without appropriate headers that allow caching)? > Does the cache just pass the reply on and does not update its cache? Does it > remove the stale entry from the cache? > > Apart from this the proxy could add a note to r->notes if there was a network > issue with the backend such that the cache can reply > with 504 in this case. But of course this only helps in the case that the > next hop was not reachable. If the 502 is created by a > further gateway between us and the origin server the issue remains. That is too many questions. The purpose of the cache requirement is so that the cache does not deliver a non-validated entry after receiving a failed validation. It doesn't really matter what code is returned so long as it is 5xx, so the specs are over-constraining here. The CoAdvisor test suite is overly pedantic. And, as stated, this only applies when the request contains max-revalidate and the action being done is a cache revalidation. No other code should behave this way. >> You should do that, it's not my veto. Failing to resolve the >> discussion, the commit should be reverted right? >> >>> >>> The last on the thread is that Roy was asked for advice, but he was busy. >>> In the light of RFC7230 it would be useful to get a new answer on this >>> question. >> >> Sure. > > Can you please help us resolving this Roy? Reverting the change is the correct call, regardless, but it is also the right choice. I have filed a bug on the Cache spec to change that MUST send 504 to a MUST send 5xx. https://github.com/httpwg/http-core/issues/608 <https://github.com/httpwg/http-core/issues/608> If you think we need to change other things in the specs, please file bugs now. ....Roy
