Closed
Bug 634262
Opened 13 years ago
Closed 13 years ago
Ensure that nsHtml5StreamParser::WriteStreamBytes is correct given the Unicode decoder interface
Categories
(Core :: DOM: HTML Parser, defect)
Core
DOM: HTML Parser
Tracking
()
RESOLVED
FIXED
mozilla2.0
Tracking | Status | |
---|---|---|
status1.9.2 | --- | unaffected |
status1.9.1 | --- | unaffected |
People
(Reporter: hsivonen, Assigned: hsivonen)
Details
(Whiteboard: [sg:nse][keep private until bug 600974 and bug 634257 are public])
Attachments
(1 file)
2.80 KB,
patch
|
smontagu
:
review+
beltzner
:
approval2.0-
|
Details | Diff | Splinter Review |
From bug 600974: > The decode loop in the HTML5 parser assumes that if the result is > NS_PARTIAL_MORE_OUTPUT the decoder hasn't consumed all input bytes. This is > wrong, of course, if the decoder consumed a 4-byte sequence but had room to > write only the first surrogate. > > What's the new API contract? How should I change nsHtml5StreamParser.cpp? > > I don't think there's any change, except for the assertion that you linked to > before. > > * Unless there is not enough output space, this method must consume all the > * available input data! > > This doesn't necessarily imply the converse, i.e. that where there isn't enough > output space, not all input data will be consumed. I'll add a sentence to > clarify that. > > What's the new API contract? How should I change nsHtml5StreamParser.cpp? > > The assertion > NS_ASSERTION(byteCount > 0 || NS_FAILED(convResult), > "The decoder consumed too few bytes but did not signal an error."); > will also need to change to cover the case where the decoder returns > NS_PARTIAL_MORE_OUTPUT at the very end of the input, and the caller has to call > Convert() again with no input. Marking as security-sensitive, since I'm quoting from bug 600974, which is security-sensitive. Also, bug 634257 is related and security-sensitive.
Assignee | ||
Updated•13 years ago
|
Summary: Ensure that nsHtml5StreamParser::FinalizeSniffing is correct given the Unicode decoder interface → Ensure that nsHtml5StreamParser::WriteStreamBytes is correct given the Unicode decoder interface
Assignee | ||
Comment 1•13 years ago
|
||
AFAICT, only assertions needed changing.
Updated•13 years ago
|
Whiteboard: [sg:nse][keep private until bug 600974 and bug 634257 are public]
Updated•13 years ago
|
Attachment #512770 -
Flags: review?(smontagu) → review+
Assignee | ||
Comment 2•13 years ago
|
||
Comment on attachment 512770 [details] [diff] [review] Adjust assertions Nominating for approval, because having right assertions in Gecko 2.0 may help avoid confusion about wrong assertions firing when doing branch maintenance.
Attachment #512770 -
Flags: approval2.0?
Comment 3•13 years ago
|
||
Comment on attachment 512770 [details] [diff] [review] Adjust assertions a=beltzner
Attachment #512770 -
Flags: approval2.0? → approval2.0+
Comment 4•13 years ago
|
||
Comment on attachment 512770 [details] [diff] [review] Adjust assertions Didn't land in time.
Attachment #512770 -
Flags: approval2.0+ → approval2.0-
Assignee | ||
Comment 5•13 years ago
|
||
Actually, this did land. Looks like I've failed to update the bug. Sorry. http://hg.mozilla.org/mozilla-central/rev/b1ef0685b2e0
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Group: core-security
status1.9.1:
--- → unaffected
status1.9.2:
--- → unaffected
Target Milestone: --- → mozilla2.0
You need to log in
before you can comment on or make changes to this bug.
Description
•