r.js crash

[neumino]

This query throws an error (on next)

r.js("String.fromCharCode(100000000000000000000)")

RqlRuntimeError: Javascript query `String.fromCharCode(100000000000000000000)` caused a crash in a worker process. in:
r.js("String.fromCharCode(100000000000000000000)")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

And I get in the shell where I started rethinkdb

src/tcmalloc.cc:390] Attempt to free invalid pointer: 0x7fff4e003618

Node.js returns the char \u0000 when I give it String.fromCharCode(100000000000000000000)

[coffeemug]

Pinging @Tryneus.

[Tryneus]

Looking into this. I get a crash if I just do r.js('String.fromCharCode(0)').

[danielmewes]

cJSON has problems with strings containing null characters. Don't know if that could be involved here...

[danielmewes]

... and it is also not a valid JSON string in general, so we should probably check for it somewhere and trigger an error (even if we could technically send it over to the client).

[Tryneus]

In this case, it doesn't actually touch any JSON code, it's somewhere in the v8::String -> std::string -> counted_t<ql::datum_t> conversions that this happens.

[Tryneus]

Oh, well, apparently, we don't allow ql::datum_ts to have a string with an internal null character, which is a problem. In addition, when the datum_t constructor throws, it causes this problem (possibly due to some interaction with std::move, will investigate more).

[Tryneus]

Ok, this has been fixed and put up in code review 1025. Just had to catch the exception and set the error message.

[Tryneus]

The fix has been approved and merged to next in commit 167d954, and cherry-picked into v1.10.x in commit 3271cf5 (in case we do another point release). Will be in release 1.10.2, or 1.11, whichever comes first.