test: add fs.writeSync optional params test and fix

src/bun.js/api/server.zig

@@ -2292,9 +2292,6 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
 
         flags: NewFlags(debug_mode) = .{},
 
-        /// timeout in seconds set via IncomingMessage.setTimeout().
-        request_timeout_seconds: u8 = 0,
-
         upgrade_context: ?*uws.uws_socket_context_t = null,
 
         /// We can only safely free once the request body promise is finalized
@@ -4480,10 +4477,6 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
 
             assert(this.resp == resp);
 
-            if (this.request_timeout_seconds > 0) {
-                resp.timeout(this.request_timeout_seconds);
-            }
-
             this.flags.is_waiting_for_request_body = last == false;
             if (this.isAbortedOrEnded() or this.flags.has_marked_complete) return;
             if (!last and chunk.len == 0) {
@@ -4654,10 +4647,8 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
 
         pub fn setTimeout(this: *RequestContext, seconds: c_uint) bool {
             if (this.resp) |resp| {
-                const secs: u8 = @truncate(@min(seconds, 255));
-                this.request_timeout_seconds = secs;
-                resp.timeout(secs);
-                if (secs > 0) {
+                resp.timeout(@min(seconds, 255));
+                if (seconds > 0) {
 
                     // we only set the timeout callback if we wanna the timeout event to be triggered
                     // the connection will be closed so the abort handler will be called after the timeout
@@ -4668,7 +4659,6 @@ fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comp
                     }
                 } else {
                     // if the timeout is 0, we don't need to trigger the timeout event
-                    this.request_timeout_seconds = 0;
                     resp.clearTimeout();
                 }
                 return true;

src/js/node/fs.ts

@@ -562,7 +562,23 @@ var access = function access(path, mode, callback) {
 
     return fs.readSync(fd, buffer, offset, length, position);
   },
-  writeSync = fs.writeSync.bind(fs),
+  writeSync = function writeSync(fd, buffer, offsetOrOptions, length, position) {
+    if ($isTypedArrayView(buffer)) {
+      let offset = offsetOrOptions;
+      if (arguments.length <= 3 || typeof offsetOrOptions === "object") {
+        if (offsetOrOptions !== undefined) {
+          if (typeof offsetOrOptions !== "object" || $isArray(offsetOrOptions)) {
+            throw $ERR_INVALID_ARG_TYPE("options", "object", offsetOrOptions);
+          }
+        }
+        ({ offset = 0, length = buffer.byteLength - offset, position = null } = offsetOrOptions ?? {});
+      }
+
+      return fs.writeSync(fd, buffer, offset, length, position);
+    }
+
+    return fs.writeSync(fd, buffer, offsetOrOptions, length);
+  },
   readdirSync = fs.readdirSync.bind(fs),
   readFileSync = fs.readFileSync.bind(fs),
   fdatasyncSync = fs.fdatasyncSync.bind(fs),

test/js/node/test/parallel/test-fs-write-sync-optional-params.js

@@ -0,0 +1,102 @@
+const common = require('../common');
+
+// This test ensures that fs.writeSync accepts "named parameters" object
+// and doesn't interpret objects as strings
+
+const assert = require('assert');
+const fs = require('fs');
+const tmpdir = require('../common/tmpdir');
+
+tmpdir.refresh();
+
+const dest = tmpdir.resolve('tmp.txt');
+const buffer = Buffer.from('zyx');
+
+function testInvalid(dest, expectedCode, ...bufferAndOptions) {
+  if (bufferAndOptions.length >= 2) {
+    bufferAndOptions[1] = common.mustNotMutateObjectDeep(bufferAndOptions[1]);
+  }
+  let fd;
+  try {
+    fd = fs.openSync(dest, 'w+');
+    assert.throws(
+      () => fs.writeSync(fd, ...bufferAndOptions),
+      { code: expectedCode });
+  } finally {
+    if (fd != null) fs.closeSync(fd);
+  }
+}
+
+function testValid(dest, buffer, options) {
+  const length = options?.length;
+  let fd, bytesWritten, bytesRead;
+
+  try {
+    fd = fs.openSync(dest, 'w');
+    bytesWritten = fs.writeSync(fd, buffer, options);
+  } finally {
+    if (fd != null) fs.closeSync(fd);
+  }
+
+  try {
+    fd = fs.openSync(dest, 'r');
+    bytesRead = fs.readSync(fd, buffer, options);
+  } finally {
+    if (fd != null) fs.closeSync(fd);
+  }
+
+  assert.ok(bytesWritten >= bytesRead);
+  if (length !== undefined && length !== null) {
+    assert.strictEqual(bytesWritten, length);
+    assert.strictEqual(bytesRead, length);
+  }
+}
+
+{
+  // Test if second argument is not wrongly interpreted as string or options
+  for (const badBuffer of [
+    undefined, null, true, 42, 42n, Symbol('42'), NaN, [], () => {},
+    common.mustNotCall(),
+    common.mustNotMutateObjectDeep({}),
+    {},
+    { buffer: 'amNotParam' },
+    { string: 'amNotParam' },
+    { buffer: new Uint8Array(1) },
+    { buffer: new Uint8Array(1).buffer },
+    Promise.resolve(new Uint8Array(1)),
+    new Date(),
+    new String('notPrimitive'),
+    { toString() { return 'amObject'; } },
+    { [Symbol.toPrimitive]: (hint) => 'amObject' },
+  ]) {
+    testInvalid(dest, 'ERR_INVALID_ARG_TYPE', common.mustNotMutateObjectDeep(badBuffer));
+  }
+
+  // First argument (buffer or string) is mandatory
+  testInvalid(dest, 'ERR_INVALID_ARG_TYPE');
+
+  // Various invalid options
+  testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: 5 });
+  testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { offset: 5 });
+  testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: 1, offset: 3 });
+  testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: -1 });
+  testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { offset: -1 });
+  testInvalid(dest, 'ERR_INVALID_ARG_TYPE', buffer, { offset: false });
+  testInvalid(dest, 'ERR_INVALID_ARG_TYPE', buffer, { offset: true });
+
+  // Test compatibility with fs.readSync counterpart with reused options
+  for (const options of [
+    undefined,
+    null,
+    {},
+    { length: 1 },
+    { position: 5 },
+    { length: 1, position: 5 },
+    { length: 1, position: -1, offset: 2 },
+    { length: null },
+    { position: null },
+    { offset: 1 },
+  ]) {
+    testValid(dest, buffer, common.mustNotMutateObjectDeep(options));
+  }
+}

