Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect description of HTTP Private Cache #29019

Closed
shin-mallang opened this issue Sep 9, 2023 · 16 comments · Fixed by #29064
Closed

Incorrect description of HTTP Private Cache #29019

shin-mallang opened this issue Sep 9, 2023 · 16 comments · Fixed by #29064
Assignees
Labels
Content:HTTP HTTP docs

Comments

@shin-mallang
Copy link
Contributor

shin-mallang commented Sep 9, 2023

MDN URL

https://developer.mozilla.org/en-US/docs/Web/HTTP/Caching#private_caches

What specific section or headline is this issue about?

private_caches

What information was incorrect, unhelpful, or incomplete?

The following exists in the documentation

Note that if the response has an Authorization header, it cannot be stored in the private cache (or a shared cache, unless public is specified).

However, it didn't say anything about private cache and Authorization headers in RFC 9111.
Here's what RFC 9111 says.

A cache MUST NOT store a response to a request unless:

What did you expect to see?

I think this should be left out.

Note that if the request has an Authorization header, it cannot be stored in the a shared cache, unless public is specified.

Am I misunderstanding RFC 9111?

Do you have any supporting links, references, or citations?

No response

Do you have anything more you want to share?

No response

@shin-mallang shin-mallang added the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Sep 9, 2023
@github-actions github-actions bot added the Content:HTTP HTTP docs label Sep 9, 2023
@hamishwillee
Copy link
Collaborator

Hmmm, I thought Authorization was a request header, so either way this "reads wrong". @teoli2003 thoughts on this?

@shin-mallang
Copy link
Contributor Author

shin-mallang commented Sep 11, 2023

@hamishwillee

I think I may have misread that a bit, but as you say, I think this is still wrong.

As describe below, shared cache for request with Authorization header can be stored.

A cache MUST NOT store a response to a request unless:

  • if the cache is shared: the Authorization header field is not present in the request (see Section 11.6.2 of [HTTP])

So I think you're right, the authorization is the request header.
The bottom line is that RFC9111 says anything about response with an authorization header and a private cache, so I think that's wrong.


So the content should be changed to

Note that if the 'request' has an Authorization header, it cannot be stored in the a shared cache, unless public is specified.

The above is corrected by

5.2.2.9. public
The public response directive indicates that a cache MAY store the response even if it would otherwise be prohibited, subject to the constraints defined in Section 3. In other words, public explicitly marks the response as cacheable. For example, public permits a shared cache to reuse a response to a request containing an Authorization header field (Section 3.5).

If there are no issues after checking, can I send a PR on this?

@hamishwillee
Copy link
Collaborator

@shin-mallang Sure, once we know for sure a fix would be welcome. @teoli2003 is a resident expert, and we can also get advice from another person if he doesn't know.

To me the text seems likely to be incorrect. If information is protected you don't want it stored in a shared cache, where it might be visible to users that did not originally have permission, unless it is explicitly marked as public. But a private cache should not have these risks. But you never know what we might be missing.

Might take a few days for people to respond.

@shin-mallang
Copy link
Contributor Author

@hamishwillee

Thanks for the answer

@hamishwillee
Copy link
Collaborator

@Jxck I was wondering if you could look at this and confirm if you are allowed to cache the result of an Authorization request in a private cache? The docs say you can't, but there is nothing obvious in the spec to reflect that requirement (logically the concern of leaking authorized content should not apply in a private cache right?)

@Jxck
Copy link
Contributor

Jxck commented Sep 12, 2023

Hmm it seems wrong, and not my original intention.
Change happed in #17214
We better revert it and re-modify previous sentence easy to understand.

@shin-mallang
Copy link
Contributor Author

shin-mallang commented Sep 12, 2023

@hamishwillee @Jxck
I have a question.
I'm a student, so I apologise in advance for the low quality of my question.

I'm just wondering if I'm understanding this situation correctly.

Firstly, what was written in the docs was about response with an Authentication header and a private-cache, but that seems to be against the intent, and the original intent was a request with an Authentication header and a private-cache.
Is this correct?


