-
Notifications
You must be signed in to change notification settings - Fork 236
IPIP-0523: Prefer format request param over Accept header #523
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
base: main
Are you sure you want to change the base?
Conversation
8c22f1a to
f76b2d5
Compare
Browsers will always send an Accept header so if present the format request param should take priority, not the other way round as the spec currently says.
a0ec721 to
824cf89
Compare
🚀 Build Preview on IPFS ready
|
- clarify format is URL query param, Accept is HTTP header - expand Compatibility with HTTP caching rationale - add Security and Alternatives sections - update precedence rule in both path-gateway and trustless-gateway
update car-version, car-order, car-dups to match IPIP-0523: URL query parameter SHOULD take precedence over Accept header
…pt header this change simplifies precedence rules by making the ?format= URL query parameter always take priority over the Accept HTTP header when both are present. in practice, this is largely compatible with existing browser use cases since browsers send Accept headers with wildcards which were already treated as non-specific. prioritizing ?format= also ensures deterministic HTTP caching behavior, protecting against CDNs that comingle different response types under the same cache key. the only breaking change is for edge cases where a client sends both a specific Accept header and a different ?format= value. previously Accept would win, now ?format= wins. this scenario is rare and arguably represents client misconfiguration. when detected, gateway returns HTTP 400 to signal the ambiguity. specs: ipfs/specs#523 tests: ipfs/gateway-conformance#252
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for cleaning this up. This is def. a technical debt we asccumulated by adding features over time. Work on Service Worker gateway makes it a good time to do this.
lgtm, added some meat to missing/empty sections
GO implementation is ready – details in ipfs/gateway-conformance#252 (review) (if no concerns, we will merge later this week and ship in Kubo 0.40)
| Users will be able to use the `format` URL query parameter to control the | ||
| response type of requests made from browser address bars. | ||
|
|
||
| ### Compatibility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ℹ️ @achingbrain fyi I've expanded the Compatibility section. While formally this is a breaking change, in practice boxo/gateway already had an escape hatch in customResponseFormat which prioritized format if Accept has a wildcard or used something other than raw, car or ipns-record
(imo this is good news, means IPIP is actually cleaning up and formalizing real world behavior in browsers)
src/ipips/ipip-0523.md
Outdated
| **Keep status quo with wildcard exception**: The previous spec already had a | ||
| carve-out where wildcards (e.g., `*/*`, `application/*`) in the `Accept` HTTP | ||
| header did not take precedence over the `format` URL query parameter. This meant |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also a bad alternative since there's still a need to handle wildcard complexity, but it seems like it'd be helpful if implementations rejected as invalid conflicting requests (e.g. request for a block in the header, but a CAR in the query parameter).
i.e. the rule becomes:
- If the accept header asks for one of multiple possible options and none of them match the one requested in the query parameter (including wildcards matching) -> error
- Otherwise take the more precise variant (e.g. normally the query parameter except for CARs where you might need to check the accept header for a more precise CAR description)
- If there's ever ambiguity here just error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was not included in the IPIP on purpose, for pragmatic reasons (cure causes more problems than it solves):
-
The whole point of this IPIP is to simplify: "URL param
?formatalways wins" means no extra code for conflict handling needed. No surface for bugs no matter how your HTTP stack looks like.-
The important nuance is that this aligns with how CDNs and HTTP caches actually work in practice: they often ignore or poorly handle
Acceptheaders. This includes defaults in software like Nginx and major CDNs like Cloudflare, which can be... liberal with RFC compliance at the edges. -
In the real world, a request traverses N hops, each one with different quirks. The URL is the only reliable way to ensure correct namespacing of HTTP caches for different response formats across all hops.
-
This is not a theory.
ipfs.io/delegated-ipfs.devare literally adding explicit?format=on the fly (internally) at Cloudflare CDN right now to ensure CAR, raw, and deserialized responses are not commingled at their and/or internal Nginx caches. Yes, it is this janky, and yes, very likely everyone else running a gateway either ignores this, or is not aware they (in some permutations of caches, cdns, and default reverse-proxy configurations) may have commingled caches if they don't use explicit?format=and only depend onAccept. -
Adding a HTTP 400 requirement on conflicting values would be (IMO) a dead/janky spec. At very least, would make the spec harder to test reliably via conformance:
- In many deployments, CDNs/reverse-proxies strip or transform
Acceptheaders before they reach the cache or final server. A test expecting HTTP 400 for "conflicting headers" would fail with HTTP 200,
not because the gateway is broken, but because the CDN never forwarded theAcceptheader in the first place, or because someone has extra cache commingling protections like ipfs.io does. - If we dont include it in spec, but do it in boxo, then it would not pass gateway-conformance, forcing everyone to add
-skipfor that test (aka dead spec / dead test).
- In many deployments, CDNs/reverse-proxies strip or transform
-
-
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on flagging this, I realize this rationale is likely hidden tribal knowledge. Added shortened version of the above to IPIP in 06f8cc5
add alternative explaining why we don't error when Accept header and ?format disagree: complexity, real-world HTTP infra behavior, and testability concerns
Browsers will always send an Accept header so if present the format request param should take priority, not the other way round as the spec currently says.
Things to land before ratifying (merging) this IPIP