Incorrect min-fresh? about cacherules

[aw]

OK I ran some tests, and I think you are correct. There's an error in the calculation. In fact, it should be:

freshness_lifetime.to_i >= (current_age - token.to_i)

I tested with this change and it seems to behave correctly. Essentially, if you supply the min-fresh Cache-Control directive, an expired response should be given more time, but as it is now, those seconds are added to the expiry time, thus making it even more expired haha - oops!

Nice catch @SleeplessByte, I'll push an update shortly after adjusting the tests.

[aw]

Ohhhh!! I see what you mean. I completely mis-interpreted the meaning of the following RFC 7234

"the client wants a response that will still be fresh for at least the specified number of seconds."

I somehow totally missed that part. OK let me review my code and fix it correctly. Sorry about that.

[aw]

@SleeplessByte thanks for looking into this. It seems revalidation is the way to go. I'll try out your suggestion.

[SleeplessByte]

Little more info: I think this would show up as "freshness expired + allow stale" and then STALE according to the table you have posted, if min-fresh can't be satisfied, where in fact it probably should give either MISS (it can't give a response that satisfied min-fresh, so the cache is not expired, but missed) or EXPIRED (but that's weird, because the request shouldn't change the "expiration" of the cache, just what it's willing to return).

[aw]

Hi @SleeplessByte, thanks for the message. This code is not fresh in my mind, so i'll have a look and reply once I figure it out.

[SleeplessByte]

Actually, an expired response should be MORE expired.

min-fresh=10 can not serve a request that will be stale within ten seconds. So if you translate that, the expiration comes sooner if you give a min-fresh (or you add it to the age).

Regardless, it should be removed from the validate allow stale. min-fresh only indicates if a request may be returned, it does not allow for staleness.

[SleeplessByte]

Given a cached response with
Date: Fri, 13 Jul 2018 16:40:00 +0000 (Unix 1531500000)
Expires: Fri, 13 Jul 2018 16:45:00 +0000 (Unix 1531500300)

> Date: Fri, 13 Jul 2018 16:43:00 +0000
> Cache-Control: min-fresh=60

< HTTP 200
< X-Cache: HIT

In this case we can calculate the age as 3 minutes old, we want it to be fresh for at least another minute, and this would be fresh for another 2 minutes.

> Date: Fri, 13 Jul 2018 16:44:30 +0000
> Cache-Control: min-fresh=60

< HTTP 307
< X-Cache: MISS / EXPIRED

In this case the cached value has not expired (it's still fresh) but it can't be served because we want it to be fresh for another minute, this would be for another 30 seconds. Note: I don't know if you would call this EXPIRED, but it's definitely not HIT nor STALE.

> Date: Fri, 13 Jul 2018 16:44:30 +0000
> Cache-Control: min-fresh=60, max-stale=60

< HTTP 307
< X-Cache: MISS / EXPIRED

I couldn't actually find this combination in the message boards (and I've searched for at least an hour), but it seems that in ALL CASES min-fresh takes precedence. It doesn't matter how stale you "accept" the response, it MUST be at least this fresh, so will always be a MISS

[SleeplessByte]

Haha I know what you mean! @aw. I am trying to build a similar lib in Kotlin, using your work as guidance, and I had to re-read a lot of the RFC multiple times to make sure I understood it. Read it wrongly the first time as well :)

[SleeplessByte]

Found it:

https://github.com/aw/CacheRules/blob/master/lib/validations.rb#L95

If you implement it here, I think you're spec-compliant:

if request has min-fresh
   if freshness.currentAge > freshness.lifetime - min-fresh value
     return 1
   end
end

[aw]

Oh man.. i'm racking my brain on this. Yes I've also spent countless hours pouring over these RFCs. They're too vague.

If moving the min-fresh validation there, it would force a revalidation in that scenario.. I'm not sure if that's correct, I need to review that behaviour

[SleeplessByte]

Got it; I don't know if that's intended; Let me see what the browser does / CloudFlare does.

[SleeplessByte]

As I find information I'll post it here, if you don't mind.

https://github.com/Kevinrob/guzzle-cache-middleware/blob/6952064f7747756b0be7b4c234c0fd7535ea4c8c/src/CacheMiddleware.php#L144-L157

They implement this as: "If we can find something fresh, it's a hit", if not, ignore. Probably not the right way.

[SleeplessByte]

Chromium actually has tests around this case: https://github.com/chromium/chromium/blob/03f9320605ad419e8ddfcfad0c6436b34e527382/third_party/WebKit/LayoutTests/external/wpt/fetch/http-cache/cc-request.html#L99

It says: "not_cached" if it's not fresh enough, and thus you must revalidate.

[SleeplessByte]

Firefox has similar tests:
https://dxr.mozilla.org/mozilla-central/source/netwerk/test/unit/test_cache-control_request.js#324-328

It asserts "hit the server", as in a revalidation.

[SleeplessByte]

That seems right, now; I'll close and we can re-open should we find more issues :)