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

fix(parser) Fix freezing issue with illegal 0 width matches #2524

Merged
merged 4 commits into from
May 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,9 @@ Brower build:
- [Issue](https://github.com/highlightjs/highlight.js/issues/2505) (bug) Fix: Version 10 fails to load as CommonJS module. (#2511) [Josh Goebel][]
- [Issue](https://github.com/highlightjs/highlight.js/issues/2505) (removal) AMD module loading support has been removed. (#2511) [Josh Goebel][]


Parser Engine Changes:

- ...
- [Issue](https://github.com/highlightjs/highlight.js/issues/2522) fix(parser) Fix freez issue with illegal 0 width matches (#2524) [Josh Goebel][]


[Josh Goebel]: https://github.com/yyyc514
Expand Down
19 changes: 19 additions & 0 deletions src/highlight.js
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,23 @@ const HLJS = function(hljs) {
}
}

// edge case for when illegal matches $ (end of line) which is technically
// a 0 width match but not a begin/end match so it's not caught by the
// first handler (when ignoreIllegals is true)
if (match.type === "illegal" && lexeme === "") {
// advance so we aren't stuck in an infinite loop
return 1;
}

// infinite loops are BAD, this is a last ditch catch all. if we have a
// decent number of iterations yet our index (cursor position in our
// parsing) still 3x behind our index then something is very wrong
// so we bail
if (iterations > 100000 && iterations > match.index * 3) {
const err = new Error('potential infinite loop, way more iterations than matches');
throw err;
}
Comment on lines +376 to +383
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I'm late to the party. Quick fix was the right thing to do.
I'd move this check out to the for (;;) loop, in order not to deal with iterations inside processLexeme function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except then we'd literally have to check it ALL the time... where-as right now it's more of a fallback... but I'll play around with it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But technically your right in that if there were a bug in the above code that this wouldn't be well suited to catch it... :-/


/*
Why might be find ourselves here? Only one occasion now. An end match that was
triggered but could not be completed. When might this happen? When an `endSameasBegin`
Expand Down Expand Up @@ -396,12 +413,14 @@ const HLJS = function(hljs) {
var mode_buffer = '';
var relevance = 0;
var index = 0;
var iterations = 0;
var continueScanAtSamePosition = false;

try {
top.matcher.considerAll();

for (;;) {
iterations++;
if (continueScanAtSamePosition) {
continueScanAtSamePosition = false;
// only regexes not matched previously will now be
Expand Down