Skip to content

Commit

Permalink
buffer: neuter external nullptr buffers
Browse files Browse the repository at this point in the history
Neuter external `nullptr` buffers, otherwise their contents will be
materialized on access, and the buffer instance will be internalized.

This leads to a crash like this:

    v8::ArrayBuffer::Neuter Only externalized ArrayBuffers can be
    neutered

Fix: #3619
PR-URL: #3624
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
indutny authored and rvagg committed Nov 6, 2015
1 parent 7d0b589 commit 5667369
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,11 @@ MaybeLocal<Object> New(Environment* env,
}

Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(), data, length);
// `Neuter()`ing is required here to prevent materialization of the backing
// store in v8. `nullptr` buffers are not writable, so this is semantically
// correct.
if (data == nullptr)
ab->Neuter();
Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
Maybe<bool> mb =
ui->SetPrototype(env->context(), env->buffer_prototype_object());
Expand Down
40 changes: 40 additions & 0 deletions test/addons/null-buffer-neuter/binding.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#include <node.h>
#include <node_buffer.h>
#include <util.h>
#include <v8.h>

static int alive;

static void FreeCallback(char* data, void* hint) {
CHECK_EQ(data, nullptr);
alive--;
}

void Run(const v8::FunctionCallbackInfo<v8::Value>& args) {
v8::Isolate* isolate = args.GetIsolate();
alive++;

{
v8::HandleScope scope(isolate);
v8::Local<v8::Object> buf = node::Buffer::New(
isolate,
nullptr,
0,
FreeCallback,
nullptr).ToLocalChecked();

char* data = node::Buffer::Data(buf);
CHECK_EQ(data, nullptr);
}

isolate->RequestGarbageCollectionForTesting(
v8::Isolate::kFullGarbageCollection);

CHECK_EQ(alive, 0);
}

void init(v8::Local<v8::Object> target) {
NODE_SET_METHOD(target, "run", Run);
}

NODE_MODULE(binding, init);
8 changes: 8 additions & 0 deletions test/addons/null-buffer-neuter/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
'targets': [
{
'target_name': 'binding',
'sources': [ 'binding.cc' ]
}
]
}
7 changes: 7 additions & 0 deletions test/addons/null-buffer-neuter/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';
// Flags: --expose-gc

require('../../common');
var binding = require('./build/Release/binding');

binding.run();

0 comments on commit 5667369

Please sign in to comment.