[PATCH] Upstream: prioritise Cache-Control over Expires

[Maxim Dounin]

Hello!

On Tue, Apr 19, 2022 at 10:59:12PM +0100, Vadim Fedorenko wrote:

> On 19.04.2022 16:01, Maxim Dounin wrote:
> > 
> > On Sun, Apr 17, 2022 at 08:50:25PM +0100, Vadim Fedorenko via nginx-devel wrote:
> > 
> >> On 17.04.2022 02:55, Maxim Dounin wrote:
> >>
> >>> I'm quite sceptical about attempts to fix this by introducing
> >>> various flags and reverting cacheable status back to 1.  This is
> >>> not how it should be fixed.
> >>
> >> Yeah, the solution might be a bit complicated, but I couldn't find
> >> another way without breaking concept of independent header parsing.
> >> Could you please suggest something if you think that current approach
> >> is wrong? The ticket that this patch tries to fix is 6 years old and
> >> still has discussions going on without any solution.
> > 
> > That's basically why the ticket is still not fixed: there is no
> > easy fix, and the issue is rather minor, especially compared to
> > the complexity of the changes required to fix it.
> > 
> Hmm.. I would argue on the minority of the issue, especially when we talk about 
> absence of compatibility with RFC and actual discussions in the ticket. Anyway.

While current behaviour might be suboptimal in some use cases, it 
does not violate RFC, as caching is not something required to 
happen.  So we are talking about optimizing some use cases here, 
and the same optimization can be easily obtained by adjusting 
headers in the backend response.

> > IMHO, the most promising approach would be to move
> > Cache-Control/Exires/X-Accel-Expires handling elsewhere, and
> > handle them somewhere in ngx_http_upstream_process_headers(),
> > probably along with ngx_http_file_cache_valid() as well.
> > 
> Ok, cool, I updated my patch, now it's based on the work from Yugo that was left 
> without comments and is following the way you suggest. Take a look at v2, please.

Apart multiple style issues, the patch fails to address the 
X-Accel-Expires vs. Cache-Control order issue as outlined in the 
ticket, that is, the response with:

: X-Accel-Expires: 10
: Cache-Control: max-age=100, stale-while-revalidate=1000

results in different behaviour than the one with:

: Cache-Control: max-age=100, stale-while-revalidate=1000
: X-Accel-Expires: 10

It is also seems to be completely unreadable.  (Yugo's patch tried 
to address this, though had multiple style and readability issues 
as well.)

-- 
Maxim Dounin
http://mdounin.ru/