Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v10.x] Revert buffer crash #23875

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions lib/internal/encoding.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}) {
Expand Down Expand Up @@ -409,7 +403,7 @@ function makeTextDecoderICU() {
}
}

return { hasConverter, TextDecoder };
return TextDecoder;
}

function makeTextDecoderJS() {
Expand Down Expand Up @@ -497,7 +491,7 @@ function makeTextDecoderJS() {
}
}

return { hasConverter, TextDecoder };
return TextDecoder;
}

// Mix in some shared properties.
Expand Down Expand Up @@ -552,7 +546,6 @@ function makeTextDecoderJS() {

module.exports = {
getEncodingFromLabel,
hasTextDecoder,
TextDecoder,
TextEncoder
};
10 changes: 5 additions & 5 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2217,10 +2217,11 @@ void DebugProcess(const FunctionCallbackInfo<Value>& args) {
return env->ThrowError("Invalid number of arguments.");
}

CHECK(args[0]->IsNumber());
pid_t pid = args[0].As<Integer>()->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");
}
Expand Down Expand Up @@ -2262,8 +2263,7 @@ static void DebugProcess(const FunctionCallbackInfo<Value>& args) {
CloseHandle(mapping);
});

CHECK(args[0]->IsNumber());
pid = args[0].As<Integer>()->Value();
pid = (DWORD) args[0]->IntegerValue();

process = OpenProcess(PROCESS_CREATE_THREAD | PROCESS_QUERY_INFORMATION |
PROCESS_VM_OPERATION | PROCESS_VM_WRITE |
Expand Down
9 changes: 4 additions & 5 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,7 @@ inline MUST_USE_RESULT bool ParseArrayIndex(Local<Value> arg,
return true;
}

CHECK(arg->IsNumber());
int64_t tmp_i = arg.As<Integer>()->Value();
int64_t tmp_i = arg->IntegerValue();

if (tmp_i < 0)
return false;
Expand Down Expand Up @@ -772,7 +771,7 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
SPREAD_BUFFER_ARG(args[0], ts_obj);

Local<String> needle = args[1].As<String>();
int64_t offset_i64 = args[2].As<Integer>()->Value();
int64_t offset_i64 = args[2]->IntegerValue();
bool is_forward = args[4]->IsTrue();

const char* haystack = ts_obj_data;
Expand Down Expand Up @@ -888,7 +887,7 @@ void IndexOfBuffer(const FunctionCallbackInfo<Value>& 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<Integer>()->Value();
int64_t offset_i64 = args[2]->IntegerValue();
bool is_forward = args[4]->IsTrue();

const char* haystack = ts_obj_data;
Expand Down Expand Up @@ -958,7 +957,7 @@ void IndexOfNumber(const FunctionCallbackInfo<Value>& args) {
SPREAD_BUFFER_ARG(args[0], ts_obj);

uint32_t needle = args[1].As<Uint32>()->Value();
int64_t offset_i64 = args[2].As<Integer>()->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);
Expand Down
9 changes: 4 additions & 5 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -977,16 +977,15 @@ void SecureContext::SetDHParam(const FunctionCallbackInfo<Value>& args) {
void SecureContext::SetOptions(const FunctionCallbackInfo<Value>& 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<long>(val)); // NOLINT(runtime/int)
SSL_CTX_set_options(
sc->ctx_.get(),
static_cast<long>(args[0]->IntegerValue())); // NOLINT(runtime/int)
}


Expand Down
8 changes: 2 additions & 6 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,16 +289,12 @@ class Parser : public AsyncWrap, public StreamListener {
MaybeLocal<Value> head_response =
MakeCallback(cb.As<Function>(), 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();
}


Expand Down
5 changes: 2 additions & 3 deletions src/process_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,8 @@ class ProcessWrap : public HandleWrap {
options->stdio[i].data.stream = StreamForWrap(env, stdio);
} else {
Local<String> fd_key = env->fd_string();
Local<Value> fd_value = stdio->Get(context, fd_key).ToLocalChecked();
CHECK(fd_value->IsNumber());
int fd = static_cast<int>(fd_value.As<Integer>()->Value());
int fd = static_cast<int>(
stdio->Get(context, fd_key).ToLocalChecked()->IntegerValue());
options->stdio[i].flags = UV_INHERIT_FD;
options->stdio[i].data.fd = fd;
}
Expand Down
5 changes: 1 addition & 4 deletions src/tcp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,7 @@ void TCPWrap::Open(const FunctionCallbackInfo<Value>& 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<int>(val);
int fd = static_cast<int>(args[0]->IntegerValue());
int err = uv_tcp_open(&wrap->handle_, fd);

if (err == 0)
Expand Down
21 changes: 21 additions & 0 deletions test/parallel/test-buffer-copy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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/);