Bug 201769 - Web Inspector: HTML Formatting: Handle infinite loop for incomplete script data
Summary: Web Inspector: HTML Formatting: Handle infinite loop for incomplete script data
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-09-13 13:05 PDT by Joseph Pecoraro
Modified: 2019-09-13 18:59 PDT (History)
4 users (show)

See Also:

Attachments
[PATCH] Proposed Fix (4.03 KB, patch)
2019-09-13 13:06 PDT, Joseph Pecoraro 		no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (6.09 KB, patch)
2019-09-13 16:42 PDT, Joseph Pecoraro 		hi: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2019-09-13 13:05:54 PDT 
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
Comment 1 Joseph Pecoraro 2019-09-13 13:06:51 PDT 
Created attachment 378749 [details]
[PATCH] Proposed Fix
Comment 2 Joseph Pecoraro 2019-09-13 16:35:43 PDT 
Comment on attachment 378749 [details]
[PATCH] Proposed Fix

Revising this with some more edge cases.
Comment 3 Joseph Pecoraro 2019-09-13 16:42:41 PDT 
Created attachment 378762 [details]
[PATCH] Proposed Fix
Comment 4 Devin Rousso 2019-09-13 18:06:47 PDT 
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?
Comment 5 Joseph Pecoraro 2019-09-13 18:53:54 PDT 
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
Comment 6 Joseph Pecoraro 2019-09-13 18:58:14 PDT 
https://trac.webkit.org/r249866
Comment 7 Radar WebKit Bug Importer 2019-09-13 18:59:21 PDT 
<rdar://problem/55360232>