fix not parsing the buffer when only two arguments are provided

repro.ts

@@ -0,0 +1,4 @@
+import { writeSync, openSync } from "fs";
+
+const fd = openSync("repro.txt", "w");
+writeSync(fd, "abc");

repro.txt

@@ -0,0 +1 @@
+abc

src/bun.js/node/node_fs.zig

@@ -2422,10 +2422,10 @@ pub const Arguments = struct {
         fd: FileDescriptor,
         buffer: StringOrBuffer,
         // buffer_val: JSC.JSValue = JSC.JSValue.zero,
-        offset: u64 = 0,
-        length: u64 = std.math.maxInt(u64),
-        position: ?ReadPosition = null,
-        encoding: Encoding = Encoding.buffer,
+        offset: u64,
+        length: u64,
+        position: ?ReadPosition,
+        encoding: Encoding,
 
         pub fn deinit(this: *const @This()) void {
             this.buffer.deinit();
@@ -2440,86 +2440,101 @@ pub const Arguments = struct {
         }
 
         pub fn fromJS(ctx: *JSC.JSGlobalObject, arguments: *ArgumentsSlice) bun.JSError!Write {
+            // fs.write(fd, string[, position[, encoding]], callback)
             const fd_value = arguments.nextEat() orelse JSC.JSValue.undefined;
             const fd = try bun.FD.fromJSValidated(fd_value, ctx) orelse {
                 return throwInvalidFdError(ctx, fd_value);
             };
 
-            const buffer_value = arguments.next();
-            const buffer = try StringOrBuffer.fromJS(ctx, bun.default_allocator, buffer_value orelse {
-                return ctx.throwInvalidArguments("data is required", .{});
-            }) orelse {
-                return ctx.throwInvalidArgumentTypeValue("buffer", "string or TypedArray", buffer_value.?);
+            const buffer_value = arguments.next() orelse {
+                return ctx.ERR(.INVALID_ARG_TYPE, "The \"buffer\" argument must be of type string or an instance of Buffer, TypedArray, or DataView", .{}).throw();
             };
-            if (buffer_value.?.isString() and !buffer_value.?.isStringLiteral()) {
-                return ctx.throwInvalidArgumentTypeValue("buffer", "string or TypedArray", buffer_value.?);
+
+            if (buffer_value.isString() and !buffer_value.isStringLiteral()) {
+                return ctx.ERR(.INVALID_ARG_TYPE, "The \"buffer\" argument must be of type string or an instance of Buffer, TypedArray, or DataView", .{}).throw();
             }
 
-            var args = Write{
-                .fd = fd,
-                .buffer = buffer,
-                .encoding = switch (buffer) {
-                    .buffer => Encoding.buffer,
-                    inline else => Encoding.utf8,
-                },
-            };
-            errdefer args.deinit();
+            var encoding: Encoding = Encoding.buffer;
+            var position_res: ?ReadPosition = null;
+            var offset: u64 = 0;
+            var length_res: u64 = std.math.maxInt(u64);
             arguments.eat();
 
+            var buffer: ?StringOrBuffer = null;
+            errdefer if (buffer) |*b| b.deinit();
+
+            const allow_string_object = false;
             parse: {
                 var current = arguments.next() orelse break :parse;
-                switch (buffer) {
-                    // fs.write(fd, string[, position[, encoding]], callback)
-                    else => {
-                        if (current.isNumber()) {
-                            args.position = current.to(i52);
-                            arguments.eat();
-                            current = arguments.next() orelse break :parse;
-                        }
+                if (buffer_value.isString()) {
+                    encoding = Encoding.utf8;
+                    if (current.isNumber()) {
+                        position_res = current.to(i52);
+                        arguments.eat();
+                        current = arguments.next() orelse break :parse;
+                    }
+
+                    if (current.isString()) {
+                        encoding = try Encoding.assert(current, ctx, encoding);
+                        try bun.validators.validateEncoding(ctx, buffer_value, encoding, "encoding");
+                        arguments.eat();
+                    }
+                } else {
+                    buffer = try StringOrBuffer.fromJSWithEncodingMaybeAsync(ctx, bun.default_allocator, buffer_value, encoding, arguments.will_be_async, allow_string_object) orelse {
+                        return ctx.ERR(.INVALID_ARG_TYPE, "The \"buffer\" argument must be of type string or an instance of Buffer, TypedArray, or DataView", .{}).throw();
+                    };
+                    if (buffer.? != .buffer) {
+                        return ctx.ERR(.INVALID_ARG_TYPE, "The \"buffer\" argument must be of type string or an instance of Buffer, TypedArray, or DataView", .{}).throw();
+                    }
 
-                        if (current.isString()) {
-                            args.encoding = try Encoding.assert(current, ctx, args.encoding);
-                            arguments.eat();
-                        }
-                    },
                     // fs.write(fd, buffer[, offset[, length[, position]]], callback)
-                    .buffer => {
-                        if (current.isUndefinedOrNull() or current.isFunction()) break :parse;
-                        args.offset = @intCast(try JSC.Node.validators.validateInteger(ctx, current, "offset", 0, 9007199254740991));
-                        arguments.eat();
-                        current = arguments.next() orelse break :parse;
+                    if (current.isUndefinedOrNull() or current.isFunction()) break :parse;
+                    offset = @intCast(try JSC.Node.validators.validateInteger(ctx, current, "offset", 0, 9007199254740991));
+                    arguments.eat();
+                    current = arguments.next() orelse break :parse;
 
-                        if (!(current.isNumber() or current.isBigInt())) break :parse;
-                        const length = current.to(i64);
-                        const buf_len = args.buffer.buffer.slice().len;
-                        const max_offset = @min(buf_len, std.math.maxInt(i64));
-                        if (args.offset > max_offset) {
-                            return ctx.throwRangeError(
-                                @as(f64, @floatFromInt(args.offset)),
-                                .{ .field_name = "offset", .max = @intCast(max_offset) },
-                            );
-                        }
-                        const max_len = @min(buf_len - args.offset, std.math.maxInt(i32));
-                        if (length > max_len or length < 0) {
-                            return ctx.throwRangeError(
-                                @as(f64, @floatFromInt(length)),
-                                .{ .field_name = "length", .min = 0, .max = @intCast(max_len) },
-                            );
-                        }
-                        args.length = @intCast(length);
+                    if (!(current.isNumber() or current.isBigInt())) break :parse;
+                    const length = current.to(i64);
+                    const buf_len = buffer.?.buffer.slice().len;
+                    const max_offset = @min(buf_len, std.math.maxInt(i64));
+                    if (offset > max_offset) {
+                        return ctx.throwRangeError(
+                            @as(f64, @floatFromInt(offset)),
+                            .{ .field_name = "offset", .max = @intCast(max_offset) },
+                        );
+                    }
+                    const max_len = @min(buf_len - offset, std.math.maxInt(i32));
+                    if (length > max_len or length < 0) {
+                        return ctx.throwRangeError(
+                            @as(f64, @floatFromInt(length)),
+                            .{ .field_name = "length", .min = 0, .max = @intCast(max_len) },
+                        );
+                    }
+                    length_res = @intCast(length);
 
-                        arguments.eat();
-                        current = arguments.next() orelse break :parse;
+                    arguments.eat();
+                    current = arguments.next() orelse break :parse;
 
-                        if (!(current.isNumber() or current.isBigInt())) break :parse;
-                        const position = current.to(i52);
-                        if (position >= 0) args.position = position;
-                        arguments.eat();
-                    },
+                    if (!(current.isNumber() or current.isBigInt())) break :parse;
+                    const position = current.to(i52);
+                    if (position >= 0) position_res = position;
+                    arguments.eat();
                 }
             }
+            if (buffer == null) {
+                buffer = try StringOrBuffer.fromJSWithEncodingMaybeAsync(ctx, bun.default_allocator, buffer_value, encoding, arguments.will_be_async, allow_string_object) orelse {
+                    return ctx.ERR(.INVALID_ARG_TYPE, "The \"buffer\" argument must be of type string or an instance of Buffer, TypedArray, or DataView", .{}).throw();
+                };
+            }
 
-            return args;
+            return Write{
+                .fd = fd,
+                .buffer = buffer.?,
+                .encoding = encoding,
+                .position = position_res,
+                .offset = offset,
+                .length = length_res,
+            };
         }
     };
 

src/bun.js/node/util/validators.zig

@@ -296,3 +296,13 @@ pub fn validateStringEnum(comptime T: type, globalThis: *JSGlobalObject, value:
     };
     return throwErrInvalidArgTypeWithMessage(globalThis, name_fmt ++ " must be one of: {s}", name_args ++ .{values_info});
 }
+
+pub fn validateEncoding(globalThis: *JSGlobalObject, value: JSValue, encoding: @import("../types.zig").Encoding, param_name: string) bun.JSError!void {
+    if (encoding != .hex) return;
+    if (!value.isString()) {
+        return;
+    }
+    const str = value.asString();
+    if (str.length() % 2 == 0) return;
+    return globalThis.ERR(.INVALID_ARG_VALUE, "The argument '{s}' is invalid for data of length {d}. Received 'hex'", .{ param_name, str.length() }).throw();
+}

test/js/node/test/parallel/test-fs-write.js

@@ -0,0 +1,221 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+// Flags: --expose_externalize_string
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const fs = require('fs');
+const tmpdir = require('../common/tmpdir');
+
+tmpdir.refresh();
+
+const fn = tmpdir.resolve('write.txt');
+const fn2 = tmpdir.resolve('write2.txt');
+const fn3 = tmpdir.resolve('write3.txt');
+const fn4 = tmpdir.resolve('write4.txt');
+const expected = 'ümlaut.';
+const constants = fs.constants;
+
+if(typeof Bun !== "undefined") {
+  global.createExternalizableString = str => str;
+  global.externalizeString = str => {};
+  global.isOneByteString = str => {
+    for(let i = 0; i < str.length; i++) {
+      if(str.charCodeAt(i) > 0xFF) {
+        return false;
+      }
+    }
+    return true;
+  };
+}
+
+const {
+  createExternalizableString,
+  externalizeString,
+  isOneByteString,
+} = global;
+
+// Account for extra globals exposed by --expose_externalize_string.
+common.allowGlobals(
+  createExternalizableString,
+  externalizeString,
+  isOneByteString,
+  global.x,
+);
+
+{
+  // Must be a unique string.
+  const expected = createExternalizableString('ümlaut sechzig');
+  externalizeString(expected);
+  assert.strictEqual(isOneByteString(expected), true);
+  const fd = fs.openSync(fn, 'w');
+  fs.writeSync(fd, expected, 0, 'latin1');
+  fs.closeSync(fd);
+  assert.strictEqual(fs.readFileSync(fn, 'latin1'), expected);
+}
+
+{
+  // Must be a unique string.
+  const expected = createExternalizableString('ümlaut neunzig');
+  externalizeString(expected);
+  assert.strictEqual(isOneByteString(expected), true);
+  const fd = fs.openSync(fn, 'w');
+  fs.writeSync(fd, expected, 0, 'utf8');
+  fs.closeSync(fd);
+  assert.strictEqual(fs.readFileSync(fn, 'utf8'), expected);
+}
+
+{
+  // Must be a unique string.
+  const expected = createExternalizableString('Zhōngwén 1');
+  externalizeString(expected);
+  assert.strictEqual(isOneByteString(expected), false);
+  const fd = fs.openSync(fn, 'w');
+  fs.writeSync(fd, expected, 0, 'ucs2');
+  fs.closeSync(fd);
+  assert.strictEqual(fs.readFileSync(fn, 'ucs2'), expected);
+}
+
+{
+  // Must be a unique string.
+  const expected = createExternalizableString('Zhōngwén 2');
+  externalizeString(expected);
+  assert.strictEqual(isOneByteString(expected), false);
+  const fd = fs.openSync(fn, 'w');
+  fs.writeSync(fd, expected, 0, 'utf8');
+  fs.closeSync(fd);
+  assert.strictEqual(fs.readFileSync(fn, 'utf8'), expected);
+}
+
+fs.open(fn, 'w', 0o644, common.mustSucceed((fd) => {
+  const done = common.mustSucceed((written) => {
+    assert.strictEqual(written, Buffer.byteLength(expected));
+    fs.closeSync(fd);
+    const found = fs.readFileSync(fn, 'utf8');
+    fs.unlinkSync(fn);
+    assert.strictEqual(found, expected);
+  });
+
+  const written = common.mustSucceed((written) => {
+    assert.strictEqual(written, 0);
+    fs.write(fd, expected, 0, 'utf8', done);
+  });
+
+  fs.write(fd, '', 0, 'utf8', written);
+}));
+
+const args = constants.O_CREAT | constants.O_WRONLY | constants.O_TRUNC;
+fs.open(fn2, args, 0o644, common.mustSucceed((fd) => {
+  const done = common.mustSucceed((written) => {
+    assert.strictEqual(written, Buffer.byteLength(expected));
+    fs.closeSync(fd);
+    const found = fs.readFileSync(fn2, 'utf8');
+    fs.unlinkSync(fn2);
+    assert.strictEqual(found, expected);
+  });
+
+  const written = common.mustSucceed((written) => {
+    assert.strictEqual(written, 0);
+    fs.write(fd, expected, 0, 'utf8', done);
+  });
+
+  fs.write(fd, '', 0, 'utf8', written);
+}));
+
+fs.open(fn3, 'w', 0o644, common.mustSucceed((fd) => {
+  const done = common.mustSucceed((written) => {
+    assert.strictEqual(written, Buffer.byteLength(expected));
+    fs.closeSync(fd);
+  });
+
+  fs.write(fd, expected, done);
+}));
+
+
+[false, 'test', {}, [], null, undefined].forEach((i) => {
+  assert.throws(
+    () => fs.write(i, common.mustNotCall()),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError'
+    }
+  );
+  assert.throws(
+    () => fs.writeSync(i),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError'
+    }
+  );
+});
+
+[
+  false, 5, {}, [], null, undefined, true, 5n, () => {}, Symbol(), new Map(),
+  new String('notPrimitive'),
+  { [Symbol.toPrimitive]: (hint) => 'amObject' },
+  { toString() { return 'amObject'; } },
+  Promise.resolve('amPromise'),
+  common.mustNotCall(),
+].forEach((data) => {
+  assert.throws(
+    () => fs.write(1, data, common.mustNotCall()),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: /"buffer"/
+    }
+  );
+  assert.throws(
+    () => fs.writeSync(1, data),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: /"buffer"/
+    }
+  );
+});
+
+{
+  // Regression test for https://github.com/nodejs/node/issues/38168
+  const fd = fs.openSync(fn4, 'w');
+
+  assert.throws(
+    () => fs.writeSync(fd, 'abc', 0, 'hex'),
+    {
+      code: 'ERR_INVALID_ARG_VALUE',
+      message: /'encoding' is invalid for data of length 3/
+    }
+  );
+
+  assert.throws(
+    () => fs.writeSync(fd, 'abc', 0, 'hex', common.mustNotCall()),
+    {
+      code: 'ERR_INVALID_ARG_VALUE',
+      message: /'encoding' is invalid for data of length 3/
+    }
+  );
+
+  assert.strictEqual(fs.writeSync(fd, 'abcd', 0, 'hex'), 2);
+
+  fs.write(fd, 'abcd', 0, 'hex', common.mustSucceed((written) => {
+    assert.strictEqual(written, 2);
+    fs.closeSync(fd);
+  }));
+}