From 7ce4938aab4c3205a7df4a4891b2bd45ea4457e1 Mon Sep 17 00:00:00 2001 From: Soul Lee Date: Sat, 22 Aug 2026 16:26:12 +0900 Subject: [PATCH 1/2] ffi: throw on missing memory helper arguments ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return undefined instead of throwing when a required argument is omitted, so a call that read or wrote nothing cannot be told apart from one that read a zero byte. All 22 helpers behave this way. GetValidatedPointerAddress() and GetValidatedSize() already reject the same argument when it is passed explicitly as undefined. The args.Length() test in front of them short-circuits the call and returns Nothing without scheduling an exception. These six are the only tests in src/ where args.Length() can skip a call that throws; the only other Length() tests that guard a call at all guard Buffer::HasInstance(), which cannot throw. The remaining tests in this file guard an inline predicate and throw in the branch, which is why setUint8(ptr) reports "Expected an offset argument" while setUint8() reports nothing at all. Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for an out-of-range index, which is exactly the value these validators reject, so each missing argument now produces the error its explicit undefined counterpart produces. The documentation already describes this behavior: the signatures are ffi.getInt8(pointer[, offset]), ffi.setInt8(pointer, offset, value) and ffi.toBuffer(pointer, length[, copy]), and the getters are documented to return a number or a bigint. ExportBytes() carried the same two tests. They are unreachable through the public API because exportBytes is not exported and its three callers all validate len in JavaScript first, but they are the same shape. Signed-off-by: Soul Lee --- src/ffi/data.cc | 15 ++++++--------- test/ffi/test-ffi-memory.js | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/ffi/data.cc b/src/ffi/data.cc index 8bd2c0d9f352..bc287ddd13f9 100644 --- a/src/ffi/data.cc +++ b/src/ffi/data.cc @@ -163,8 +163,7 @@ Maybe ValidateStringLength(Environment* env, size_t len) { Maybe> GetValidatedPointerAndOffset( Environment* env, const FunctionCallbackInfo& args) { uintptr_t raw_ptr; - if (args.Length() < 1 || - !GetValidatedPointerAddress(env, args[0], "pointer").To(&raw_ptr)) { + if (!GetValidatedPointerAddress(env, args[0], "pointer").To(&raw_ptr)) { return {}; } @@ -204,8 +203,7 @@ Maybe GetValidatedPointerOffsetAndValue( size_t offset; Local value; uintptr_t raw_ptr; - if (args.Length() < 1 || - !GetValidatedPointerAddress(env, args[0], "pointer").To(&raw_ptr)) { + if (!GetValidatedPointerAddress(env, args[0], "pointer").To(&raw_ptr)) { return {}; } @@ -556,7 +554,7 @@ void ToBuffer(const FunctionCallbackInfo& args) { } size_t len; - if (args.Length() < 2 || !GetValidatedSize(env, args[1], "length").To(&len)) { + if (!GetValidatedSize(env, args[1], "length").To(&len)) { return; } @@ -618,7 +616,7 @@ void ToArrayBuffer(const FunctionCallbackInfo& args) { } size_t len; - if (args.Length() < 2 || !GetValidatedSize(env, args[1], "length").To(&len)) { + if (!GetValidatedSize(env, args[1], "length").To(&len)) { return; } @@ -696,13 +694,12 @@ void ExportBytes(const FunctionCallbackInfo& args) { } uintptr_t ptr; - if (args.Length() < 2 || - !GetValidatedPointerAddress(env, args[1], "pointer").To(&ptr)) { + if (!GetValidatedPointerAddress(env, args[1], "pointer").To(&ptr)) { return; } size_t len; - if (args.Length() < 3 || !GetValidatedSize(env, args[2], "length").To(&len)) { + if (!GetValidatedSize(env, args[2], "length").To(&len)) { return; } diff --git a/test/ffi/test-ffi-memory.js b/test/ffi/test-ffi-memory.js index 5e667af2eb13..6d47200db187 100644 --- a/test/ffi/test-ffi-memory.js +++ b/test/ffi/test-ffi-memory.js @@ -318,3 +318,23 @@ test('ffi validates memory access arguments', () => { } })); }); + +test('ffi memory helpers reject missing required arguments', () => { + const widths = ['Int8', 'Uint8', 'Int16', 'Uint16', 'Int32', 'Uint32', + 'Int64', 'Uint64', 'Float32', 'Float64']; + + // Calling a helper with no arguments must report the missing pointer the + // same way an explicitly passed `undefined` does, instead of returning + // `undefined` as if the read or the write had succeeded. + for (const width of widths) { + for (const name of [`get${width}`, `set${width}`]) { + assert.throws(() => ffi[name](), { code: 'ERR_INVALID_ARG_VALUE' }); + assert.throws(() => ffi[name](undefined), { code: 'ERR_INVALID_ARG_VALUE' }); + } + } + + assert.throws(() => ffi.toBuffer(1n), { code: 'ERR_INVALID_ARG_VALUE' }); + assert.throws(() => ffi.toBuffer(1n, undefined), { code: 'ERR_INVALID_ARG_VALUE' }); + assert.throws(() => ffi.toArrayBuffer(1n), { code: 'ERR_INVALID_ARG_VALUE' }); + assert.throws(() => ffi.toArrayBuffer(1n, undefined), { code: 'ERR_INVALID_ARG_VALUE' }); +}); From 3158b7926ff1ab89957dcd20e1235861e8b9cb64 Mon Sep 17 00:00:00 2001 From: Soul Lee Date: Sun, 6 Sep 2026 15:50:39 +0900 Subject: [PATCH 2/2] ffi: throw ERR_INVALID_ARG_TYPE for wrong-typed pointer and size GetValidatedPointerAddress() and GetValidatedSize() throw ERR_INVALID_ARG_VALUE when the argument is not a bigint or not a number. doc/api/errors.md reserves that code for a value of the right type that is invalid or unsupported; a wrong type is ERR_INVALID_ARG_TYPE. That is what the JavaScript validators behind exportString() and exportBuffer() throw, and what the inline IsBigInt() checks in toString(), toBuffer() and toArrayBuffer() throw for their first argument. So ffi.exportBuffer(buf, ptr, 'x') reports a type error while ffi.toBuffer(ptr, 'x') reports a value error for the same mistake. Switch the two type-check branches to ERR_INVALID_ARG_TYPE. The branches that reject a negative or non-integer value, or a value outside the platform range, keep ERR_INVALID_ARG_VALUE and ERR_OUT_OF_RANGE. This changes the code thrown for a non-bigint pointer by the getters, the setters, exportBuffer(), exportArrayBuffer() and exportArrayBufferView(), and for a non-number offset or length by the getters, the setters, toBuffer() and toArrayBuffer(). The messages are unchanged. No existing test asserted ERR_INVALID_ARG_VALUE on any of these paths. Refs: https://github.com/nodejs/node/pull/65500 Signed-off-by: Soul Lee --- src/ffi/data.cc | 4 ++-- test/ffi/test-ffi-memory.js | 40 +++++++++++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/ffi/data.cc b/src/ffi/data.cc index bc287ddd13f9..204d0424dfc3 100644 --- a/src/ffi/data.cc +++ b/src/ffi/data.cc @@ -39,7 +39,7 @@ Maybe GetValidatedSize(Environment* env, Local value, const char* label) { if (!value->IsNumber()) { - THROW_ERR_INVALID_ARG_VALUE(env, "The %s must be a number", label); + THROW_ERR_INVALID_ARG_TYPE(env, "The %s must be a number", label); return Nothing(); } @@ -62,7 +62,7 @@ Maybe GetValidatedPointerAddress(Environment* env, Local value, const char* label) { if (!value->IsBigInt()) { - THROW_ERR_INVALID_ARG_VALUE(env, "The %s must be a bigint", label); + THROW_ERR_INVALID_ARG_TYPE(env, "The %s must be a bigint", label); return Nothing(); } diff --git a/test/ffi/test-ffi-memory.js b/test/ffi/test-ffi-memory.js index 6d47200db187..e2e707bda362 100644 --- a/test/ffi/test-ffi-memory.js +++ b/test/ffi/test-ffi-memory.js @@ -328,13 +328,41 @@ test('ffi memory helpers reject missing required arguments', () => { // `undefined` as if the read or the write had succeeded. for (const width of widths) { for (const name of [`get${width}`, `set${width}`]) { - assert.throws(() => ffi[name](), { code: 'ERR_INVALID_ARG_VALUE' }); - assert.throws(() => ffi[name](undefined), { code: 'ERR_INVALID_ARG_VALUE' }); + assert.throws(() => ffi[name](), { code: 'ERR_INVALID_ARG_TYPE' }); + assert.throws(() => ffi[name](undefined), { code: 'ERR_INVALID_ARG_TYPE' }); } } - assert.throws(() => ffi.toBuffer(1n), { code: 'ERR_INVALID_ARG_VALUE' }); - assert.throws(() => ffi.toBuffer(1n, undefined), { code: 'ERR_INVALID_ARG_VALUE' }); - assert.throws(() => ffi.toArrayBuffer(1n), { code: 'ERR_INVALID_ARG_VALUE' }); - assert.throws(() => ffi.toArrayBuffer(1n, undefined), { code: 'ERR_INVALID_ARG_VALUE' }); + assert.throws(() => ffi.toBuffer(1n), { code: 'ERR_INVALID_ARG_TYPE' }); + assert.throws(() => ffi.toBuffer(1n, undefined), { code: 'ERR_INVALID_ARG_TYPE' }); + assert.throws(() => ffi.toArrayBuffer(1n), { code: 'ERR_INVALID_ARG_TYPE' }); + assert.throws(() => ffi.toArrayBuffer(1n, undefined), { code: 'ERR_INVALID_ARG_TYPE' }); +}); + +test('ffi memory helpers distinguish wrong-typed from invalid arguments', () => { + withAllocations(common.mustCall((alloc) => { + const ptr = alloc(8); + const type = { code: 'ERR_INVALID_ARG_TYPE' }; + const value = { code: 'ERR_INVALID_ARG_VALUE' }; + + // A pointer that is not a bigint, or an offset or length that is not a + // number, is a type error, like the JavaScript validators report it. + assert.throws(() => ffi.getInt8('x'), type); + assert.throws(() => ffi.getInt8(ptr, 'x'), type); + assert.throws(() => ffi.setInt8('x', 0, 1), type); + assert.throws(() => ffi.setInt8(ptr, 'x', 1), type); + assert.throws(() => ffi.toBuffer(ptr, 'x'), type); + assert.throws(() => ffi.toArrayBuffer(ptr, 'x'), type); + assert.throws(() => ffi.exportBuffer(Buffer.from([1]), 'x', 1), type); + assert.throws(() => ffi.exportArrayBuffer(new ArrayBuffer(1), 'x', 1), type); + assert.throws(() => ffi.exportArrayBufferView(new Uint8Array(1), 'x', 1), type); + + // A bigint or number of the right type that is out of range stays a + // value error. + assert.throws(() => ffi.getInt8(-1n), value); + assert.throws(() => ffi.getInt8(ptr, -1), value); + assert.throws(() => ffi.setInt8(ptr, 1.5, 1), value); + assert.throws(() => ffi.toBuffer(ptr, 1.5), value); + assert.throws(() => ffi.exportBuffer(Buffer.from([1]), -1n, 1), value); + })); });