[18.8.0] char* node::Buffer::Data(v8::Local<v8::Object> val) returns a NULL pointer when given a valid v8::Object

[gierschv]

Version

18.8.0

Platform

macOS Monterey 12.4

Subsystem

No response

What steps will reproduce the bug?

Try to initialise a lame Encoder using this lib : https://github.com/FlatIO/node-lame (check Example : https://github.com/FlatIO/node-lame#example)

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

Everytime

What is the expected behavior?

node::Buffer::Data() returns a valid pointer when a valid v8::Object is provided.
Then, the lame Encoder is initialized without any issue.

What do you see instead?

node::Buffer::Data() returns a NULL pointer.
The lame encoder returns this error: “error setting prop “channels”: -1"

Additional information

This was working fine with node v18.7.0 and all the previous versions so this is very probably an issue with Node.
I did some investigation and found out that it was caused by a NULL pointer returned by node::Buffer::Data() when provided with a seemingly valid v8::Object
I tested the v8::Object with ->IsObject(), it returned true, and with ->IsNullOrUndefined() and it returned false;
The v8::Object (GFP) represents Lame global flags, and node::Buffer::Data is used to unwrap the pointer after sending it through a JS binding

[targos]

/cc @nodejs/cpp-reviewers @kvakil. This seems caused by 8c35a4e#diff-81f77f34c4f5b8cc35f1a506808b94e2a281f3341c0f1a38ef2b8a8ba730b22bL247 ?

[bnoordhuis]

I suspect the problem is here: it calls Nan::NewBuffer() with length == 0.

Maybe that worked before but it's basically saying "this is an empty buffer" and those don't have to point to anything.

[kvakil]

Thanks @targos for the bisect. I've confirmed Ben's comment that this is an edge case with zero-length buffers: if you bump the "length" in WrapPointer from 0 to 1, then everything works. I am a little ambivalent about fixing the issue, but a codesearch did reveal several repos which are using this pattern, so I think we should just revert.

A fix is available here; #44579