fix: potential super-linear regular expressions#463
fix: potential super-linear regular expressions#463lumirlumir merged 1 commit intoeslint:mainfrom ericcornelissen:patch-1
Conversation
|
Thanks for the pull request. Could you also add unit tests to show that the problems has been fixed? Each |
|
Hi everyone :) I'm not very familiar with this area, so I have a quick question about this PR. (This isn't a blocking comment—just asking out of curiosity.) Is this PR related to a potential security issue? We had a security concern recently that was resolved here: GHSA-7q7g-4xm8-89cq Does this PR address the same issue I mentioned above? |
|
@fasttime updated, though the 30s timeout in this project means bigger test vectors than eslint/rewrite#240 are required. Also, these large test vectors kinda clutter the test output for this project... @lumirlumir please see GHSA-xffm-g5w8-qvg7. |
fasttime
left a comment
fasttime
left a comment
There was a problem hiding this comment.
I can't say if the 30 seconds timeout is still needed in this project. To avoid cluttering the output, an option is to run ESLint directly. The test could just check that no timeout occurs. For example:
import { Linter } from "eslint";
it("should not timeout for large inputs", () => {
const linter = new Linter();
linter.verify(`# example${" ".repeat(500_000)}?#`, {
language: "markdown/commonmark",
plugins: { markdown },
rules: { "markdown/no-duplicate-headings": "error" },
});
});
fasttime
left a comment
fasttime
left a comment
There was a problem hiding this comment.
The changes to the regular expressions are looking good. I left a couple of suggestions about the tests.
Improve regular expressions that could match in super-linear runtime for certain inputs.
fasttime
left a comment
fasttime
left a comment
There was a problem hiding this comment.
LGTM, thanks! I would like another review from a team member.
github-project-automation
bot
moved this from Second Review Needed
to Complete
in Triage
3 participants
Prerequisites checklist
What is the purpose of this pull request?
Improve regular expressions in this repository that could match with unexpected overhead for certain inputs.
What changes did you make? (Give an overview)
I have improved 3 regular expressions that could match in super-linear runtime for certain inputs. In the order of the changed files, the following test vectors demonstrate the improved runtime:
"# example" + " ".repeat(100_000) + "?#""[".repeat(100_000) + "x""<div>" + "<".repeat(100_000) + "x</div>"(for(?<!<))b.
"<div><" + " ".repeat(100_000) + " x</div>"(for\s)I found (using js-re-scan) four more candidate regular expressions with potential super-linear runtime. 1 for
no-invalid-label-refswhich appears to be a false positive because all input strings are safe aftersliceing. The other three (inno-multiple-h1,no-reversed-media-syntax,require-alt-text) I was not able to confirm nor refute.Related Issues
refs eslint/rewrite#240
Is there anything you'd like reviewers to focus on?
no