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

CORS protocol and HTTP caches #402

Closed
yutakahirano opened this issue Oct 25, 2016 · 12 comments
Closed

CORS protocol and HTTP caches #402

yutakahirano opened this issue Oct 25, 2016 · 12 comments

Comments

@yutakahirano
Copy link
Member

The section says

If CORS protocol requirements are more complicated than setting Access-Control-Allow-Origin to * or a static origin, Vary is to be used.

Is it saying that we need a Vary header if we want to add "Access-Control-Allow-Origin: *" to a response only when an Origin header is present on the request?

@annevk
Copy link
Member

annevk commented Oct 25, 2016

No, it's saying that if you want to use different values for ACAO than * or a fixed origin, you need to use Vary.

@yutakahirano
Copy link
Member Author

yutakahirano commented Oct 25, 2016

Thank you.

Let req1 and req2 be simple requests for the same url where req1's mode is no-cors and req2's mode is cors. Let res1 be the response for req1. res1 doesn't have "Access-Control-Allow-Origin: *" . Assume res1 is cacheable.

If (req1, res1) is cached, is the HTTP cache allowed to serve res1 for req2? That leads to a CORS failure, but I failed to find a description forbidding that.

@annevk
Copy link
Member

annevk commented Oct 25, 2016

That is #307 I think, unless you mean that credentials mode is identical?

If it is identical that would indeed be a problem of sorts with the server.

@yutakahirano
Copy link
Member Author

I thought about the CORS mode (request's mode), not about the credentials mode. The issue was reported at https://bugs.chromium.org/p/chromium/issues/detail?id=658575. Though the example at Comment 5 uses different credentials mode (no cors + include vs. cors + same-origin), using the same credentials mode (no cors + include vs. cors + include) results in the same behavior.

From https://bugzilla.mozilla.org/show_bug.cgi?id=1274555:

Note that it's impossible to have two entries for the same URL in the same load context info (same originattributes/anonymous flag/private load tuple)

If #307 means specifying the "load context info", that will resolve this issue as well (My understanding is it isn't specified currently. Please correct me if I'm wrong).

@annevk
Copy link
Member

annevk commented Oct 25, 2016

#307 is only about credentials at the moment. I'm not familiar with "load context info" or how that corresponds to Fetch's concepts. @mayhemer, could you maybe clarify?

@mayhemer
Copy link

I will ask for some clarifications too:

  • what is "the credentials mode"? does it mean to allow sending credentials and cookies with the request?

If so, then it's what we call "anonymous" mode (or flag) on a request.

To clarify "load context info": it's a set of flags and values that separate (identifies) the context where from the request is being made and separates cache entries in the http cache this way.

Load context info is carrying the following information:

  • origin attributes, which is a structure holding:
    • user context id (personal/work/backing/shopping...)
    • private browsing mode (aka incognito)
    • few others, not related to this bug
  • anonymous load (aka 'no credentials')

An entry in the http cache is identified by: [load context info][URL w/o hash] tuple.

Re #402 (comment):

If (req1, res1) is cached, is the HTTP cache allowed to serve res1 for req2?

Yes, if there is nothing more said about the cachability of the response. Only solution is the server side as mentioned in https://bugs.chromium.org/p/chromium/issues/detail?id=658575#c5.

What you suggest sounds like we may want to isolate the cache by a CORS flag (add it to the load context info).

The result would be: let's have pages: foo.com/, bar.com/, baz.com/, all referring a resources at foo.com/script.js. In the http cache there would then be exactly two entries:

  • foo.com/script.js as referenced from foo.com/
  • foo.com/script.js as referenced from bar.com/ and baz.com/ (and whatever origin not being foo.com)

Tho, I believe this could well break the web. Hence there is the requirement (suggestion?) to use the 'Vary: origin' response header which can be managed by the web admins. OTOH, this puts the CORS protection on their shoulders. Maybe the browser should help here somehow?

@annevk
Copy link
Member

annevk commented Nov 1, 2016

does it mean to allow sending credentials and cookies with the request?

Yeah. So I think varying on that for the cache is still sufficient as only without that do you get an actual security issue. And recommending Vary for the remainder. Thanks.

@yutakahirano
Copy link
Member Author

Thank you for the reply.

I will ask for some clarifications too:

what is "the credentials mode"? does it mean to allow sending credentials and cookies with the request?

I meant the mode specified in the spec.

Back to the original question, the conclusion is that we recommend Vary: Origin in that case, right? Then making it a bit explicit in the note is helpful, I think.

@annevk
Copy link
Member

annevk commented Nov 1, 2016

@yutakahirano the original question is about a response setting the Access-Control-Allow-Origin: * header conditionally based upon the presence of the Origin header, right? That does not seem like a sensible setup and we should maybe recommend against it in that section, but as I tried to explain before the note that is currently there is about something else.

@yutakahirano
Copy link
Member Author

I see, then let's close this issue.

@dotnetCarpenter
Copy link

FYI The exact use case explained in comment 3 describe the default Amazon AWS configuration.

Ref:https://bugs.chromium.org/p/chromium/issues/detail?id=809891

@ziyunfei
Copy link

ziyunfei commented Jun 24, 2018

There's another bug report: https://bugs.chromium.org/p/chromium/issues/detail?id=855713

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants