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: #65500
Signed-off-by: Soul Lee <alus20x@gmail.com>
PR-URL: #65842
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>

Author: soulee-dev

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>();
   }
 

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);
+  }));
 });