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

fix wrong explanation about cache-control and authroization header #29064

Merged
merged 3 commits into from
Sep 15, 2023

Conversation

Jxck
Copy link
Contributor

@Jxck Jxck commented Sep 12, 2023

Description

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

has incorrect explanation about cache-control and authorization header.

Motivation

in Spec (RFC 9111) only mentions that if request contains Authorization header, it can't be stored in Shared Cache by default.

https://httpwg.org/specs/rfc9111.html#response.cacheability:~:text=if%20the%20cache%20is%20shared%3A%20the%20authorization%20header%20field%20is%20not%20present%20in%20the%20request%20(see%20section%2011.6.2%20of%20%5Bhttp%5D)

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

But the content mentions like below.

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).

should remove private cache from here.

Related issues and pull requests

Fixes #29019

@Jxck Jxck requested a review from a team as a code owner September 12, 2023 07:02
@Jxck Jxck requested review from teoli2003 and removed request for a team September 12, 2023 07:02
@github-actions github-actions bot added the Content:HTTP HTTP docs label Sep 12, 2023
Co-authored-by: Donghun Shin <huipulci1@naver.com>
Copy link
Contributor

@shin-mallang shin-mallang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jxck
The changes are great.
But I think we've overlooked something.
The changes don't seem to have anything to do with private cache after all.
Is it right for this to be in the private cache paragraph?


(append)
And it seems to include similar content to this

The public value has the effect of making the response storable even if the Authorization header is present.
Note: The public directive should only be used if there is a need to store the response when the Authorization header is set. It is not required otherwise, because a response will be stored in the shared cache as long as max-age is given.
So if the response is personalized with basic authentication, the presence of public may cause problems. If you are concerned about that, you can choose the second-longest value, 38 (1 month).

(The above is what exists in the Cache Busting paragraph.)

@Jxck
Copy link
Contributor Author

Jxck commented Sep 13, 2023

hmm, so it seems reasonable to just remove.
@hamishwillee how do you think ?

@hamishwillee
Copy link
Collaborator

Yes this should be removed. Serves me right for not reading all the context. I have made that change. Thanks @shin-mallang for sticking with it, and @Jxck for providing the canonical answer.

@hamishwillee hamishwillee enabled auto-merge (squash) September 15, 2023 02:50
@Jxck
Copy link
Contributor Author

Jxck commented Sep 15, 2023

@hamishwillee @shin-mallang Thanks for tons of helps!

@shin-mallang
Copy link
Contributor

@hamishwillee @Jxck
I am honored😊
Thanks for your help.🙇🏻‍♂️

@hamishwillee hamishwillee merged commit 6eea9fd into mdn:main Sep 15, 2023
6 checks passed
mfuji09 pushed a commit to mfuji09/MDN-content that referenced this pull request Sep 20, 2023
…dn#29064)

* fix wrong explanation about cache-control and authroization header

* Update files/en-us/web/http/caching/index.md

Co-authored-by: Donghun Shin <huipulci1@naver.com>

* Remove line

---------

Co-authored-by: Donghun Shin <huipulci1@naver.com>
Co-authored-by: Hamish Willee <hamishwillee@gmail.com>
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 this pull request may close these issues.

Incorrect description of HTTP Private Cache
3 participants