"zlib.createGunzip()._transform" results in an abort

[zyscoder]

• Version: v14.15.1
• Platform: Linux 5.8.0-38-generic The binary and long term compatibility with node #43~20.04.1-Ubuntu SMP Tue Jan 12 16:39:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: zlib.createGunzip

What steps will reproduce the bug?

Setup a node instance,

» node

and run the following javascript code.

zlib.createGunzip()._transform("")

Then an abort occurs.

How often does it reproduce? Is there a required condition?

This abort can always be triggered following the steps above.

What is the expected behavior?

If any error occurs, an exception or other similar error-reporting stuff should be thrown. There is no reason to abort the whole node process.

What do you see instead?

» node
Welcome to Node.js v14.15.1.
Type ".help" for more information.
> zlib.createGunzip()._transform("")
node[180922]: ../src/node_zlib.cc:316:static void node::{anonymous}::CompressionStream<CompressionContext>::Write(const v8::FunctionCallbackInfo<v8::Value>&) [with bool async = true; CompressionContext = node::{anonymous}::ZlibContext]: Assertion `Buffer::HasInstance(args[1])' failed.
 1: 0xa03530 node::Abort() [node]
 2: 0xa035ae  [node]
 3: 0xac2baa  [node]
 4: 0xbe369b  [node]
 5: 0xbe4c46  [node]
 6: 0xbe52c6 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [node]
 7: 0x13ff259  [node]
[1]    180922 abort (core dumped)  node

Additional information

[addaleax]

Generally speaking, you should not call the internals of Node.js API objects and expect good results. _transform is a method that you should not call from other code, as noted by the streams docs.

So while this is fixable, I don't think it's necessarily worth doing so.

[zyscoder]

I'm confused since the docs mentioned that application code must not invoke this kind of APIs, why not declare them as private fields (or something like that)? BTW, zlib.createGunzip()._write("") can also trigger this abort. I'm not sure If there any possibility of misusing between write and _write.

[addaleax]

I'm confused since the docs mentioned that application code must not invoke this kind of APIs, why not declare them as private fields (or something like that)?

Because they are supposed to be called by the streams superclasses (and by them only). I think putting them behind e.g. a symbol would make sense, but I’m not sure if that isn’t too big of a change to streams these days.

[jasnell]

I'd like to see these transition into symbols but, by definition the symbols would still need to be exposed so we can't prevent direct calls entirely. We should at least make sure direct calls don't abort and do throw proper errors, however.