From: Karl Skomski Date: Sun, 16 Aug 2015 14:09:02 +0000 (+0200) Subject: src: fix memory leak in ExternString X-Git-Tag: v4.0.0~41 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5201cb0ff188fd68ba66fcc75489379c8dd1d274;p=platform%2Fupstream%2Fnodejs.git src: fix memory leak in ExternString v8 will silently return an empty handle which doesn't delete our data if string length is above String::kMaxLength Fixes: https://github.com/nodejs/node/issues/1374 PR-URL: https://github.com/nodejs/node/pull/2402 Reviewed-By: trevnorris - Trevor Norris Reviewed-By: indutny - Fedor Indutny Reviewed-By: bnoordhuis - Ben Noordhuis Amended by @rvagg to change author date from "1970-08-16 16:09:02 +0200" to "2015-08-16 16:09:02 +0200" as per discussion @ https://github.com/nodejs/node/issues/2713 --- diff --git a/lib/buffer.js b/lib/buffer.js index 4166724..1393300 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -350,10 +350,14 @@ function slowToString(encoding, start, end) { Buffer.prototype.toString = function() { - const length = this.length | 0; - if (arguments.length === 0) - return this.utf8Slice(0, length); - return slowToString.apply(this, arguments); + if (arguments.length === 0) { + var result = this.utf8Slice(0, this.length); + } else { + var result = slowToString.apply(this, arguments); + } + if (result === undefined) + throw new Error('toString failed'); + return result; }; diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 54c4711..3eceae5 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -1024,6 +1024,10 @@ void Initialize(Handle target, target->Set(env->context(), FIXED_ONE_BYTE_STRING(env->isolate(), "kMaxLength"), Integer::NewFromUnsigned(env->isolate(), kMaxLength)).FromJust(); + + target->Set(env->context(), + FIXED_ONE_BYTE_STRING(env->isolate(), "kStringMaxLength"), + Integer::New(env->isolate(), String::kMaxLength)).FromJust(); } diff --git a/src/string_bytes.cc b/src/string_bytes.cc index 3d568c6..7fc03c0 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -22,13 +22,14 @@ using v8::Local; using v8::Object; using v8::String; using v8::Value; +using v8::MaybeLocal; template class ExternString: public ResourceType { public: ~ExternString() override { - delete[] data_; + free(const_cast(data_)); isolate()->AdjustAmountOfExternalAllocatedMemory(-byte_length()); } @@ -52,7 +53,11 @@ class ExternString: public ResourceType { if (length == 0) return scope.Escape(String::Empty(isolate)); - TypeName* new_data = new TypeName[length]; + TypeName* new_data = + static_cast(malloc(length * sizeof(*new_data))); + if (new_data == nullptr) { + return Local(); + } memcpy(new_data, data, length * sizeof(*new_data)); return scope.Escape(ExternString::New(isolate, @@ -72,10 +77,15 @@ class ExternString: public ResourceType { ExternString* h_str = new ExternString(isolate, data, length); - Local str = String::NewExternal(isolate, h_str); + MaybeLocal str = String::NewExternal(isolate, h_str); isolate->AdjustAmountOfExternalAllocatedMemory(h_str->byte_length()); - return scope.Escape(str); + if (str.IsEmpty()) { + delete h_str; + return Local(); + } + + return scope.Escape(str.ToLocalChecked()); } inline Isolate* isolate() const { return isolate_; } @@ -765,11 +775,14 @@ Local StringBytes::Encode(Isolate* isolate, case ASCII: if (contains_non_ascii(buf, buflen)) { - char* out = new char[buflen]; + char* out = static_cast(malloc(buflen)); + if (out == nullptr) { + return Local(); + } force_ascii(buf, out, buflen); if (buflen < EXTERN_APEX) { val = OneByteString(isolate, out, buflen); - delete[] out; + free(out); } else { val = ExternOneByteString::New(isolate, out, buflen); } @@ -797,14 +810,17 @@ Local StringBytes::Encode(Isolate* isolate, case BASE64: { size_t dlen = base64_encoded_size(buflen); - char* dst = new char[dlen]; + char* dst = static_cast(malloc(dlen)); + if (dst == nullptr) { + return Local(); + } size_t written = base64_encode(buf, buflen, dst, dlen); CHECK_EQ(written, dlen); if (dlen < EXTERN_APEX) { val = OneByteString(isolate, dst, dlen); - delete[] dst; + free(dst); } else { val = ExternOneByteString::New(isolate, dst, dlen); } @@ -813,13 +829,16 @@ Local StringBytes::Encode(Isolate* isolate, case HEX: { size_t dlen = buflen * 2; - char* dst = new char[dlen]; + char* dst = static_cast(malloc(dlen)); + if (dst == nullptr) { + return Local(); + } size_t written = hex_encode(buf, buflen, dst, dlen); CHECK_EQ(written, dlen); if (dlen < EXTERN_APEX) { val = OneByteString(isolate, dst, dlen); - delete[] dst; + free(dst); } else { val = ExternOneByteString::New(isolate, dst, dlen); } diff --git a/test/parallel/test-stringbytes-external.js b/test/parallel/test-stringbytes-external.js index ba3f0c5..be0c90d 100644 --- a/test/parallel/test-stringbytes-external.js +++ b/test/parallel/test-stringbytes-external.js @@ -107,3 +107,69 @@ var PRE_3OF4_APEX = Math.ceil((EXTERN_APEX / 4) * 3) - RADIOS; assert.equal(a, b); assert.equal(b, c); })(); + +// v8 fails silently if string length > v8::String::kMaxLength +(function() { + // v8::String::kMaxLength defined in v8.h + const kStringMaxLength = process.binding('buffer').kStringMaxLength; + + assert.throws(function() { + new Buffer(kStringMaxLength + 1).toString(); + }, /toString failed|Buffer allocation failed/); + + try { + new Buffer(kStringMaxLength * 4); + } catch(e) { + assert.equal(e.message, 'Buffer allocation failed - process out of memory'); + console.log( + '1..0 # Skipped: intensive toString tests due to memory confinements'); + return; + } + + assert.throws(function() { + new Buffer(kStringMaxLength + 1).toString('ascii'); + }, /toString failed/); + + assert.throws(function() { + new Buffer(kStringMaxLength + 1).toString('utf8'); + }, /toString failed/); + + assert.throws(function() { + new Buffer(kStringMaxLength * 2 + 2).toString('utf16le'); + }, /toString failed/); + + assert.throws(function() { + new Buffer(kStringMaxLength + 1).toString('binary'); + }, /toString failed/); + + assert.throws(function() { + new Buffer(kStringMaxLength + 1).toString('base64'); + }, /toString failed/); + + assert.throws(function() { + new Buffer(kStringMaxLength + 1).toString('hex'); + }, /toString failed/); + + var maxString = new Buffer(kStringMaxLength).toString(); + assert.equal(maxString.length, kStringMaxLength); + // Free the memory early instead of at the end of the next assignment + maxString = undefined; + + maxString = new Buffer(kStringMaxLength).toString('binary'); + assert.equal(maxString.length, kStringMaxLength); + maxString = undefined; + + maxString = + new Buffer(kStringMaxLength + 1).toString('binary', 1); + assert.equal(maxString.length, kStringMaxLength); + maxString = undefined; + + maxString = + new Buffer(kStringMaxLength + 1).toString('binary', 0, kStringMaxLength); + assert.equal(maxString.length, kStringMaxLength); + maxString = undefined; + + maxString = new Buffer(kStringMaxLength + 2).toString('utf16le'); + assert.equal(maxString.length, (kStringMaxLength + 2) / 2); + maxString = undefined; +})();