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.

Sign up for GitHub

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

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

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
31ad3e7
fix[parser] add edge case handle for illegal 0 width matches
joshgoebel May 2, 2020
a9a5adf
last ditch catch all
joshgoebel May 2, 2020
f83749d
Merge branch 'master' into fix_freeze
joshgoebel May 3, 2020
9edb030
Update CHANGES.md
joshgoebel May 3, 2020
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
19 changes: 19 additions & 0 deletions src/highlight.js
View file
Open in desktop
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