test/js/node/test/parallel/test-http-server-consumed-timeout.js

@@ -1,84 +0,0 @@
-'use strict';
-const common = require('../common');
-const assert = require('assert');
-const http = require('http');
-
-const durationBetweenIntervals = [];
-let timeoutTooShort = false;
-const TIMEOUT = common.platformTimeout(200);
-const INTERVAL = Math.floor(TIMEOUT / 8);
-
-runTest(TIMEOUT);
-
-function runTest(timeoutDuration) {
-  let intervalWasInvoked = false;
-  let newTimeoutDuration = 0;
-  const closeCallback = (err) => {
-    assert.ifError(err);
-    if (newTimeoutDuration) {
-      runTest(newTimeoutDuration);
-    }
-  };
-
-  const server = http.createServer((req, res) => {
-    server.close(common.mustCall(closeCallback));
-
-    res.writeHead(200);
-    res.flushHeaders();
-
-    req.setTimeout(timeoutDuration, () => {
-      if (!intervalWasInvoked) {
-        // Interval wasn't invoked, probably because the machine is busy with
-        // other things. Try again with a longer timeout.
-        newTimeoutDuration = timeoutDuration * 2;
-        console.error('The interval was not invoked.');
-        console.error(`Trying w/ timeout of ${newTimeoutDuration}.`);
-        return;
-      }
-
-      if (timeoutTooShort) {
-        intervalWasInvoked = false;
-        timeoutTooShort = false;
-        newTimeoutDuration =
-          Math.max(...durationBetweenIntervals, timeoutDuration) * 2;
-        console.error(`Time between intervals: ${durationBetweenIntervals}`);
-        console.error(`Trying w/ timeout of ${newTimeoutDuration}`);
-        return;
-      }
-
-      assert.fail('Request timeout should not fire');
-    });
-
-    req.resume();
-    req.once('end', () => {
-      res.end();
-    });
-  });
-
-  server.listen(0, common.mustCall(() => {
-    const req = http.request({
-      port: server.address().port,
-      method: 'POST'
-    }, () => {
-      let lastIntervalTimestamp = Date.now();
-      const interval = setInterval(() => {
-        const lastDuration = Date.now() - lastIntervalTimestamp;
-        durationBetweenIntervals.push(lastDuration);
-        lastIntervalTimestamp = Date.now();
-        if (lastDuration > timeoutDuration / 2) {
-          // The interval is supposed to be about 1/8 of the timeout duration.
-          // If it's running so infrequently that it's greater than 1/2 the
-          // timeout duration, then run the test again with a longer timeout.
-          timeoutTooShort = true;
-        }
-        intervalWasInvoked = true;
-        req.write('a');
-      }, INTERVAL);
-      setTimeout(() => {
-        clearInterval(interval);
-        req.end();
-      }, timeoutDuration);
-    });
-    req.write('.');
-  }));
-}