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

Pattern handling breaks when passing patterns with flags #325

Open
kyrandita opened this issue Nov 17, 2020 · 5 comments
Open

Pattern handling breaks when passing patterns with flags #325

kyrandita opened this issue Nov 17, 2020 · 5 comments
Assignees
Labels
enhancement feedback Waiting for more information from OP. May be closed after 2 weeks with no response.
Milestone

Comments

@kyrandita
Copy link

kyrandita commented Nov 17, 2020

Recently updated to 1.3.0, the bug seems to have been introduced in 1.2.7 relating to commit 35db08d (thanks to issue #311 for finding this for me) confirmed still present in 1.4.0

The issue is that when passing an existing regex pattern /abc/i (matches 'abc', 'ABC' and various combinations) the existing flags are overridden by the addition of the 'u' flag making the resulting pattern /abc/u (matches ONLY 'abc'). By using the new RegExp(pattern, flags) constructor the flags parameter when provided overrides all existing flags if the pattern has any.

In my opinion forcing the u flag on all regex is wrong, if that is the default when passing as string patterns I don't see anything strictly wrong with that, but if I pass a pattern why are my flags being messed with? If adding the u flag is for some reason required even when I didn't set it myself my flags should at least be preserved.

The following would be my preferred solution

try {
  if(pattern instanceof RegExp) {
    regexp = pattern;
  } else {
    regexp = new RegExp(pattern, 'u');
  }
} catch(_e) {

If forcing the u flag is deemed necessary then substituting regexp = new RegExp(pattern, pattern.flags + 'u'); should work for the passing a regex object case

@kyrandita
Copy link
Author

kyrandita commented Nov 18, 2020

technically the entire try/catch could be moved inside this else block since it's only to provide backup behavior for when the forcing of the u flag breaks things on string patterns

@awwright
Copy link
Collaborator

Passing a RegExp literal is not currently supported (it's not described anywhere, and there's no test for it).

But this is a reasonable feature request, I'll take a look.

@awwright awwright self-assigned this Nov 18, 2020
@kyrandita
Copy link
Author

kyrandita commented Nov 18, 2020

examples/all.js line 207 (last updated in 2016) is passing a RegExp literal, although not using flags to be sure... Though this was implemented long enough ago initially that I can't be sure it wasn't just "try and see if it works" on our side, in either case thank you for considering this :)

@awwright awwright added this to the v1.5 milestone Nov 18, 2020
@awwright
Copy link
Collaborator

awwright commented Feb 7, 2021

@kyrandita Can you please post a code sample that you think should work, but doesn't? This seems to work for me:

validate('0{123}', {'type': 'string', 'pattern': /.{1.3}/});

@awwright awwright added the feedback Waiting for more information from OP. May be closed after 2 weeks with no response. label Feb 7, 2021
@alanosullivancs
Copy link

@awwright here is an example is the case sensitivity flag which should be observed but is overwritten in the RegExp constructor:
validate('cAsEsenSitIve', {'type': 'string', 'pattern': /casesensitive/i});
this returns an error that is incorrect: 'instance does not match pattern "/casesensitive/i"'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feedback Waiting for more information from OP. May be closed after 2 weeks with no response.
Projects
None yet
Development

No branches or pull requests

3 participants