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: front slashes only get escaped now if they aren't already #376

Closed

Conversation

nickytonline
Copy link
Contributor

@nickytonline nickytonline commented Apr 27, 2023

🎉 Thanks for sending this pull request! 🎉

Please make sure the title is clear and descriptive.

If you are fixing a typo or documentation, please skip these instructions.

Otherwise please fill in the sections below.

Which problem is this pull request solving?

Currently Next.js middleware breaks because the edge bundler modifies the regular expression matchers for URLs that Next.js generates.

List other issues or pull requests related to this problem

fixes #375
fixes https://github.com/netlify/pod-ecosystem-frameworks/issues/414
relates to opennextjs/opennextjs-netlify#1919

Describe the solution you've chosen

I've modified the regex to do a negative lookbehind to ensure we don't escape a front slash if it's already escaped.

Describe alternatives you've considered

N/A but as I mentioned in the issue, do we want the edge bundler modifying regex patterns?

Checklist

Please add a x inside each checkbox:

  • I have read the contribution guidelines.
  • The status checks are successful (continuous integration). Those can be seen below.

A picture of a cute animal (not mandatory but encouraged)

@nickytonline nickytonline self-assigned this Apr 27, 2023
@nickytonline nickytonline marked this pull request as ready for review April 27, 2023 16:12
@nickytonline nickytonline requested a review from a team April 27, 2023 16:12
Copy link
Member

@eduardoboucas eduardoboucas left a comment

Choose a reason for hiding this comment

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

LGTM! I left just tiny style nits.

Do you want to move this out of draft?

node/declaration.ts Outdated Show resolved Hide resolved
node/declaration.test.ts Outdated Show resolved Hide resolved
node/declaration.test.ts Outdated Show resolved Hide resolved
node/declaration.test.ts Outdated Show resolved Hide resolved
@nickytonline nickytonline marked this pull request as ready for review April 28, 2023 14:46
nickytonline and others added 4 commits April 28, 2023 10:46
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
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.

Error thrown when escaping front slashes in regex patterns that have already been escaped
2 participants