From 827ee498e332e316421f565c5f763f347f81274c Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor@indutny.com>
Date: Sun, 1 Nov 2015 19:34:19 -0500
Subject: [PATCH] buffer: neuter external `nullptr` buffers

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: https://github.com/nodejs/node/pull/3624
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
---
 src/node_buffer.cc                         |  5 +++
 test/addons/null-buffer-neuter/binding.cc  | 40 ++++++++++++++++++++++
 test/addons/null-buffer-neuter/binding.gyp |  8 +++++
 test/addons/null-buffer-neuter/test.js     |  7 ++++
 4 files changed, 60 insertions(+)
 create mode 100644 test/addons/null-buffer-neuter/binding.cc
 create mode 100644 test/addons/null-buffer-neuter/binding.gyp
 create mode 100644 test/addons/null-buffer-neuter/test.js

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index 7d428b2b13..e4af4909ff 100644
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -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());
diff --git a/test/addons/null-buffer-neuter/binding.cc b/test/addons/null-buffer-neuter/binding.cc
new file mode 100644
index 0000000000..da75919011
--- /dev/null
+++ b/test/addons/null-buffer-neuter/binding.cc
@@ -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);
diff --git a/test/addons/null-buffer-neuter/binding.gyp b/test/addons/null-buffer-neuter/binding.gyp
new file mode 100644
index 0000000000..3bfb84493f
--- /dev/null
+++ b/test/addons/null-buffer-neuter/binding.gyp
@@ -0,0 +1,8 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'sources': [ 'binding.cc' ]
+    }
+  ]
+}
diff --git a/test/addons/null-buffer-neuter/test.js b/test/addons/null-buffer-neuter/test.js
new file mode 100644
index 0000000000..bca76e27af
--- /dev/null
+++ b/test/addons/null-buffer-neuter/test.js
@@ -0,0 +1,7 @@
+'use strict';
+// Flags: --expose-gc
+
+require('../../common');
+var binding = require('./build/Release/binding');
+
+binding.run();