ffi: throw ERR_INVALID_ARG_TYPE for wrong-typed pointer and size

src/ffi/data.cc

@@ -39,7 +39,7 @@ Maybe<size_t> GetValidatedSize(Environment* env,
                                Local<Value> 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<size_t>();
   }
 
@@ -62,7 +62,7 @@ Maybe<uintptr_t> GetValidatedPointerAddress(Environment* env,
                                             Local<Value> 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<uintptr_t>();
   }
 
@@ -163,8 +163,7 @@ Maybe<void> ValidateStringLength(Environment* env, size_t len) {
 Maybe<std::pair<uint8_t*, size_t>> GetValidatedPointerAndOffset(
     Environment* env, const FunctionCallbackInfo<Value>& 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<PointerOffsetAndValue> GetValidatedPointerOffsetAndValue(
   size_t offset;
   Local<Value> 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<Value>& 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<Value>& 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<Value>& 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;
   }
 

test/ffi/test-ffi-memory.js

@@ -318,3 +318,51 @@ 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_TYPE' });
+      assert.throws(() => ffi[name](undefined), { code: 'ERR_INVALID_ARG_TYPE' });
+    }
+  }
+
+  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);
+  }));
+});