If the above is correct, then my second question is, what is the statement about the private cache for requests with the Authentication header based on?
(If my previous assumption is wrong, please translate the request into a response).
I couldn't find anything about this in the spec.

My understanding is that because it's a private cache, it can cache requests that include the Authentication header.
Are you writing that this is a security issue and therefore cannot be cached?
If so, can you tell me what security issues this might cause?


Or did you have some sort of rationale for saying it can't be cached?

Whatever it is, if it's not stated in the spec, I think it should be that it's not recommended to cache it, rather than that it can't be cached.

@hamishwillee
Copy link
Collaborator

@shin-mallang What @Jxck is saying is that the current text looks wrong.

I injected it as part of #17214 when trying to clean up some confusing text (which I had forgotten). The text before was also probably not quite right. He is suggesting that we change this back to saying what it did in #17214 and after that try to make it readable again, without injecting the incorrect statement.

I will have a look at this, probably on Friday.

@shin-mallang
Copy link
Contributor Author

@hamishwillee
Ah I understand what @Jxck said.
I'm just asking if my understanding is correct🙂

@hamishwillee
Copy link
Collaborator

hamishwillee commented Sep 12, 2023

So @Jxck The problematic change here was:

From

If the response has an Authorization header, it cannot be stored in either the private cache or the shared cache, even if it has max-age — and public can be used to allow such a case. But if you are not using basic authentication and do not have an Authorization header, then there is no need to add public; it's just a waste of bytes in that case.

TO

Note that if the response has an Authorization header, it cannot be stored in the private cache (or a shared cache, unless public is specified).

What did you intend to say in the original text - there is no point restoring the original because that also has the bit we are not sure about. Noting that a response cannot have an Authorization header, so the sentence would have to start:

The response to an Authorization header cannot be stored in an Xxxx cache ...

@shin-mallang
Copy link
Contributor Author

shin-mallang commented Sep 12, 2023

@hamishwillee

What did you intend to say in the original text - there is no point restoring the original because that also has the bit we are not sure about. Noting that a response cannot have an Authorization header, so the sentence would have to start:
The response to an Authorization header cannot be stored in an Xxxx cache ...

Thank you for your reply. 🙇‍♂️
That clears up my questions.

@Jxck
Copy link
Contributor

Jxck commented Sep 12, 2023

@shin-mallang

Firstly, what was written in the docs was about response with an Authentication header and a private-cache, but that seems to be against the intent, and the original intent was a request with an Authentication header and a private-cache.
Is this correct?

Yes and No

We need to mention about Authorization (not Authentication header) header is in request and behavior of Shared-Cache. Not an Authorization in response and Private-Cache.

If the above is correct, then my second question is, what is the statement about the private cache for requests with the Authentication header based on?

I also confused why it happen. My intention & what described in spec is how Authorization header affects shared cache behavior.

My understanding is that because it's a private cache, it can cache requests that include the Authentication header.

Yes, I also believe Authorized response (corresponding response for request with Authorization header) can be store in private-cache.

@hamishwillee

I found previous sentence also require improve.
Not a revert but a re-write could be fit here.
I’ll PR to fix them soon.

@shin-mallang
Copy link
Contributor Author

@Jxck
Thank you so much 🙇‍♂️🙇‍♂️🙇‍♂️

@shin-mallang shin-mallang changed the title There's something I don't understand about HTTP Private Cache. Incorrect description of HTTP Private Cache. Sep 12, 2023
@shin-mallang shin-mallang changed the title Incorrect description of HTTP Private Cache. Incorrect description of HTTP Private Cache Sep 12, 2023
@Jxck
Copy link
Contributor

Jxck commented Sep 12, 2023

@shin-mallang @hamishwillee done. only removing mentioning to "private cache" seems fine.
Please check it.

@hamishwillee
Copy link
Collaborator

Awesome. Thank you. Comment added.

@shin-mallang
Copy link
Contributor Author

Checked.
Thank you.

@Josh-Cena Josh-Cena removed the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTTP HTTP docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants