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

provide fix for bug related to parsing GET parameters #832

Merged
merged 2 commits into from
Oct 5, 2018

Conversation

kyle-mccarthy
Copy link
Contributor

@kyle-mccarthy kyle-mccarthy commented Oct 3, 2018

This fixes #831. I've updated the code and provided a test case.

This problem is caused when the path option is a GET parameter. With the current version, you can make it work if you escape the ?; however, I didn't see documentation detailing this. In this PR I escape the ? if it hasn't already been properly escaped when setting the option.

Basically, this currently doesn't work blog?page={{#}} but doing this blog\\?page={{#}} will work. Prior to changing out {{#}} with the regexp to match the page number, I convert blog?page={{#}} to blog\\?page={{#}} if the user hasn't already done so.

@desandro
Copy link
Member

desandro commented Oct 5, 2018

Whoa, thanks for all the hard work here! Excellent PR. Merging now.

@desandro desandro merged commit a24528d into metafizzy:master Oct 5, 2018
@desandro
Copy link
Member

@kyle-mccarthy Looks like this fix isn't working in Firefox. /(?<!\\)\?/ is an invalid regular expression. Could you take another look?

@kyle-mccarthy
Copy link
Contributor Author

@desandro Sorry about that, I did not realize that Firefox didn't support negative lookbehinds. I opened another PR fixing the problem (and tested it in FF this time :)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Does not correctly parse pageIndex from URL when set as a GET parameter
2 participants