Closed
Bug 1396557
Opened 7 years ago
Closed 7 years ago
JS::StringIsUTF8, used for Wasm name validation, must be more stringent
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: lth, Assigned: lth)
References
Details
Attachments
(1 file)
1.60 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
As pointed out in bug 1396220, the checker is too lenient with illegal pairs and some out-of-range values. I failed to notice those restrictions when I implemented the code earlier. The correct restrictions are in https://github.com/WebAssembly/spec/blob/master/interpreter/binary/utf8.ml. We'll not wait for bug 1396220 to be resolved (since that involves politics), but will fix this in our codebase and let that thing play out independently.
Assignee | ||
Comment 1•7 years ago
|
||
This adds the value computation and the range checks from the reference implementation, see link in comment 0.
Attachment #8904224 -
Flags: review?(luke)
Comment 2•7 years ago
|
||
(In reply to Lars T Hansen [:lth] from comment #1) > This adds the value computation FWIW, Computing the scalar value isn't logically necessary. The validity can be decided from the byte values alone by limiting the range of the first continuation depending on what the value of the lead was.
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Henri Sivonen (:hsivonen) from comment #2) > (In reply to Lars T Hansen [:lth] from comment #1) > > This adds the value computation > > FWIW, Computing the scalar value isn't logically necessary. The validity can > be decided from the byte values alone by limiting the range of the first > continuation depending on what the value of the lead was. OK, that's an interesting fact. Even so I'll leave that for future work, I think.
Updated•7 years ago
|
Attachment #8904224 -
Flags: review?(luke) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8822dc7f2d3c Make JS::StringIsUTF8 stricter. r=luke
Keywords: checkin-needed
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8822dc7f2d3c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•