Closed Bug 1176668 Opened 5 years ago Closed 4 years ago

Code from handling NCR overflow is bogus, especially upon EOF

Categories

(Core :: DOM: HTML Parser, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: hsivonen, Assigned: hsivonen)

References

()

Details

Attachments

(2 files, 2 obsolete files)

Steps to reproduce:
 1) http://software.hixie.ch/utilities/js/live-dom-viewer/saved/3540

Actual results:
Not U+FFFD.

Expected result:
U+FFFD.
Attached patch htmlparser repo patch (obsolete) — Splinter Review
Test cases: https://github.com/html5lib/html5lib-tests/pull/59
Attachment #8625262 - Flags: review?(wchen)
Attached patch m-c patch (obsolete) — Splinter Review
Attachment #8625263 - Flags: review?(wchen)
Blocks: 1029671
See bug 1029671 comment 36 for the big picture of the whole queue and about landing.
Comment on attachment 8625262 [details] [diff] [review]
htmlparser repo patch

Review of attachment 8625262 [details] [diff] [review]:
-----------------------------------------------------------------

We should also add an assertion to handleNcrValue to make sure that value is non-negative.

::: src/nu/validator/htmlparser/impl/Tokenizer.java
@@ +3352,5 @@
>                              seenDigits = true;
> +                            // Avoid overflow
> +                            if (value <= 0x10FFFF) {
> +                                value *= 16;
> +                                value += c - '0';                                

trailing whitespace
Attachment #8625262 - Flags: review?(wchen) → review+
Attachment #8625263 - Flags: review?(wchen) → review+
Thanks. Attaching the fix patch for the record.
Attachment #8625262 - Attachment is obsolete: true
Attachment #8644264 - Flags: review+
Attachment #8625263 - Attachment is obsolete: true
Attachment #8644265 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/bb56d50195c4
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.