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

Change how CORS filtered response filters the headers. #265

Conversation

mkruisselbrink
Copy link
Collaborator

Rather than just directling taking the exposed headers from the
Access-Control-Expose-Headers header, this adds a separate list of
exposed headers to a response, initialized from that header. This makes
it possible for foreign fetch to filter headers differently.

This will be used by foreign fetch in mkruisselbrink/ServiceWorker@bf4585b

Rather than just directling taking the exposed headers from the
`Access-Control-Expose-Headers` header, this adds a separate list of
exposed headers to a response, initialized from that header. This makes
it possible for foreign fetch to filter headers differently.
<p>A <dfn>CORS-safelisted response-header name</dfn>, given a
<span title=concept-response-header-list>header list</span> <var>list</var>, is a
<span title=concept-header>header</span> <span title=concept-header-name>name</span> that is one of:
<p>A <dfn id="cors-safelisted-response-header-name">CORS-safelisted response-header name</dfn>, given a
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add an id here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, user error. Somehow managed to copy generated html into the source file... Fixed.

to the result of <span title=concept-header-parse>parsing</span>
`<code title=http-access-control-expose-headers>Access-Control-Expose-Headers</code>` in
<var>response</var>'s
<span title=concept-response-header-list>header list</span>.
Copy link
Member

Choose a reason for hiding this comment

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

Although it does not matter in a black-box-observable way, I still think we should add a conditional here for response tainting being "cors".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@annevk
Copy link
Member

annevk commented Mar 30, 2016

This looks pretty good. It could use some rewrapping here and there to account for 100 column lines (while keeping inline elements on one line at all cost, even if it requires exceeding the limit after putting them on their own line), but I'm happy to do that for you.

@mkruisselbrink
Copy link
Collaborator Author

This looks pretty good. It could use some rewrapping here and there to account for 100 column lines (while keeping inline elements on one line at all cost, even if it requires exceeding the limit after putting them on their own line), but I'm happy to do that for you.

Thanks. I tried to do wrapping that seemed to match most of the rest of the document, but yeah, it's certainly possible that I missed something/wrapped something differently than you'd like.

annevk pushed a commit that referenced this pull request Mar 31, 2016
Rather than just directling taking the exposed headers from the
`Access-Control-Expose-Headers` header, this adds a separate list of
exposed headers to a response, initialized from that header. This makes
foreign fetch integration easier.

PR: #265
@annevk
Copy link
Member

annevk commented Mar 31, 2016

Thank you, merged as 32411c7.

@annevk annevk closed this Mar 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants