Subject: Artart telechat review of draft-ietf-httpbis-cache-header-09

From: Martin Dürst via Datatracker <[email-masked]>
Date: Tue, 10 Aug 2021 01:07:11 -0700

Reviewer: Martin Dürst
Review result: Ready with Issues

Reviewer: Martin J. Dürst
Review result: Ready with Nits

This document is mostly ready, but a few places would benefit from
updated/clarified wording.

Overall: The draft says the header's purpose is "to add debugging". Is the
intent that this header is consumed by debugging tools, or is it simply
intended for human debuggers? If the former, that should be called out more
clearly because it might help implementing senders to be more careful. If the
later, there's a chance that implementations will degrade over time, because
humans are the ultimate example for the second half of Jon Postel's robustness
principle. Also, it would be interesting to know if other uses besides
debugging are possible.

Section 2, first paragraph: The sentence is grammatically correct, but avoiding
"caches'" and the final "within" would definitely make it more readable. E.g.:
"The Cache-Status HTTP response header field indicates how the caches have
handled the request corresponding to the response where the header field
occurs.".

Section 2, second paragraph: "Its value is a List ([RFC8941], Section 3.1):":
RFC 8941 is just referenced in passing. If the header field is using the syntax
from RFC 8941, that should be said independently up front. If only parts of
that syntax are used, that should also be said explicitly.

Section 2, ~forth paragraph (fifth by different counting): This paragraph, and
in particular its first sentence, have left me wondering about its exact
meaning repeatedly. When the draft says "The Cache-Status header field is only
applicable to responses that have been generated by an origin server.", is that
another way of saying that the server  (which may be a cache, but not for the
response in question) originally creating a response SHOULD NOT add such a
header field to that response? The problem with the current language is that in
my understanding, essentially all responses at one point are generated by an
origin server, and so the quoted sentence doesn't in any way restrict anything.
Or is the header also inappropriate for the case when a cache serves a full
fresh response as originally received from the origin server, with 200 OK?
Wouldn't that defy the purpose of this header field?

Section 2.4, first paragraph: "measured when the response header section is
sent by the cache": This may be splitting hairs, but some header sections are
quite large and may not be sent in one go, and on the other hand, generating a
header field and sending it may not happen exactly synchronously, in which case
it would be easy to measure and note down the ttl when generating, but
difficult to do so when sending.

Section 3, last example: There is only one example with two layers of caching.
One or more additional examples of multi-layer caching might greatly enhance
understanding for example-directed readers.

---

From: Mark Nottingham <[email-masked]>
Date: Tue, 17 Aug 2021 11:35:04 +1000

Hi Martin,

Thanks for the review. Responses below.

> On 10 Aug 2021, at 6:07 pm, Martin Dürst via Datatracker <[email-masked]> wrote:

> principle. Also, it would be interesting to know if other uses besides

Potentially both, although it's hard to predict the future. I've written tools in the (distant) past to consume this sort of information from proxy logs, for example. It remains to be seen whether it'll be consumed by tools from headers.

That said, I'm not convinced that adding a phrase like 'including by automated tools' will result in more careful implementation; what's much more likely to result in that is the existence of such tools themselves. In the spec, we've already carefully outlined the serialisation rules (through use of structured fields) in an effort to improve interoperability.

> handled the request corresponding to the response where the header field

Already addressed when handling previous feedback.

> that syntax are used, that should also be said explicitly.

It's not conformant to use 'only parts' of SF syntax; as per the spec, you either use it for a field or you don't. If that isn't clear, can you make a suggestion? E.g.,

Its value is a Structured Field List (...)

?

> Wouldn't that defy the purpose of this header field?

That paragraph has also been substantially reworked based upon previous feedback. See:
  https://tools.ietf.org/rfcdiff?url1=https://tools.ietf.org/id/draft-ietf-httpbis-cache-header.txt&url2=https://httpwg.github.io/http-extensions/draft-ietf-httpbis-cache-header.txt

