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

Operate on internal response #23

Closed
annevk opened this issue Mar 13, 2015 · 8 comments
Closed

Operate on internal response #23

annevk opened this issue Mar 13, 2015 · 8 comments

Comments

@annevk
Copy link
Member

annevk commented Mar 13, 2015

When getting a response back from the service worker, need to operate on internal response.

@annevk
Copy link
Member Author

annevk commented Mar 16, 2015

The problem is that initially Fetch was a closed system and we'd only add a restriction on response on the way out. Service workers however allow injecting a response half-way in with a restriction in place.

So we should probably fix this by making everything aware of a response potentially being a filtered response. On the way out we then need to decide whether to switch filters or not.

@annevk
Copy link
Member Author

annevk commented Mar 16, 2015

In HTTP Fetch step 2.2 we check request's mode, we could also check request's response tainting. The difference would be that a opaque response to a same-origin (but no-cors) request would not be allowed whereas that is allowed today. Status quo is probably okay.

@annevk
Copy link
Member Author

annevk commented Mar 16, 2015

So for HTTP Fetch we need to acknowledge there's a response and potentially an internal response. If there's an internal response we want to check the internal response's status, headers, etc. in that algorithm.

For fetch step 11 I think we should basically skip responses that already have an internal response (i.e. that are filtered responses). Combined with HTTP Fetch step 2.2 that works fine I think.

@nikhilm @jakearchibald @wanderview am I missing anything?

@nikhilm
Copy link

nikhilm commented Mar 16, 2015

Are you saying step 11 can be skipped because the response returned from the SW will already be filtered due to SW being able to obtain a filtered response only from fetch() or cache.match(), both of which will 'do the right thing'? That sounds correct.

@annevk
Copy link
Member Author

annevk commented Mar 17, 2015

Yeah, that was my reasoning. Combined with the fact that we already disallow certain responses from the service worker (e.g. opaque response is not allowed if the request wasn't no-cors).

@annevk
Copy link
Member Author

annevk commented Mar 17, 2015

So I don't really see how a service worker can end up with a response that's opaque which HTTP fetch would need to worry about. The only opaque responses it can get are at the end of a redirect or authentication chain, no?

annevk added a commit that referenced this issue Mar 17, 2015
@annevk
Copy link
Member Author

annevk commented Mar 27, 2015

Closing this since I think I'm correct and I fixed the other issue.

@annevk annevk closed this as completed Mar 27, 2015
annevk added a commit that referenced this issue Jul 15, 2015
…, but didn't. Fixes #23 again (and better this time).
@annevk
Copy link
Member Author

annevk commented Jul 15, 2015

I should have worried about authentication at least. That works in a no-cors scenario. And soon redirects will be a tad more exposed too. I apparently also missed various cases in what is now "main fetch" but they should all be fixed now.

yutakahirano added a commit that referenced this issue Jun 23, 2020
# This is the 1st commit message:

# This is a combination of 23 commits.
# This is the 1st commit message:

Integrate CORP and COEP

This is part of the introduction of COEP
(whatwg/html#5454). The CORP check now takes
COEP into account. Also, responses coming from service workers
are checked.

# This is the commit message #2:

Update fetch.bs

Co-authored-by: Domenic Denicola <[email protected]>
# This is the commit message #3:

Update fetch.bs

Co-authored-by: Domenic Denicola <[email protected]>
# This is the commit message #4:

fix

# This is the commit message #5:

fix

# This is the commit message #6:

fix

# This is the commit message #7:

fix

# This is the commit message #8:

fix

# This is the commit message #9:

fix

# This is the commit message #10:

fix

# This is the commit message #11:

fix

# This is the commit message #12:

fix

# This is the commit message #13:

fix

# This is the commit message #14:

fix

# This is the commit message #15:

fix

# This is the commit message #16:

fix

# This is the commit message #17:

fix

# This is the commit message #18:

Update fetch.bs

Co-authored-by: Anne van Kesteren <[email protected]>
# This is the commit message #19:

Update fetch.bs

Co-authored-by: Anne van Kesteren <[email protected]>
# This is the commit message #20:

fix

# This is the commit message #21:

fix

# This is the commit message #22:

fix

# This is the commit message #23:

fix

# This is the commit message #2:

fix
This issue was closed.
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

2 participants