375639 - Incorrect uneval of string containing null character

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

js> uneval("a\0b") 
"a\b"

js> function() { return "a\0b"; }
function () {
    return "a\b";
}

Comment 1

[Jesse Ruderman]

MOZILLA_1_8_BRANCH gets this right (using "\x00"), so this is a regression.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Minimal patch

Apparently strchr will happily match the null at the end of the target string so we have to make sure we don't pass it the null character, which breaks the Sprint statement below.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 280792 [details] [diff] [review]
Minimal patch

If I'm reading right, more context would show that this change leads to a %hc format conversion of '\0' in c, which puts a NUL byte into the result buffer. That is not valid UTF-8.

We need to get JS_C_STRINGS_ARE_UTF8 better tested, ideally by defining it for Firefox.

/be

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: How 'bout this then

This seems a little sneaky, sticking something in the char array that strchr won't see, but it fixes the bug.

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 281217 [details] [diff] [review]
How 'bout this then

No, that's the right fix -- data-driven logic should suffice here, no need for more ad-hoc logic.

/be

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Fixed on trunk -- we should now use the \0 escape for this case.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Please land a test?  Should be easy to do as either JS eng test or mochitest...

Comment 9

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_5/decompilation/regress-375639.js,v  <--  regress-375639.js
initial revision: 1.1

Comment 10

[Bob Clary [:bc] (inactive)]

verified fixed 1.9.0 linux|mac|win