Terminate on \0 in data passed to String::New()

Terminate on \0 in data passed to String::New()


Remove \0 chars from String::New fuzz-test data

If strings are made to terminate on null characters in the data provided
to String::New(), then the Traverse tests will fail because 0 is part of
the fuzz-testing data.

This change makes it so that they don't try to put null bytes in the
test data.

Failing test to demonstrate null-handling behavior

There are many cases where you have a bunch of bytes which are
null-padded or in some other way represent a null-terminated ASCII or
UTF-8 string.

Even if a number of bytes are provided, a null character should always
terminate the string.

[Mads Ager (chromium), 2011-02-16 09:30:12]

Thanks for you patch.

Null characters are perfectly legal in JavaScript strings and they do not
terminate the strings:


V8 version 3.1.4 (candidate)
> var s = "\u0000"
> s.length
1
> s.charCodeAt(0)
0
> 

The API call is doing the right thing by putting the null characters in the
resulting string and honoring the length parameter. As an embedder you should
pass in the right length.

[isaacs, 2011-02-16 17:08:29]

I agree that "\u0000" is a valid JavaScript string.  However, "x\0\0\0" is a C
string of length 1, not length 4, and the "\0" codepoint should be encoded in
UTF-8 as "\xC0\x80".

However, this patch actually doesn't prevent v8 from handling JavaScript strings
with \0 characters in them.

$ ./shell
V8 version 3.1.5 (candidate)
> var s = "asdf\u0000foo\0bar"
> s.length
12
> JSON.stringify(s)
"asdf\u0000foo\u0000bar"

It only terminates byte arrays on null bytes when interpreted as ASCII or UTF-8.
 Since a null byte is not valid ASCII or UTF-8, and is traditionally reserved as
a terminator in those encodings, this seems like it should work this way.

Are there indeed cases where ASCII or UTF-8 data is encoded in an array of
bytes, and one wants to include null characters in the resulting string?  That
would be surprising to me, but if you say so, I believe you.

If so, it seems a bit unfortunate to have to scan through the bytes twice.  If
I'm sure that I want to terminate the byte array on any nulls, how would you
feel about a modification to the API to enable setting a maximum length rather
than an explicit length?

[ry, 2011-02-16 19:40:41]

Mads,

The issue is that String::New(char*, size_t) is meant to read a UTF8 encoded
byte array. In UTF8 the null character terminates the sequence.

To achieve the javascript string "\u0000" from a UTF8 sequence the user should
do String::New("\xC0\x80", 2); or String::New("\xC0\x80\0", 3);

[Lasse Reichstein, 2011-02-17 09:18:26]

Can you give a reference for null-bytes not being valid utf-8?

The way I read the Unicode standard (ISO-10646) on UTF-8, all code points in the
range 0000 through 007f are encoded as one byte. The octet sequence \xC0\x80 is,
as far as I can see, *invalid* UTF-8 (0xC0 and 0xC1 cannot occur in valid
UTF-8). It's used as a hack in some cases where people know that it'll be
interpreted as the null code-point anyway, but only because the decoder doesn't
validate its input.
(See: http://en.wikipedia.org/wiki/UTF-8#Invalid_byte_sequences)

Where I see people talk about null-terminated UTF-8 strings, it's typically in
contexts where a null character would not make sense in the string.

I think the current function, using start and length of the UTF-8 byte sequence,
should be retained.
If nothing else, changing the function in this way can break existing code that
uses it. We don't want that.

If null-terminated UTF-8 strings are common, it would be better to add a
separate function to create JS strings from those.

If you are creating the UTF-8 byte sequences yourself, you are likely to already
know where it ends, and it's just a matter of not throwing that information
away. If you are using a library that returns null-terminated UTF-8 sequences,
then it's obviously not as simple.

[isaacs, 2011-02-17 16:56:33]

So, \0 is valid UTF-8, but \xC0\x80 is the encoding for the 0 code-point in
"Modified UTF-8", which is backwards compatible with null-terminated c-strings.

It's interesting to me that v8 *does* terminate on \0 if a length is not given,
no matter what the bytes are.

> If null-terminated UTF-8 strings are common, it would be better to add a
> separate function to create JS strings from those.

That would be very agreeable.  Would it be better to add an optional flag?  It
seems like it's not much of a change in behavior.

> If you are creating the UTF-8 byte sequences yourself, you are likely to
already
> know where it ends, and it's just a matter of not throwing that information
> away. If you are using a library that returns null-terminated UTF-8 sequences,
> then it's obviously not as simple.

Here's a bit more information about a use case that prompted this.  (It's worse
than a library.)

Node's "Buffer" objects represent a block of bytes outside of v8's heap.  The
Buffer::ToString method takes the block of bytes, and passes it to v8's
String::New method.  The length of the buffer is known, and is provided to
String::New.  However, the length of the string it contains might *not* be
known.

For example, say that you are writing a client for a binary protocol.  Part of
the message contains a block of data that is 10 bytes long, which contains a
string that is between 1 and 10 characters, terminated with a \0 byte if it is
less than 10 bytes long.  Anything between the first \0 and the 10th byte is
considered garbage, and if no \0 byte is found, then the string is 10 bytes
long.

It makes sense to allocate a 10-byte buffer to read in this block, since you
know that it will be 10 bytes of the message.  It would be disastrous to rely on
the string ending in \0, since the 11th byte is the start of some *other* part
of the message (or another Buffer, or maybe just some random spot in memory). 
So, we must provide a length.  However, if there are only 5 characters in this
block, you only really want a 5-byte string.

The options seem to be:

1. Scan the bytes twice.  Once to get the length, and a second time in v8 to
convert them to a string.  This seems suboptimal.
2. Tell v8 to terminate on \0 bytes in String::New.