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

should Cache API Vary matching use combined header values? #991

Closed
wanderview opened this issue Oct 11, 2016 · 6 comments
Closed

should Cache API Vary matching use combined header values? #991

wanderview opened this issue Oct 11, 2016 · 6 comments

Comments

@wanderview
Copy link
Member

At some point the fetch spec was changed such that Headers.get() now returns combined values. The SW spec was using Headers.get() for Vary header matching previously and I don't think it was updated to account for this change.

What is the intention here? Should Cache API vary matching be performed on combined values (new fetch get()) or first header value (old fetch get())?

@jakearchibald @annevk @inexorabletash

@annevk
Copy link
Member

annevk commented Oct 11, 2016

Wait, why did Cache API use headers.get()? Did it actually use the JavaScript call? That seems almost certainly broken. And even if not, not considering all headers seems to break with HTTP, no?

@wanderview
Copy link
Member Author

If http cache considers all headers, then I think the spec is mostly correct now.

In regards to calling webidl calls, it seems fetch spec does not expose algorithms that can be used instead?

@annevk
Copy link
Member

annevk commented Oct 12, 2016

I'd expect the Cache API to just operate on the data structure directly, as it does earlier on Query Cache as well, e.g., "If cachedResponse’s response’s header list contains no header named Vary".

@annevk
Copy link
Member

annevk commented Oct 12, 2016

We can certainly add algorithms if Cache API needs particular abstractions that are likely reused elsewhere though.

@annevk
Copy link
Member

annevk commented Oct 25, 2021

I think this can be closed with the remaining issue dealt with in #1609. @wanderview what do you think?

@wanderview
Copy link
Member Author

SGTM.

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

No branches or pull requests

3 participants