1320706 - JSAutoByteString is a footgun that we should stop using, because it mishandles embedded nulls

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

We should certainly stop using it on any data we do not control.  It doesn't keep track of the length of the string it produces, which means that input with embedded nulls will be corrupted.

For sanity's sake, we should just remove it entirely; as long as we leave it, people will reach for it and end up with buggy code.

The other option is to fix it to output a (char, length) pair, of course.

Comment 1

[Tooru Fujisawa [:arai]]

do you mean we just need to return correct length from |length()| method, or remove methods that returns `char*` alone?

Comment 2

[Tooru Fujisawa [:arai]]

if former, we could just add `length` out parameter to some APIs and keep correct length in JSAutoByteString.

if it also means latter, we could use Latin1Chars and UTF8Chars as return type instead of raw `char*`.
maybe we need to separate JSAutoByteString for each encoding, to make it clear which encoding it's returning tho.

Comment 3

[Boris Zbarsky [:bzbarsky]]

The former would be a bare minimum.  The latter might be nicer.

Comment 4

[André Bargull [:anba]]

Some of the JSAutoByteString uses in js/src/builtin/Intl.cpp can be changed to call js::StringEqualsAscii(JSLinearString*, const char*) instead. Actually this will also avoid an additional pod_malloc, so let's just update it to use StringEqualsAscii. (bug 1321771)

Comment 5

[Tooru Fujisawa [:arai]]

bug 1485066 removed JSAutoByteString