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

Access-Control-Expose-Headers: * can be interpreted in two ways #548

Closed
yutakahirano opened this issue May 24, 2017 · 15 comments
Closed

Access-Control-Expose-Headers: * can be interpreted in two ways #548

yutakahirano opened this issue May 24, 2017 · 15 comments

Comments

@yutakahirano
Copy link
Member

access-control-allow-origin = #field-name / wildcard

where

field-name = token
token = 1*tchar
tchar contains *
.

It means "*" can be interpreted in two ways.

  1. Allow all headers.
  2. Allow a header whose name is "*".
  1. Let headerNames be the result of extracting header list values given Access-Control-Expose-Headers and response’s header list.
  2. If headerNames is * and request’s credentials mode is not "include", then set response’s CORS-exposed header-name list to all unique header names in response’s header list.
  3. Otherwise, if headerNames is not null, failure, or *, then set response’s CORS-exposed header-name list to headerNames.

This sounds like

  1. if headerName is *, the symbol should be interpreted in the first way.
  2. if headerName contains * but headerName is not *, the symbol should be interpreted in the second way.

I feel it confusing.

@yutakahirano
Copy link
Member Author

@annevk @tyoshino

It was introduced in #252

@annevk
Copy link
Member

annevk commented May 24, 2017

I didn't realize you can have a header name that is *. If that is true I'm not sure how to best address this. Perhaps we need to use a different syntax for it.

@annevk
Copy link
Member

annevk commented May 24, 2017

It seems that if we want to avoid conflicts we should do something with

(DQUOTE and "(),/:;<=>?@[]{}").

per https://tools.ietf.org/html/rfc7230#section-3.2.6.

It seems this is a problem for:

  • Access-Control-Expose-Headers
  • Access-Control-Allow-Methods
  • Access-Control-Allow-Headers

but not Access-Control-Allow-Origin (origins cannot be *).

@mnot thoughts? Can we decide not to care about a method or header name which is *?

@tyoshino tyoshino changed the title access-conrol-allow-origin: * can be interpreted in two ways Access-Control-Expose-Headers: * can be interpreted in two ways May 25, 2017
@Mouvedia
Copy link

Mouvedia commented Jun 17, 2017

That would mean that the mere presence of a header field which name is * would prevent any other headers to be exposed unless you used a comma.

e.g. Access-Control-Expose-Headers: *, A, B would expose 3 headers.

@annevk
Copy link
Member

annevk commented Jun 26, 2017

Right, the question is whether that's acceptable or whether we should find an alternative syntax? And if the latter, what syntax?

@christophfriedrich
Copy link

In pursuit of meaningful, easy-to-understand syntax, I'd discourage finding an alternative syntax.

  1. The asterisk * is generally used as the wildcard character for "anything"
  2. Nobody seriously names their headers *

-> * should be interpreted as "all headers"
(if necessary, explicitly disallow a header named nothing but *)

Regards,
a dev stumbling across this

@mnot
Copy link
Member

mnot commented Jun 28, 2017

See httpwg/http-core#30.

@annevk
Copy link
Member

annevk commented Aug 25, 2017

Idea:

  1. We change the syntax and remove / wildcard. It's not needed as it's already part of field-name.
  2. For requests without credentials we interpret -Expose-Headers, -Allow-Methods, -Allow-Headers containing a wildcard as meaning ALL. For requests with credentials we interpret a wildcard as a literal, the name of a header or method.

So basically * gets special meaning for requests without credentials, which is what we wanted, but we don't go out of our way to make using * impossible.

The only scenario where you'd be out of luck is if for requests without credentials you only want to expose or allow a header/method named *.

The only alternative is using separators, such as @ or <*>, but the downside of that would be that it's inconsistent with -Allow-Origin, which already uses *.

@yutakahirano are you okay with the idea? Would be nice to unblock this and get this feature implemented.

@mnot
Copy link
Member

mnot commented Aug 29, 2017

I'd rather leave it as-is; it makes it clear that when it's alone, the wildcard has special meaning (i.e., the # rule isn't applied to it). As per the issue above, we might remove * from field-names in the future.

@yutakahirano
Copy link
Member Author

#548 (comment) sg
#548 (comment) is also acceptable but I would like some notes or examples.

@annevk
Copy link
Member

annevk commented Aug 30, 2017

I'm okay with making * special when it's the single value, but I will change the grammar for now. If field-name indeed changes we'll just fix our grammar at that point.

I'll start working on a PR to clarify the behavior as I think that will clarify any outstanding questions better than discussing it here would.

@annevk
Copy link
Member

annevk commented Aug 30, 2017

So while working on the PR I realized what @mnot suggested doesn't work as we need *, Authorization to mean ALL, including Authorization, and * to mean ALL, excluding Authorization. That's the status quo for Access-Control-Allow-Headers.

annevk added a commit that referenced this issue Aug 30, 2017
Since `*` is a valid header name and method, only count it as a special value for requests without credentials.

Fixes #548.
@annevk
Copy link
Member

annevk commented Aug 31, 2017

#592 is my newly proposed model. Basically using * never leads to an error as it's a valid value. And if the header takes comma-separated values than it's fine for * to just be one of the values per earlier comment.

As mentioned earlier the only limitation is that you cannot solely safelist a header name or method that is * for requests without credentials, but that's not a big deal as it's a rather questionable character for that purpose anyway.

@annevk
Copy link
Member

annevk commented Sep 1, 2017

Relevant Safari bug: https://bugs.webkit.org/show_bug.cgi?id=169194

annevk added a commit to web-platform-tests/wpt that referenced this issue Sep 1, 2017
annevk added a commit to web-platform-tests/wpt that referenced this issue Sep 1, 2017
annevk added a commit that referenced this issue Sep 4, 2017
Since `*` is a valid header name and method, only count it as a special value for requests without credentials. And no longer differentiate between `*` being a single value or one of many.

Tests: web-platform-tests/wpt#7223.

Fixes #548.
annevk added a commit that referenced this issue Sep 7, 2017
Since `*` is a valid header name and method, only count it as a special value for requests without credentials. And no longer differentiate between `*` being a single value or one of many.

Tests: web-platform-tests/wpt#7223.

Fixes #548.
annevk added a commit to web-platform-tests/wpt that referenced this issue Sep 7, 2017
rachelandrew pushed a commit to rachelandrew/web-platform-tests that referenced this issue Nov 8, 2017
jakearchibald pushed a commit to jakearchibald/web-platform-tests that referenced this issue Nov 16, 2017
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