buffer: don't CHECK on zero-sized realloc

bnoordhuis · jasnell · commit 2a45b72b4af5 · 2015-10-29T08:38:42.000-07:00
malloc(0) and realloc(ptr, 0) have implementation-defined behavior in that the standard allows them to either return a unique pointer or a nullptr for zero-sized allocation requests. Normalize by always using a nullptr. Fixes: #3496 PR-URL: #3499 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -211,18 +211,30 @@ MaybeLocal<Object> New(Isolate* isolate,
                        enum encoding enc) {
   EscapableHandleScope scope(isolate);
 
-  size_t length = StringBytes::Size(isolate, string, enc);
-  char* data = static_cast<char*>(malloc(length));
+  const size_t length = StringBytes::Size(isolate, string, enc);
+  size_t actual = 0;
+  char* data = nullptr;
+
+  // malloc(0) and realloc(ptr, 0) have implementation-defined behavior in
+  // that the standard allows them to either return a unique pointer or a
+  // nullptr for zero-sized allocation requests.  Normalize by always using
+  // a nullptr.
+  if (length > 0) {
+    data = static_cast<char*>(malloc(length));
 
-  if (data == nullptr)
-    return Local<Object>();
+    if (data == nullptr)
+      return Local<Object>();
 
-  size_t actual = StringBytes::Write(isolate, data, length, string, enc);
-  CHECK(actual <= length);
+    actual = StringBytes::Write(isolate, data, length, string, enc);
+    CHECK(actual <= length);
 
-  if (actual < length) {
-    data = static_cast<char*>(realloc(data, actual));
-    CHECK_NE(data, nullptr);
+    if (actual == 0) {
+      free(data);
+      data = nullptr;
+    } else if (actual < length) {
+      data = static_cast<char*>(realloc(data, actual));
+      CHECK_NE(data, nullptr);
+    }
   }
 
   Local<Object> buf;
diff --git a/test/parallel/test-buffer.js b/test/parallel/test-buffer.js
@@ -550,6 +550,9 @@ for (var i = 0; i < segments.length; ++i) {
 }
 assert.equal(b.toString('binary', 0, pos), 'Madness?! This is node.js!');
 
+// Regression test for https://github.com/nodejs/node/issues/3496.
+assert.equal(Buffer('=bad'.repeat(1e4), 'base64').length, 0);
+
 // Creating buffers larger than pool size.
 var l = Buffer.poolSize + 5;
 var s = '';

Original file line number	Diff line number	Diff line change
`@@ -550,6 +550,9 @@ for (var i = 0; i < segments.length; ++i) {`
`550`	`550`	`}`
`551`	`551`	`assert.equal(b.toString('binary', 0, pos), 'Madness?! This is node.js!');`
`552`	`552`
	`553`	`+// Regression test for https://github.com/nodejs/node/issues/3496.`
	`554`	`+assert.equal(Buffer('=bad'.repeat(1e4), 'base64').length, 0);`
	`555`	`+`
`553`	`556`	`// Creating buffers larger than pool size.`
`554`	`557`	`var l = Buffer.poolSize + 5;`
`555`	`558`	`var s = '';`