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

Added check for global flag in pattern. #397

Merged
merged 1 commit into from
Aug 14, 2014
Merged

Conversation

arb
Copy link
Contributor

@arb arb commented Aug 13, 2014

If you had /g in your pattern option, the results of pattern.regex.test(key) here will toggle back and forth between true and false creating very unpredictable validation results.

Read up about this design decision http://stackoverflow.com/a/1520853/1011616

I'm not sure what to do about the versioning of this. If you're using a pattern function with "/g" then this is going to break your code.

@geek
Copy link
Member

geek commented Aug 14, 2014

Migration step

Change any global regex pattern to not be global

geek added a commit that referenced this pull request Aug 14, 2014
Added check for global flag in pattern.
@geek geek merged commit 68fa78d into hapijs:master Aug 14, 2014
@geek geek self-assigned this Aug 14, 2014
@hueniverse
Copy link
Contributor

This is the wrong solution.

First, we also need to handle string.regex() which has the same issue. Second, global flag means nothing for validation because it is always called once per string. Instead of a breaking change, it would be better to remove the global flag from the expression, at least for 4.x. We can make a breaking change in 5.x by no longer allowing it.

There is also the sticky flag which I am not even sure how it works. So to be safe, we should always create a new regex and only copy the i flag over.

@hueniverse hueniverse added this to the 4.7.0 milestone Aug 17, 2014
@hueniverse hueniverse assigned hueniverse and unassigned geek Aug 17, 2014
hueniverse added a commit that referenced this pull request Aug 17, 2014
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug or defect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants