Skip to content

Added check for global flag in pattern.#397

Merged
geek merged 1 commit intohapijs:masterfrom
arb:pattern-fix
Aug 14, 2014
Merged

Added check for global flag in pattern.#397
geek merged 1 commit intohapijs:masterfrom
arb:pattern-fix

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