Bug 201769 – Web Inspector: HTML Formatting: Handle infinite loop for incomplete script data

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

HTML Formatting: Handle infinite loop for incomplete script data

Steps to Reproduce:
1. Attempt to format something like "<script src="foo.js"> </sc"
  => Infinite loop

[Joseph Pecoraro]

Created attachment 378749 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Comment on attachment 378749 [details]
[PATCH] Proposed Fix

Revising this with some more edge cases.

[Joseph Pecoraro]

Created attachment 378762 [details]
[PATCH] Proposed Fix

[Devin Rousso]

Comment on attachment 378762 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=378762&action=review

r=me, looks reasonable :)

> Source/WebInspectorUI/UserInterface/Workers/Formatter/HTMLParser.js:181
> +        if (this._isEOF() && this._data.endsWith("<"))

Could you move this check into the `if (text)` above, since we would’ve consumed up to a “<“ there anyways?  I think then we could still use `_handleEOF`.

> Source/WebInspectorUI/UserInterface/Workers/Formatter/HTMLParser.js:199
> +                this._handleEOF(startPos);

Could we have `_handleEOF` return whether `_isEOF` so we don’t have to check it twice?

[Joseph Pecoraro]

Comment on attachment 378762 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=378762&action=review

>> Source/WebInspectorUI/UserInterface/Workers/Formatter/HTMLParser.js:181
>> +        if (this._isEOF() && this._data.endsWith("<"))
> 
> Could you move this check into the `if (text)` above, since we would’ve consumed up to a “<“ there anyways?  I think then we could still use `_handleEOF`.

I think we will need to do both anyways. For example `<` (empty text, but last character is "<"),  and `a<` (non-empty text, but last character is "<"). Or am I overlooking something?

>> Source/WebInspectorUI/UserInterface/Workers/Formatter/HTMLParser.js:199
>> +                this._handleEOF(startPos);
> 
> Could we have `_handleEOF` return whether `_isEOF` so we don’t have to check it twice?

Sure

[Joseph Pecoraro]

https://trac.webkit.org/r249866

[Radar WebKit Bug Importer]

<rdar://problem/55360232>