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

Block 206s entering the cache (via put & addAll) #937

Closed
jakearchibald opened this Issue Jul 29, 2016 · 7 comments

Comments

Projects
None yet
5 participants
@jakearchibald
Copy link
Collaborator

jakearchibald commented Jul 29, 2016

Right now it seems like this would only happen by accident (passthrough caching), and it's something we can open up later if we need to.

@jakearchibald jakearchibald added this to the Version 1 milestone Jul 29, 2016

@wanderview

This comment has been minimized.

Copy link
Member

wanderview commented Aug 1, 2016

@inexorabletash

This comment has been minimized.

dstockwell pushed a commit to dstockwell/chromium that referenced this issue Aug 2, 2016

Cache API should reject 206 Partial Responses
Per spec issue[1], the cache API should reject partial responses,
failing calls to put()/add()/addAll() that come back with an HTTP
status code 206.

[1] w3c/ServiceWorker#937

R=jkarlin@chromium.org,cmumford@chromium.org
BUG=633338

Review-Url: https://codereview.chromium.org/2204573002
Cr-Commit-Position: refs/heads/master@{#409314}
@ricea

This comment has been minimized.

Copy link
Contributor

ricea commented Sep 29, 2016

Shouldn't 416 "Requested Range Not Satisfiable." also be blocked?

jungkees pushed a commit that referenced this issue Oct 17, 2017

Jungkee Song
Block partial responses entering the cache
This change adds a guard to put() and addAll() that blocks 206 responses
entering the cache.

Fixes #937.
@jungkees

This comment has been minimized.

Copy link
Collaborator

jungkees commented Oct 17, 2017

Just made a PR for this.

I didn't include 416 in the PR. Currently, addAll() filters out 416 while put() doesn't. Should we not allow 416 in put()?

/cc @jakearchibald, @wanderview

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Nov 16, 2017

I think 416 is fine for the same reasons 404 is fine. The danger of 206 is it's an 'ok' response that likely only represents part of what the developer expects.

Also, by not-allowing 206 we can potentially add range coalescing behaviour in future.

@jungkees

This comment has been minimized.

Copy link
Collaborator

jungkees commented Nov 16, 2017

Sounds good. I'll merge #1210.

@jungkees

This comment has been minimized.

jungkees added a commit that referenced this issue Nov 16, 2017

Block partial responses entering the cache (#1210)
This change adds a guard to put() and addAll() that blocks 206 responses
entering the cache.

Fixes #937.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment