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

[routing-utils] Update header replacing to handle more cases #4942

Merged
merged 3 commits into from
Jul 30, 2020

Conversation

ijjk
Copy link
Member

@ijjk ijjk commented Jul 28, 2020

This adds handling for more cases while updating header values to make sure to escape any characters that could break compiling with path-to-regexp

x-ref: vercel/next.js#15592

Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Beautiful, thanks! 🦄

kodiakhq bot pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2020
Instead of trying to parse header values as URLs and then replace path segments in them this switches to escaping characters that can break `path-to-regexp` compiling

Fixes: #15580
x-ref: vercel/vercel#4942
Copy link
Member

@Timer Timer left a comment

Choose a reason for hiding this comment

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

LGTM, as we landed the same code in Next.js! Thanks!

@ijjk ijjk added the automerge label Jul 30, 2020
@kodiakhq kodiakhq bot merged commit ba9e1dd into master Jul 30, 2020
@kodiakhq kodiakhq bot deleted the ijjk/routing-utils/handle-headers branch July 30, 2020 18:22
@styfle
Copy link
Member

styfle commented Jul 30, 2020

@ijjk Don't forget to update the API repo after publishing canary 👍

LauraBeatris pushed a commit to LauraBeatris/next.js that referenced this pull request Sep 1, 2020
Instead of trying to parse header values as URLs and then replace path segments in them this switches to escaping characters that can break `path-to-regexp` compiling

Fixes: vercel#15580
x-ref: vercel/vercel#4942
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants