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

Gateway redirects should preserve original HTTP method #8080

Open
lidel opened this issue Apr 20, 2021 · 0 comments
Open

Gateway redirects should preserve original HTTP method #8080

lidel opened this issue Apr 20, 2021 · 0 comments
Labels
need/analysis Needs further analysis before proceeding topic/gateway Topic gateway

Comments

@lidel
Copy link
Member

lidel commented Apr 20, 2021

Potential problem in the future

We use HTTP 301 redirects all over the place:

  • URI router: https://dweb.link/ipfs/?uri={ipfs:// uri} endpoint for navigator/registerProtocolHandler redirects to generic content path like https://dweb.link/ipfs/{cid}
  • subdomain router: https://dweb.link/ipfs/{cid} redirects to isolated origin at https://{cid}.ipfs.dweb.link
  • dir normalization: when a CID or a path points at a unixfs directory node, and the path does not end with / gateway returns redirect to URL with / suffix. This is both security (scoping service workers etc) and UX feature.

Returning 301 is may "just work" the spec remains ambiguous, and that may not be compatible with the idea of a "writable" gateway (ipfs/in-web-browsers#180, protocol/web3-dev-team#1) because it does not guarantee the POST request won't be changed to GET.

Fix

[Returning HTTP 308 Permanent Redirect ensures] the request method and the body will not be altered, whereas 301 may incorrectly sometimes be changed to a GET method.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308 / https://tools.ietf.org/html/rfc7238

IIUC this should be a fairly easy fix: replace every place we use 301 with 308.
No change in functionality (HTTP clients operating on GET won't see any difference), but makes things more future-proof.

Caveat*: IE11 on Windows 7 may be a problem, and we should not ship unless the impact is mitigated or acceptable.

@lidel lidel added topic/gateway Topic gateway need/triage Needs initial labeling and prioritization need/analysis Needs further analysis before proceeding and removed need/triage Needs initial labeling and prioritization labels Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/analysis Needs further analysis before proceeding topic/gateway Topic gateway
Projects
None yet
Development

No branches or pull requests

1 participant