Skip to content

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 a team and lidel 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 ipfs/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
45 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
lidel added a commit to ipfs/specs that referenced this pull request Aug 23, 2025
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.

3 participants