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(gateway): preserve query parameters on _redirects #886

Merged
merged 2 commits into from
Mar 21, 2025

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Mar 18, 2025

Preserve query parameters for _redirects file redirects

Still needs a test.. and potentially CHANGELOG.md update.

related ipfs/service-worker-gateway#628 + ipfs/specs#498

cc @lidel

@lidel lidel force-pushed the fix/preserve-query-params-with_redirects-file branch from f715771 to 9b4b453 Compare March 19, 2025 15:22
@ipfs ipfs deleted a comment from welcome bot Mar 19, 2025
adds missing test + ensures any params from _redirects are preserved

this follows what is already supported by Cloudflare Pages
implementation:
https://developers.cloudflare.com/pages/configuration/redirects/#per-file
@lidel lidel force-pushed the fix/preserve-query-params-with_redirects-file branch from 9b4b453 to 2d9d254 Compare March 19, 2025 15:50
@lidel lidel changed the title fix: preserve query parameters fix(gateway): preserve query parameters on _redirects Mar 19, 2025
lidel added a commit to ipfs/specs that referenced this pull request Mar 19, 2025
@lidel lidel marked this pull request as ready for review March 19, 2025 16:06
@lidel lidel requested review from lidel and a team as code owners March 19, 2025 16:06
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't help with Cloudflare-hosted sw gateway, but we should fix the bug where params are not preserved ( we preserve them in all other cases such as subdomain redirects).

  • Added support for merging static and dynamic query parameters, prioritizing user values
  • Added tests and
  • CHANGELOG.md updated
  • Opened redirects file: clarify query parameters specs#498 to fill the spec gap and clarify how query parameters should be handled.

@SgtPooki in spare time take a look, if specs/tests look fine, I think it is fine to merge this fix (query params should be preserved).

Copy link
Member Author

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests look good to me

@lidel lidel mentioned this pull request Mar 20, 2025
27 tasks
@SgtPooki SgtPooki merged commit abb4ed8 into main Mar 21, 2025
15 checks passed
@SgtPooki SgtPooki deleted the fix/preserve-query-params-with_redirects-file branch March 21, 2025 15:53
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

Successfully merging this pull request may close these issues.

2 participants