Skip to content
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

Some regexps use ? quantifiers for look-aheads #2842

Closed

Description

@RReverser

Probably purely a code style issue, but some regular expressions use constructs like (?=...)? which is valid in JavaScript but don't make sense and only complicate expressions. Here's a list of all the found ones:

{ input: '^http://www\\.heteml\\.jp/[^?]*(?=\\?)?',
  error: 
   { name: 'RegexSyntaxError',
     type: 'NothingRepeat',
     lastIndex: 36,
     message: 'Nothing to repeat!Repeat after assertion doesn\'t make sense!' } }
{ input: '^http://store\\.mandriva\\.com/[^?]*(?=\\?.*)?',
  error: 
   { name: 'RegexSyntaxError',
     type: 'NothingRepeat',
     lastIndex: 43,
     message: 'Nothing to repeat!Repeat after assertion doesn\'t make sense!' } }
{ input: '^http://soe\\.stanford\\.edu/+(?=\\?.*)?$',
  error: 
   { name: 'RegexSyntaxError',
     type: 'NothingRepeat',
     lastIndex: 37,
     message: 'Nothing to repeat!Repeat after assertion doesn\'t make sense!' } }
{ input: '^http://(?:www\\.)?tescofinance\\.com/+(?=\\?.*)?$',
  error: 
   { name: 'RegexSyntaxError',
     type: 'NothingRepeat',
     lastIndex: 46,
     message: 'Nothing to repeat!Repeat after assertion doesn\'t make sense!' } }
{ input: '^http://hilfe\\.udmedia\\.de/[^?]*(?=\\?.*)?',
  error: 
   { name: 'RegexSyntaxError',
     type: 'NothingRepeat',
     lastIndex: 41,
     message: 'Nothing to repeat!Repeat after assertion doesn\'t make sense!' } }
{ input: '^http://livechat\\.udmedia\\.de/[^?]*(?=\\?.*)?',
  error: 
   { name: 'RegexSyntaxError',
     type: 'NothingRepeat',
     lastIndex: 44,
     message: 'Nothing to repeat!Repeat after assertion doesn\'t make sense!' } }

Activity

  1. fuglede commented on Sep 12, 2015

    @fuglede
    Contributor

    Are you talking about that the ? after the look-ahead? If so, I'm not quite sure what it means that it doesn't make sense? While I agree that those expressions aren't the easiest to read, the question mark is not non-functional. For example, the string / is matched by /[^?](?=?.)? but not by /[^?](?=?.).

  2. forgreatsh commented on Sep 12, 2015

    @forgreatsh

    Back references "don't make sense" if by "don't make sense" you mean describe something other than a formal language. ?! and ?= make perfect sense in pcre (which I concede are irregular).

    There are at least two good reasons for using such lookaheads:

    • It makes it harder to misplace or misnumber backreferences. It's also easier to review when the intention to drop or keep the matched pattern is clear, which the distinction between ?!, ?=, and \?[^?!:] makes so.
      • Admittedly ?: Also serves this purpose, but it's even easier to see ?! and ?=.
    • Complicated but direct rewrites can be turned into exclusions, such that it is impossible for a malicious author to include an expression that rewrites to, say, $evil_tracking_location. The worst they could do is exclude a pattern that we would expect to be secured.
  3. RReverser commented on Sep 13, 2015

    @RReverser
    ContributorAuthor

    Are you talking about that the ? after the look-ahead? If so, I'm not quite sure what it means that it doesn't make sense? While I agree that those expressions aren't the easiest to read, the question mark is not non-functional. For example, the string / is matched by /[^?]*(?=\?.*)? but not by /[^?]*(?=\?.*).

    Original message was from regulex parser which is stricter than regular JS engines. My understanding is that it's correct, you just chose incorrect alternative. The thing is that either both me and regulex authors (cc @JexCheng) are missing something, or /[^?]*(?=\?.*)? has just the same effect as /[^?]* since "optional lookahead" inside of the regular expression is just the same as if there would be no look-ahead at all. Thus it just hurts both readability and performance of such regexes without any real effect.

  4. RReverser commented on Sep 13, 2015

    @RReverser
    ContributorAuthor

    @forgreatsh No, that's not what I mean by "doesn't make sense". I know what are look-aheads for in generic cases :) Please see explanation of this specific problem in previous comment.

  5. CJex commented on Sep 13, 2015

    @CJex

    @RReverser Sorry it's my fault, I have noticed this kind bug these days, it was fixed now:
    CJex/regulex#29
    It should be a warning, not an error.

  6. fuglede commented on Sep 13, 2015

    @fuglede
    Contributor

    I think you're right though, that /[^?](?=?.)? is equivalent to /[^?]*, so there must be a reason it was done the other way. The Mandriva ruleset has the following comment, which may or may not have something to do with it:

        <!--    Server drops both trailing slash and path, but we can't
                emulate the former due to ruleset validation:
                                                                -->
    
  7. RReverser commented on Sep 13, 2015

    @RReverser
    ContributorAuthor

    I see, keeping it open then.

  8. RReverser commented on Sep 14, 2015

    @RReverser
    ContributorAuthor

    due to ruleset validation

    @fuglede If this is a reason, maybe it would make sense to fix rule validation. What do you think?

  9. fuglede commented on Sep 14, 2015

    @fuglede
    Contributor

    It validates just fine with /[^?]* though. @2d1?

  10. fuglede commented on Sep 29, 2015

    @fuglede
    Contributor

    Super. @RReverser, do you want to submit a PR fixing the redundant ones?

  11. RReverser commented on Sep 29, 2015

    @RReverser
    ContributorAuthor

    I can a bit later.

  12. added a commit that references this issue on Sep 29, 2015
    7173afd
  13. RReverser commented on Sep 29, 2015

    @RReverser
    ContributorAuthor

    Done!

  14. added a commit that references this issue on Nov 16, 2015
    9e13ebf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions