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

This regex can be stucked by strings #7354

Closed
2bdenny opened this issue Apr 16, 2018 · 2 comments
Closed

This regex can be stucked by strings #7354

2bdenny opened this issue Apr 16, 2018 · 2 comments
Labels
Priority:P3 Cosmetic bugs, minor bugs with a clear workaround Type:Bug Product defects .Unable to Reproduce

Comments

@2bdenny
Copy link

2bdenny commented Apr 16, 2018

.replace(/(\n|\s)*but found:?/, " but found ")

Hello =), I try to extract all regexes in the project and test them, and find this one can be stucked by inputs. If the string, error.message contains too many \n, like "\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n", it will be stucked over 30 seconds.
Recommend /(\s)*but found:?/ instead.

@salsakran salsakran added Type:Bug Product defects Priority:P3 Cosmetic bugs, minor bugs with a clear workaround Query Builder labels Apr 20, 2018
@senior
Copy link
Contributor

senior commented Apr 27, 2018

What version of Metabase were you testing this on? I created a test like the one below:

    it("can ignore whitespace", () => {
        let s = "A * \n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n +"
        expect(compile(s, expressionOpts)).toEqual(["field-id", 1]);
    });

And it failed (as expected) but there was no noticeable impact on performance.

@victoriaspek
Copy link
Contributor

Closing - has been labeled as "needs more info" and "unable to reproduce" for over a year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:P3 Cosmetic bugs, minor bugs with a clear workaround Type:Bug Product defects .Unable to Reproduce
Projects
None yet
Development

No branches or pull requests

4 participants