Uncatchable EINVAL when passing a 0-length array of buffers to FileHandle.writev

[isker]

Version

16.13.2

Platform

Linux [myhostname] 3.10.0-1160.42.2.el7.x86_64 #1 SMP Tue Sep 7 14:49:57 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

fs

What steps will reproduce the bug?

Invoke https://nodejs.org/api/fs.html#filehandlewritevbuffers-position with an empty array:

/tmp λ cat repro.mjs
import * as fs from 'fs';

try {
  const handle = await fs.promises.open('./out.txt', 'w');
  await handle.writev([]);
  await handle.close();
} catch (e) {
  console.error('caught', e);
}

console.log('done');

How often does it reproduce? Is there a required condition?

Unconditionally with the above reproduction. It also happens on macOS.

What is the expected behavior?

The resulting error should be caught and logged by the catch block.

Alternatively, this should not be an error at all. Writing no data with other FS functions does not result in errors.

What do you see instead?

The resulting error is uncatchable and kills the process.

/tmp λ node repro.mjs
node:internal/fs/promises:547
  const bytesWritten = (await binding.writeBuffers(handle.fd, buffers, position,
                                      ^

Error: EINVAL: invalid argument, write
    at writev (node:internal/fs/promises:547:39)
    at fsCall (node:internal/fs/promises:313:18)
    at FileHandle.writev (node:internal/fs/promises:182:12)
    at file:///tmp/repro.mjs:5:16 {
  errno: -22,
  code: 'EINVAL',
  syscall: 'write'
}

Additional information

No response

[benjamingr]

Confirmed this reproduces on master, taking a look

[benjamingr]

Opened a PR to fix at #41919 thanks for the detailed report

[benjamingr]

There is a second issue here (other than the EINVAL) - the fact it's not catchable, I'll investigate

[benjamingr]

This causes a segfault for example:

    // Writev with bad array-like
    await assert.rejects(async () => {
      const handle = await fs.open(getFileName(), 'w');
      const badArray = new Proxy([], { get(target, prop) {
        if(prop === 'length') return -1;
        return Reflect.get(target, prop);
      }});      
      const result = await handle.writev(badArray);
      handle.close();
    }, { code: 'EINVAL' });

➜  node git:(fix-ev-error) ./out/Release/node --inspect test/parallel/test-fs-writev-promises.js
Debugger listening on ws://127.0.0.1:9229/39dd654c-3663-4e18-8c83-684b1d8fe862
For help, see: https://nodejs.org/en/docs/inspector
./out/Release/node[10237]: ../src/node_file.cc:1891:void node::fs::WriteBuffers(const FunctionCallbackInfo<v8::Value> &): Assertion `args[1]->IsArray()' failed.
 1: 0x10549f6f5 node::Abort() [/Users/bgruenbaum/Documents/Projects/node/out/Release/node]
 2: 0x10549f521 node::Assert(node::AssertionInfo const&) [/Users/bgruenbaum/Documents/Projects/node/out/Release/node]
 3: 0x1054b1856 node::fs::WriteBuffers(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/bgruenbaum/Documents/Projects/node/out/Release/node]
 4: 0x105680228 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/Users/bgruenbaum/Documents/Projects/node/out/Release/node]
 5: 0x10567fd25 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/Users/bgruenbaum/Documents/Projects/node/out/Release/node]
 6: 0x10567f3fb v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [/Users/bgruenbaum/Documents/Projects/node/out/Release/node]
 7: 0x105f5edb9 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/Users/bgruenbaum/Documents/Projects/node/out/Release/node]

So it looks like there are few other places to fix as well

[benjamingr]

Yeah - we check IsArray which the v8 docs say:

Returns true if this value is an array. Note that it will return false for an Proxy for an array.

[isker]

Should this remain open to track the other problems discussed in #41919?

[benjamingr]

Probably a good idea to open a new issue