From 8b431ad84ce2be095d904b17ae48c04e483e3211 Mon Sep 17 00:00:00 2001 From: Chi-chi Wang Date: Fri, 12 Oct 2018 13:32:55 -0500 Subject: [PATCH 1/3] src: remove function hasTextDecoder in encoding.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit also... return TextDecoder directly from factories PR-URL: https://github.com/nodejs/node/pull/23625 Reviewed-By: Anna Henningsen Reviewed-By: Сковорода Никита Андреевич Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Sakthipriyan Vairamani --- lib/internal/encoding.js | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/lib/internal/encoding.js b/lib/internal/encoding.js index 161ae72a19eaef..c0083948e0d72d 100644 --- a/lib/internal/encoding.js +++ b/lib/internal/encoding.js @@ -341,22 +341,16 @@ Object.defineProperties( value: 'TextEncoder' } }); -const { hasConverter, TextDecoder } = +const TextDecoder = process.binding('config').hasIntl ? makeTextDecoderICU() : makeTextDecoderJS(); -function hasTextDecoder(encoding = 'utf-8') { - validateArgument(encoding, 'string', 'encoding', 'string'); - return hasConverter(getEncodingFromLabel(encoding)); -} - function makeTextDecoderICU() { const { decode: _decode, getConverter, - hasConverter - } = process.binding('icu'); + } = internalBinding('icu'); class TextDecoder { constructor(encoding = 'utf-8', options = {}) { @@ -409,7 +403,7 @@ function makeTextDecoderICU() { } } - return { hasConverter, TextDecoder }; + return TextDecoder; } function makeTextDecoderJS() { @@ -497,7 +491,7 @@ function makeTextDecoderJS() { } } - return { hasConverter, TextDecoder }; + return TextDecoder; } // Mix in some shared properties. @@ -552,7 +546,6 @@ function makeTextDecoderJS() { module.exports = { getEncodingFromLabel, - hasTextDecoder, TextDecoder, TextEncoder }; From 72ffab8bcbbd158a852446c4a6b1fd6ad6eeb460 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Thu, 25 Oct 2018 09:42:36 -0400 Subject: [PATCH 2/3] Revert "src: remove calls to deprecated v8 functions (IntegerValue)" This reverts commit 2555cb4a4049dc4c41d8a2f4ce50909cc0a12a4a. --- src/node.cc | 10 +++++----- src/node_buffer.cc | 9 ++++----- src/node_crypto.cc | 9 ++++----- src/node_http_parser.cc | 8 ++------ src/process_wrap.cc | 5 ++--- src/tcp_wrap.cc | 5 +---- 6 files changed, 18 insertions(+), 28 deletions(-) diff --git a/src/node.cc b/src/node.cc index 7e8738080b3c5e..5d22631c13df76 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2217,10 +2217,11 @@ void DebugProcess(const FunctionCallbackInfo& args) { return env->ThrowError("Invalid number of arguments."); } - CHECK(args[0]->IsNumber()); - pid_t pid = args[0].As()->Value(); - int r = kill(pid, SIGUSR1); + pid_t pid; + int r; + pid = args[0]->IntegerValue(); + r = kill(pid, SIGUSR1); if (r != 0) { return env->ThrowErrnoException(errno, "kill"); } @@ -2262,8 +2263,7 @@ static void DebugProcess(const FunctionCallbackInfo& args) { CloseHandle(mapping); }); - CHECK(args[0]->IsNumber()); - pid = args[0].As()->Value(); + pid = (DWORD) args[0]->IntegerValue(); process = OpenProcess(PROCESS_CREATE_THREAD | PROCESS_QUERY_INFORMATION | PROCESS_VM_OPERATION | PROCESS_VM_WRITE | diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 8642477db4d927..2a659589737444 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -170,8 +170,7 @@ inline MUST_USE_RESULT bool ParseArrayIndex(Local arg, return true; } - CHECK(arg->IsNumber()); - int64_t tmp_i = arg.As()->Value(); + int64_t tmp_i = arg->IntegerValue(); if (tmp_i < 0) return false; @@ -772,7 +771,7 @@ void IndexOfString(const FunctionCallbackInfo& args) { SPREAD_BUFFER_ARG(args[0], ts_obj); Local needle = args[1].As(); - int64_t offset_i64 = args[2].As()->Value(); + int64_t offset_i64 = args[2]->IntegerValue(); bool is_forward = args[4]->IsTrue(); const char* haystack = ts_obj_data; @@ -888,7 +887,7 @@ void IndexOfBuffer(const FunctionCallbackInfo& args) { THROW_AND_RETURN_UNLESS_BUFFER(Environment::GetCurrent(args), args[1]); SPREAD_BUFFER_ARG(args[0], ts_obj); SPREAD_BUFFER_ARG(args[1], buf); - int64_t offset_i64 = args[2].As()->Value(); + int64_t offset_i64 = args[2]->IntegerValue(); bool is_forward = args[4]->IsTrue(); const char* haystack = ts_obj_data; @@ -958,7 +957,7 @@ void IndexOfNumber(const FunctionCallbackInfo& args) { SPREAD_BUFFER_ARG(args[0], ts_obj); uint32_t needle = args[1].As()->Value(); - int64_t offset_i64 = args[2].As()->Value(); + int64_t offset_i64 = args[2]->IntegerValue(); bool is_forward = args[3]->IsTrue(); int64_t opt_offset = IndexOfOffset(ts_obj_length, offset_i64, 1, is_forward); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 7d9294d65a53b6..46e0dadf3a4d35 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -977,16 +977,15 @@ void SecureContext::SetDHParam(const FunctionCallbackInfo& args) { void SecureContext::SetOptions(const FunctionCallbackInfo& args) { SecureContext* sc; ASSIGN_OR_RETURN_UNWRAP(&sc, args.Holder()); - int64_t val; - if (args.Length() != 1 || - !args[0]->IntegerValue(args.GetIsolate()->GetCurrentContext()).To(&val)) { + if (args.Length() != 1 || !args[0]->IntegerValue()) { return THROW_ERR_INVALID_ARG_TYPE( sc->env(), "Options must be an integer value"); } - SSL_CTX_set_options(sc->ctx_.get(), - static_cast(val)); // NOLINT(runtime/int) + SSL_CTX_set_options( + sc->ctx_.get(), + static_cast(args[0]->IntegerValue())); // NOLINT(runtime/int) } diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 771189d0904359..3551d70af74144 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -289,16 +289,12 @@ class Parser : public AsyncWrap, public StreamListener { MaybeLocal head_response = MakeCallback(cb.As(), arraysize(argv), argv); - int64_t val; - - if (head_response.IsEmpty() || !head_response.ToLocalChecked() - ->IntegerValue(env()->context()) - .To(&val)) { + if (head_response.IsEmpty()) { got_exception_ = true; return -1; } - return val; + return head_response.ToLocalChecked()->IntegerValue(); } diff --git a/src/process_wrap.cc b/src/process_wrap.cc index cd443b34aa972c..72af8635f959c2 100644 --- a/src/process_wrap.cc +++ b/src/process_wrap.cc @@ -128,9 +128,8 @@ class ProcessWrap : public HandleWrap { options->stdio[i].data.stream = StreamForWrap(env, stdio); } else { Local fd_key = env->fd_string(); - Local fd_value = stdio->Get(context, fd_key).ToLocalChecked(); - CHECK(fd_value->IsNumber()); - int fd = static_cast(fd_value.As()->Value()); + int fd = static_cast( + stdio->Get(context, fd_key).ToLocalChecked()->IntegerValue()); options->stdio[i].flags = UV_INHERIT_FD; options->stdio[i].data.fd = fd; } diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index a07ba2d61231f4..589e5c24cb9493 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -207,10 +207,7 @@ void TCPWrap::Open(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder(), args.GetReturnValue().Set(UV_EBADF)); - int64_t val; - if (!args[0]->IntegerValue(args.GetIsolate()->GetCurrentContext()).To(&val)) - return; - int fd = static_cast(val); + int fd = static_cast(args[0]->IntegerValue()); int err = uv_tcp_open(&wrap->handle_, fd); if (err == 0) From d39da714cf1d3ebdb224d5366dd4bda231059b86 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 21 Oct 2018 00:44:48 -0400 Subject: [PATCH 3/3] buffer: test fix crash for invalid index types --- test/parallel/test-buffer-copy.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/parallel/test-buffer-copy.js b/test/parallel/test-buffer-copy.js index 8ede5101463b05..7ede479d09d37b 100644 --- a/test/parallel/test-buffer-copy.js +++ b/test/parallel/test-buffer-copy.js @@ -18,6 +18,17 @@ let cntr = 0; } } +{ + // Current behavior is to coerce values to integers. + b.fill(++cntr); + c.fill(++cntr); + const copied = b.copy(c, '0', '0', '512'); + assert.strictEqual(copied, 512); + for (let i = 0; i < c.length; i++) { + assert.strictEqual(c[i], b[i]); + } +} + { // copy c into b, without specifying sourceEnd b.fill(++cntr); @@ -144,3 +155,13 @@ assert.strictEqual(b.copy(c, 512, 0, 10), 0); assert.strictEqual(c[i], e[i]); } } + +// https://github.com/nodejs/node/issues/23668: Do not crash for invalid input. +c.fill('c'); +b.copy(c, 'not a valid offset'); +// Make sure this acted like a regular copy with `0` offset. +assert.deepStrictEqual(c, b.slice(0, c.length)); + +assert.throws(() => { + b.copy(c, { [Symbol.toPrimitive]() { throw new Error('foo'); } }); +}, /foo/);