> difficult to do so when sending.

Yes, this is a thorny area. We were trying to make it reasonably precise without constraining implementation behaviour too much. Do you have a suggestion?

> understanding for example-directed readers.

See:
  https://github.com/httpwg/http-extensions/commit/62d77c9fc

Cheers,

--
Mark Nottingham   https://www.mnot.net/

---

From: Martin J. Dürst <[email-masked]>
Date: Tue, 17 Aug 2021 17:40:19 +0900

Hello Mark,

> That said, I'm not convinced that adding a phrase like 'including by automated tools' will result in more careful implementation; what's much more likely to result in that is the existence of such…

I agree that actual tools are a stronger factor than just some language.
But "to add debugging" may invoke different images with different 
people, and so I think it would be good to make sure that spec readers 
understand that it's both. So why not add something like
"both by humans and automated tools"?

> Already addressed when handling previous feedback.

Thanks.

> It's not conformant to use 'only parts' of SF syntax; as per the spec, you either use it for a field or you don't.

As one of the authors of that spec, you obviously know that, but others 
may not.

> If that isn't clear, can you make a suggestion? E.g.,
> 

As I said, it should be said up front (before the detail about List) 
that the syntax from [RFC8941] is used.

So e.g.:

    The Cache-Status HTTP response header field indicates caches'
    handling of the request corresponding to the response it occurs
    within. The syntax of this header field conforms to [RFC8941].

    Its value is a List ([RFC8941], Section 3.1):

    Cache-Status   = sf-list

>    https://tools.ietf.org/rfcdiff?url1=https://tools.ietf.org/id/draft-ietf-httpbis-cache-header.txt&url2=https://httpwg.github.io/http-extensions/draft-ietf-httpbis-cache-header.txt

Thanks, I think this is quite a bit clearer now.

> Yes, this is a thorny area. We were trying to make it reasonably precise without constraining implementation behaviour too much. Do you have a suggestion?

Maybe write something like "measured as close as possible to when the 
response header section is sent by the cache" or so, to indicate that 
this only expects best effort by the implementations.

>    https://github.com/httpwg/http-extensions/commit/62d77c9fc

Thanks! In terms of content, that's a great example (three layers, 
detailed explanations). But we suddenly have three header fields rather 
than just one. If I understand correctly, that example could also be written

Cache-Status: ReverseProxyCache; hit,	
    ForwardProxyCache; fwd=uri-miss; collapsed; stored,	
    BrowserCache; fwd=uri-miss

As far as I remember, there isn't any text that discusses trade-offs or 
recommends one or the other or says they are equivalent. But maybe I 
missed something?
  	
Regards,   Martin.

---

From: Mark Nottingham <[email-masked]>
Date: Wed, 18 Aug 2021 11:39:44 +1000

Hi Martin,

I've addressed all of these in the latest commits on:
  https://github.com/httpwg/http-extensions/commits/main/draft-ietf-httpbis-cache-header.md

... except for:

> Thanks! In terms of content, that's a great example (three layers, detailed explanations). But we suddenly have three header fields rather than just one. If I understand correctly, that example could also be written
> 
> As far as I remember, there isn't any text that discusses trade-offs or recommends one or the other or says they are equivalent. But maybe I missed something?

That's because this is the case for all HTTP headers, not just this one. I'm extremely reluctant to re-specify or explain how HTTP works in any great detail in this specification.

Cheers,

---

From: Martin J. Dürst <[email-masked]>
Date: Wed, 18 Aug 2021 10:51:02 +0900

Hello Mark,

>    https://github.com/httpwg/http-extensions/commits/main/draft-ietf-httpbis-cache-header.md

Thanks, great!

> That's because this is the case for all HTTP headers, not just this one. I'm extremely reluctant to re-specify or explain how HTTP works in any great detail in this specification.

Sorry, reading too many mail-related discussions lately.

Regards,   Martin.