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

Fix HTTP Caching of Public Goods responses (remove Vary: Origin) #7

Open
1 of 2 tasks
lidel opened this issue Feb 2, 2024 · 2 comments
Open
1 of 2 tasks

Fix HTTP Caching of Public Goods responses (remove Vary: Origin) #7

lidel opened this issue Feb 2, 2024 · 2 comments
Assignees

Comments

@lidel
Copy link
Contributor

lidel commented Feb 2, 2024

Right now, service at delegated-ipfs.dev/routing/v1 sends CORS header only when Origin header is present in the request, and when Origin is present, the response has Vary: Origin:

$ curl -H "Origin: https://example1.com" https://delegated-ipfs.dev/routing/v1/providers/bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi -s -i | grep -i origin
access-control-allow-origin: *
vary: Origin

Problem

If the Vary header in response is set to value Origin, it indicates that the response may vary depending on the value of the Origin header in the request.

It means the response is reusable (cacheable) only as long value in Origin header matches, so responses for requests made from different websites (origins) won't benefit from caching.

IIUC this makes little sense with access-control-allow-origin: * because we want liberal access to public goods, and we don't have site-specific responses, so we want cache to be shared across websites that use public goods to maximize cache HIT rate:

👉 We want CID lookup done by JS running on https://one.example.com to be returned from cache when https://two.example.net asks for it.

Solution

Trustless public good services must have a global cache that is shared across websites (no matter what is in Origin), which means we don't want Vary: Origin at

  • delegated-ipfs.dev (needs fixing)
  • trustless-gateway.link (already ok, but mentioning here as it should share the config/setup/tests)

TODO

  • adding CORS, Vary, and HTTP Caching to our HTTP specs is tracked in gateway: add CORS to specs ipfs/specs#423 (added /routing/v1 items there, consider out of scope here)
  • remove Vary: Origin from delegated-ipfs.dev responses
    • Caveat: be careful to not remove Vary: Accept-Encoding which is used by compression
    • I suspect delegated-ipfs.dev regression was introduced when we added github.com/rs/cors to someguy (add http handlers for cors, metrics and compression ipfs/someguy#30). An easy fix may be to remove that library and instead hardcode liberal CORS headers on all response types.
@lidel
Copy link
Contributor Author

lidel commented Feb 2, 2024

@ns4plabs I am not sure how easy it will be to fix in someguy without having to modify the github.com/rs/cors library (imo it should not send Vary: Origin if CORS is effectively disabled/lifted by *).

I it helps, I think for this specific project (someguy) hardcoding liberal CORS on all GET|HEAD|POST|OPTIONS responses is fine. Are you able to add this to your queue and ping me or @hacdias for review?

@lidel
Copy link
Contributor Author

lidel commented Jul 25, 2024

Somehow related improvements landed in https://github.com/ipshipyard/waterworks-infra/issues/215
Remaining work is to remove Vary: Origin

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

No branches or pull requests

2 participants