Node: backtrace truncated if msg contains null character `\0`

[timotheecour]

backtrace truncated if msg contains null character \0

$ node main.js

abcdef
/main/t02.js:6
    throw new Error(a);
    ^

Error: abc

with this program

function test(){
  var a = "abc\0def"
  var a2 = "abcdef"
  console.log(a); // not truncated
  if(true) // change to false and backtrace works
    throw new Error(a); // backtrace msg truncated at `\0`, the whole stack frames are missing
  else
    throw new Error(a2); // backtrace shown correctly
}

test()

#

without the \0 the backtrace shows fine:

abcdef
/main/t02.js:8
    throw new Error(a2);
    ^

Error: abcdef
    at test (/main/t02.js:8:11)
    at Object.<anonymous> (/main/t02.js:11:1)
    at Module._compile (internal/modules/cjs/loader.js:774:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:785:10)
    at Module.load (internal/modules/cjs/loader.js:641:32)
    at Function.Module._load (internal/modules/cjs/loader.js:556:12)
    at Function.Module.runMain (internal/modules/cjs/loader.js:837:10)
    at internal/main/run_main_module.js:17:11

#

node -v
v12.3.1

uname -a
Darwin 18.5.0 Darwin Kernel Version 18.5.0

note

according to https://stackoverflow.com/questions/13698677/null-character-in-strings

a NUL byte should simply be "yet another character value" and have no special meaning, as opposed to other languages where it might end a SV (String value).

console.log correctly shows the string (no truncation) but the backtrace doesn't work when running node on cmd line.

note that on a browser (eg chrome) it works: no truncation:

throw new Error("abc\0def");
VM9405:1 Uncaught Error: abcdef
    at <anonymous>:1:7

[joyeecheung]

I guess the question is:

1. Do we want to escape special characters in err.message?
2. If so, do we want to just special case for \0, or include any other special characters? (off the top of my head, we would not want to escape \n), or make that configuration somehow?

[addaleax]

I’d just print the error-message as-is, but including the \0 character.

[joyeecheung]

BTW chrome prints this by ignoring \0.

Do we want to special case for BOM or invalid surrogate characters as well? I guess whatever we do we should try to be consistent.

[joyeecheung]

The OP says:

throw new Error("abc\0def");
VM9405:1 Uncaught Error: abc�def
at :1:7

On the latest chrome canary I get this though

[addaleax]

@joyeecheung That seems like a much more widely scoped issue than this bug for me … the problem here is, afaict, we convert the error information from JS strings to UTF-8 like we always do, but then use snprintf and fprintf to prepare/print out the bytes that we actually print, and those functions special-case \0 as the end-of-string. It’s just that we’re not keeping track of the string length while doing so…

We can think about escaping characters or introducing special handling for some, but that seems more like a feature request and isn’t about correct vs incorrect behaviour?

[timotheecour]

we should preserve the '\0' without escaping, it makes it easier to debug in case user tries to see why there is a null char (in my case it was from here: https://github.com/nim-lang/Nim/issues/11788)

also, that's what console.log does: it preserves the '\0'

but then use snprintf and fprintf to prepare/print out the bytes

solution: use fwrite instead of snprintf
https://stackoverflow.com/questions/6943928/show-special-characters-in-unix-while-using-less-command

If you know the length of your string as n characters, you can output it using fwrite:

if (n && fwrite(str, 1, n, stdout) != n) {
    /* Error handling. */
}

[joyeecheung]

preserve the '\0' without escaping

To preserve it in the output we need to escape it (replace it with \\0).

it makes it easier to debug in case user tries to see why there is a null char

Doesn't that also apply to \n, \r and \t?

[joyeecheung]

There are three handling that I can think of:

1. Escape \0 in err.message (while not escaping \n, \r and friends)
2. Ignore \0 in err.message - what browsers and console.log do
3. Make that configurable? (along with \n, \r and friends)

[addaleax]

preserve the '\0' without escaping

To preserve it in the output we need to escape it (replace it with \\0).

@joyeecheung Can you explain why we need to escape it? Why is it not enough to just print the \0 character as-is, even if most terminals won’t display it (basically, like @timotheecour said, act like console.log() here)?

[joyeecheung]

@addaleax By print the \0 character as-is do you mean we pass the original string with \0 into snprintf/fprintf? I think that is why they are truncated?

[addaleax]

@joyeecheung Yes, that’s why they are currently truncated as well – What I’m having in mind is that we shouldn’t use the standard C snprintf / fprintf functions for these strings, and instead use maybe a more C++-y equivalent of snprintf that can deal with e.g. std::strings, and for writing to stderr something like fwrite as suggested by @timotheecour.

[joyeecheung]

@addaleax I see, I thought the request was to print new Error('test\0test') as literally test\0test, sorry about the confusion!

In that case I think we just need to stop doing variadic arguments in PrintErrorString as that's the source of the C-style interpretation of the strings

[timotheecour]

@addaleax

maybe a more C++-y equivalent of snprintf that can deal with e.g. std::strings, and for writing to stderr something like fwrite as suggested by @timotheecour.

unless you're using any format string arguments fwrite should be enough (but maybe I'm missing something?)

[addaleax]

@timotheecour You can look around for fprintf, snprintf and PrintErrorString in src/node_errors.cc, there’s quite a few of them and none of them handle \0 characters well. (And at least theoretically, it’s possible for them to show up as part of the filename or a function name as well.)

[Himself65]

so, how we count the length of the string if it contains '\0'?

[addaleax]

@himself65 For JS strings (and C++ std::strings), the length is tracked separately from the string’s contents

[Himself65]

oh, I get it. It must be a huge task to refactor this part I think

[addaleax]

@Himself65 Yeah, it’s quite a bit of work but I’m up for it – it’s not hard, it’s pretty straightforward, just a lot to do.