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

regexp should be wrapped in try/catchs #2477

Open
RedHatter opened this issue Jul 19, 2024 · 6 comments
Open

regexp should be wrapped in try/catchs #2477

RedHatter opened this issue Jul 19, 2024 · 6 comments

Comments

@RedHatter
Copy link

RedHatter commented Jul 19, 2024

What version of Ajv you are you using?
8.17.1

What problem do you want to solve?
Tracking down invalid regex.

What do you think is the correct solution to problem?
An invalid regex in the schema (in my case ^[0-9]{2-4}) throws an error without any indication where in the schema or what pattern failed. When you have a large schema this makes it difficult to track down exactly where the bad pattern is. There's a todo in the code about this.

This can be somewhat mitigated by use the regExp option.

new Ajv({
  code: {
    regExp: (pattern: string, u: string) => {
      try {
        return new RegExp(pattern, u)
      } catch (e) {
        console.error('Bad RegExp: ', pattern, e)
      }
    },
  }
})

However this only displays the bad pattern not where in the schema it's used.

Will you be able to implement it?
No.

@jasoniangreen
Copy link
Collaborator

Hi @RedHatter, you are right, it's not very helpful in this situation. Thanks for sharing the mitigating approach. I will try and raise it with the author sometime but there are other priorities at the moment, though I am curious why it wasn't just wrapped at the time. There might be some good reason why not.

I'll keep an eye on this issue too to see how requested a change in this area is.

@jasoniangreen
Copy link
Collaborator

I'll try and find time to fit in a fix on this but have other priorities so I'm also happy to review a PR by someone else.

@jasoniangreen
Copy link
Collaborator

Sorry @RedHatter but I want to check something. You said that the existing error doesn't show you the failing pattern but I get an error message like this:

SyntaxError: Invalid regular expression: /^[0-9]{2-4}/: Incomplete quantifier

@RedHatter
Copy link
Author

The exact error message I got was

Uncaught SyntaxError: incomplete quantifier in regular expression

Note that this was in firefox. I didn't check the error message in any other browsers at the time.

@jasoniangreen
Copy link
Collaborator

Ah I see. It seems in node (at least version 18) you get a more useful message that includes the expression. I'll still keep it in mind because we could include the json schema path of the expression.

@dragonfleas
Copy link

What version of Ajv you are you using? 8.17.1

What problem do you want to solve? Tracking down invalid regex.

What do you think is the correct solution to problem? An invalid regex in the schema (in my case ^[0-9]{2-4}) throws an error without any indication where in the schema or what pattern failed. When you have a large schema this makes it difficult to track down exactly where the bad pattern is. There's a todo in the code about this.

This can be somewhat mitigated by use the regExp option.

new Ajv({
  code: {
    regExp: (pattern: string, u: string) => {
      try {
        return new RegExp(pattern, u)
      } catch (e) {
        console.error('Bad RegExp: ', pattern, e)
      }
    },
  }
})

However this only displays the bad pattern not where in the schema it's used.

Will you be able to implement it? No.

No idea how you're doing this. This actually gives me a type error.

Property 'code' is missing in type '(pattern: string, u: string) => RegExp | undefined' but required in type 'RegExpEngine'.

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

No branches or pull requests

3 participants