From eff8ef743db680848b6490d63093a50b7654cffe Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 29 Oct 2026 04:11:04 -0500 Subject: [PATCH 1/2] Read at the exact bigint position in fs.readSync Co-authored-by: MathurAditya724 <57684218+MathurAditya724@users.noreply.github.com> Fixes #2404 --- .../corpus/fs-read-bigint-position.cjs.json | 6 +++ .../fs-read-sync-bigint-position.cjs.json | 6 +++ packages/runtime/src/scr_bytes_io.c | 41 +++++++++++++++---- packages/runtime/src/scr_file_handle.h | 6 +++ packages/runtime/src/scr_lib.c | 41 ++++++++++++++----- tests/corpus/fs-read-bigint-position.cjs | 28 +++++++++++++ tests/corpus/fs-read-sync-bigint-position.cjs | 32 +++++++++++++++ .../fs-read-bigint-position.cjs.json | 1 + .../fs-read-sync-bigint-position.cjs.json | 1 + 9 files changed, 144 insertions(+), 18 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/fs-read-bigint-position.cjs.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/fs-read-sync-bigint-position.cjs.json create mode 100644 tests/corpus/fs-read-bigint-position.cjs create mode 100644 tests/corpus/fs-read-sync-bigint-position.cjs create mode 100644 tests/harness/corpus-costs/fs-read-bigint-position.cjs.json create mode 100644 tests/harness/corpus-costs/fs-read-sync-bigint-position.cjs.json diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/fs-read-bigint-position.cjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/fs-read-bigint-position.cjs.json new file mode 100644 index 000000000..24039a8ab --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/fs-read-bigint-position.cjs.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/fs-read-bigint-position.cjs" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/fs-read-sync-bigint-position.cjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/fs-read-sync-bigint-position.cjs.json new file mode 100644 index 000000000..4d4997e89 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/fs-read-sync-bigint-position.cjs.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/fs-read-sync-bigint-position.cjs" + ], + "diags": [] +} diff --git a/packages/runtime/src/scr_bytes_io.c b/packages/runtime/src/scr_bytes_io.c index 0c4bee4ee..035795619 100644 --- a/packages/runtime/src/scr_bytes_io.c +++ b/packages/runtime/src/scr_bytes_io.c @@ -166,6 +166,8 @@ static ScrDyn *scr_fs_cb_arg(ScrDyn *const *args, size_t argc, size_t index) { return index < argc ? args[index] : scr_dyn_undefined(); } +static bool scr_fs_bigint_position_chk(ScrBigInt *position, double length); + static ScrDyn *scr_fs_cb_option(const ScrDyn *options, const char *name) { if (scr_fs_dyn_absent(options)) return scr_dyn_retain(scr_dyn_undefined()); ScrDyn *object = options->kind == SCR_DYN_TYPED_REF ? scr_dyn_typed_ref_materialize(options) : scr_dyn_retain((ScrDyn *)options); @@ -1129,8 +1131,30 @@ static ScrDyn *scr_fs_cb_invoke(ScrStr *member, ScrDyn *const *args, size_t argc if (ownedBuffer) length = scr_fs_cb_option_number(options, "length", length); else if (readOp && count >= 4) length = readLength; else length = scr_fs_cb_number(ARG(3), "length", length); - double position = ownedBuffer ? scr_fs_cb_option_number(options, "position", -1) : scr_fs_cb_number(ARG(4), "position", -1); - if (!scr_exc_pending()) result = scr_dyn_new_num(readOp ? scr_fs_read_sync(fd, bytes, offset, length, position) : scr_fs_write_sync(fd, bytes, offset, length, position)); + /* A bigint read position takes readSync's range check and reads at + * its exact value: a double would round it past 2^53. */ + ScrDyn *bigPosition = NULL; + if (readOp) { + ScrDyn *value = ownedBuffer ? scr_fs_cb_option(options, "position") + : scr_dyn_retain(count > 4 ? ARG(4) : scr_dyn_undefined()); + if (value && value->kind == SCR_DYN_BIGINT) bigPosition = value; + else scr_dyn_release(value); + } + double position = bigPosition ? -1 + : ownedBuffer ? scr_fs_cb_option_number(options, "position", -1) + : scr_fs_cb_number(ARG(4), "position", -1); + if (!scr_exc_pending()) { + if (bigPosition) { + if (scr_fs_read_offset_chk(offset) && + scr_fs_bigint_position_chk(bigPosition->v.bigint, length)) + result = scr_dyn_new_num(scr_fs_read_sync_at(fd, bytes, offset, length, + scr_bigint_to_i64(bigPosition->v.bigint))); + } else { + result = scr_dyn_new_num(readOp ? scr_fs_read_sync(fd, bytes, offset, length, position) + : scr_fs_write_sync(fd, bytes, offset, length, position)); + } + } + scr_dyn_release(bigPosition); } scr_bytes_release(bytes); /* The completion retains the original buffer, including options.buffer. */ @@ -2101,12 +2125,15 @@ double scr_fs_read_sync_dyn(const ScrDyn *fd, const ScrDyn *buffer, const ScrDyn double count = 0; if (!scr_fs_read_validate(fd, buffer, offset, length, position, &count)) return 0; if (count == 0) return 0; - double at = scr_fs_dyn_absent(position) ? -1 - : position->kind == SCR_DYN_BIGINT ? scr_bigint_to_f64(position->v.bigint) - : position->v.num; + double off = scr_fs_dyn_absent(offset) ? 0 : offset->v.num; ScrBytes *bytes = scr_dyn_bytes_unbox(buffer); - double read = scr_fs_read_sync(fd->v.num, bytes, scr_fs_dyn_absent(offset) ? 0 : offset->v.num, - count, at); + /* A bigint position passed its range check above and reads at its exact + * value: a double would round it past 2^53. */ + double read = + !scr_fs_dyn_absent(position) && position->kind == SCR_DYN_BIGINT + ? scr_fs_read_sync_at(fd->v.num, bytes, off, count, scr_bigint_to_i64(position->v.bigint)) + : scr_fs_read_sync(fd->v.num, bytes, off, count, + scr_fs_dyn_absent(position) ? -1 : position->v.num); scr_bytes_release(bytes); return read; } diff --git a/packages/runtime/src/scr_file_handle.h b/packages/runtime/src/scr_file_handle.h index 5bdd15a65..4d038ffbe 100644 --- a/packages/runtime/src/scr_file_handle.h +++ b/packages/runtime/src/scr_file_handle.h @@ -60,6 +60,12 @@ ScrStats *scr_file_handle_stat(ScrFileHandle *h); * nonnegative positions leave that offset unchanged (pread/ReadFile seam). */ double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length, double position); +/* fs.readSync's offset check; false with ERR_OUT_OF_RANGE pending. */ +bool scr_fs_read_offset_chk(double offset); +/* scr_fs_read_sync after its offset and position checks, with an exact + * position (-1 for the current offset). */ +double scr_fs_read_sync_at(double fd, ScrBytes *buf, double offset, double length, + int64_t position); /* writeSync's classic Buffer window and utf8 string forms. position -1 (or * any other non-safe/nonnegative-integer value) writes at and advances the * current descriptor offset; safe nonnegative positions leave it unchanged. */ diff --git a/packages/runtime/src/scr_lib.c b/packages/runtime/src/scr_lib.c index 9ef71a223..a676b09f1 100644 --- a/packages/runtime/src/scr_lib.c +++ b/packages/runtime/src/scr_lib.c @@ -3935,7 +3935,7 @@ void scr_fs_futimes(double fd, double atime, double mtime) { * supplies pread(2). Windows follows libuv's synchronous-handle recipe: * ReadFile with an OVERLAPPED byte offset, then restore the handle position * because synchronous ReadFile updates it even when OVERLAPPED is present. */ -static ssize_t scr_fs_pread(int fd, void *data, size_t length, double position) { +static ssize_t scr_fs_pread(int fd, void *data, size_t length, int64_t position) { #ifdef _WIN32 HANDLE handle = (HANDLE)_get_osfhandle(fd); if (handle == INVALID_HANDLE_VALUE) { @@ -4311,14 +4311,11 @@ void scr_fs_write_file_fd(double fd, ScrStr *data) { * throw the RangeError shape. Position -1 means the fd's current offset; * nonnegative positions do not advance it. Returns the byte count the OS * reports; errors carry the errno name like the other fd operations. */ -double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length, - double position) { - size_t bytelen = buf->len; /* u8 buffers: elem count == byte count */ +/* fs.readSync's offset check: an integer in [0, 2^53 - 1], Node's exact + * texts (validateOffset); false with ERR_OUT_OF_RANGE pending otherwise. */ +bool scr_fs_read_offset_chk(double offset) { char msg[160]; int mlen; - /* Node's exact texts: offset validates against MAX_SAFE_INTEGER's range - * form, length against the remaining window (validateOffset vs the - * buffer-bounds check in fs.readSync). */ char numbuf[40]; if (!(isfinite(offset) && trunc(offset) == offset)) { char recv[48]; @@ -4327,7 +4324,7 @@ double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length, "The value of \"offset\" is out of range. It must be an integer. Received %s", recv); scr_throw_error_msg_code(SCR_ERR_RANGE, msg, (size_t)mlen, "ERR_OUT_OF_RANGE"); - return 0; + return false; } if (offset < 0 || offset > 9007199254740991.0) { numbuf[scr_f64_to_str(offset, numbuf)] = 0; @@ -4335,8 +4332,19 @@ double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length, "The value of \"offset\" is out of range. It must be >= 0 && <= 9007199254740991. Received %s", numbuf); scr_throw_error_msg_code(SCR_ERR_RANGE, msg, (size_t)mlen, "ERR_OUT_OF_RANGE"); - return 0; + return false; } + return true; +} + +double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length, + double position) { + char msg[160]; + int mlen; + /* Node's exact texts: offset validates against MAX_SAFE_INTEGER's range + * form, length against the remaining window (validateOffset vs the + * buffer-bounds check in fs.readSync). */ + if (!scr_fs_read_offset_chk(offset)) return 0; if (!(isfinite(position) && trunc(position) == position)) { char recv[48]; scr_num_received(position, recv); @@ -4355,6 +4363,16 @@ double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length, scr_throw_error_msg_code(SCR_ERR_RANGE, msg, (size_t)mlen, "ERR_OUT_OF_RANGE"); return 0; } + return scr_fs_read_sync_at(fd, buf, offset, length, (int64_t)position); +} + +/* fs.readSync after its offset and position checks: `position` is exact, + * so a bigint position past 2^53 reads where it says. */ +double scr_fs_read_sync_at(double fd, ScrBytes *buf, double offset, double length, + int64_t position) { + size_t bytelen = buf->len; /* u8 buffers: elem count == byte count */ + char msg[160]; + int mlen; /* `length |= 0`, then Node's order: a zero length reads nothing after * the offset and position checks; otherwise the buffer must not be * empty and the window must fit it (validateOffsetLengthRead). */ @@ -4417,7 +4435,7 @@ double scr_fs_read_zero(double fd, double position) { if (!scr_fs_write_fd_valid(fd)) return 0; uint8_t dummy; ssize_t n = position < 0 ? read((int)fd, &dummy, 0) - : scr_fs_pread((int)fd, &dummy, 0, position); + : scr_fs_pread((int)fd, &dummy, 0, (int64_t)position); if (n < 0) scr_fs_throw_nopath(errno, "read"); return 0; } @@ -4498,7 +4516,8 @@ int scr_fs_raw_open(const char *path, int flags, int mode) { } int64_t scr_fs_raw_read(int fd, void *data, size_t length, double position) { - ssize_t n = position >= 0 ? scr_fs_pread(fd, data, length, position) : read(fd, data, length); + ssize_t n = position >= 0 ? scr_fs_pread(fd, data, length, (int64_t)position) + : read(fd, data, length); return n < 0 ? -errno : n; } diff --git a/tests/corpus/fs-read-bigint-position.cjs b/tests/corpus/fs-read-bigint-position.cjs new file mode 100644 index 000000000..b54f331cf --- /dev/null +++ b/tests/corpus/fs-read-bigint-position.cjs @@ -0,0 +1,28 @@ +// fs.read's callback forms with bigint positions read at the exact +// position, including past 2^53 (beyond the file's end, so nothing is +// read), and -1n reads at the current offset. +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); + +const dir = fs.mkdtempSync(path.join(os.tmpdir(), "scriptc-read-cb-position-")); +const file = path.join(dir, "data"); +fs.writeFileSync(file, "abc"); +const fd = fs.openSync(file, "r"); +const big = 9007199254740993n; + +function step(label, run, next) { + run((error, bytesRead, buffer) => { + console.log(label, error ? error.code : null, bytesRead, buffer ? buffer.toString("latin1", 0, bytesRead) : ""); + next(); + }); +} + +step("positional", (cb) => fs.read(fd, Buffer.alloc(1), 0, 1, big, cb), () => + step("options", (cb) => fs.read(fd, { buffer: Buffer.alloc(1), position: big }, cb), () => + step("buffer and options", (cb) => fs.read(fd, Buffer.alloc(2), { position: big }, cb), () => + step("small", (cb) => fs.read(fd, Buffer.alloc(2), 0, 2, 1n, cb), () => + step("current", (cb) => fs.read(fd, Buffer.alloc(1), 0, 1, -1n, cb), () => { + fs.closeSync(fd); + fs.rmSync(dir, { recursive: true }); + }))))); diff --git a/tests/corpus/fs-read-sync-bigint-position.cjs b/tests/corpus/fs-read-sync-bigint-position.cjs new file mode 100644 index 000000000..4b3a36c22 --- /dev/null +++ b/tests/corpus/fs-read-sync-bigint-position.cjs @@ -0,0 +1,32 @@ +// fs.readSync with bigint positions reads at the exact position, including +// past 2^53 (beyond a one-byte file's end, so it reads nothing), leaves the +// descriptor's offset unchanged, and -1n reads at the current offset. +// Positions outside [-1n, 2n ** 63n - 1n - length] throw ERR_OUT_OF_RANGE. +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); + +const dir = fs.mkdtempSync(path.join(os.tmpdir(), "scriptc-read-position-")); +const file = path.join(dir, "data"); +fs.writeFileSync(file, "xyz"); +const fd = fs.openSync(file, "r"); +const buffer = Buffer.alloc(2); +try { + console.log(fs.readSync(fd, buffer, 0, 1, 0n), buffer.toString("latin1", 0, 1)); + console.log(fs.readSync(fd, buffer, 0, 2, 1n), buffer.toString("latin1")); + console.log(fs.readSync(fd, buffer, 0, 1, 9007199254740993n)); + console.log(fs.readSync(fd, buffer, 0, 1, 9223372036854775806n)); + // The positioned reads above left the current offset at 0. + console.log(fs.readSync(fd, buffer, 0, 1, -1n), buffer.toString("latin1", 0, 1)); + console.log(fs.readSync(fd, buffer, 0, 1, -1n), buffer.toString("latin1", 0, 1)); + for (const position of [-2n, 9223372036854775807n]) { + try { + fs.readSync(fd, buffer, 0, 1, position); + } catch (error) { + console.log(error.code, error.message); + } + } +} finally { + fs.closeSync(fd); + fs.rmSync(dir, { recursive: true }); +} diff --git a/tests/harness/corpus-costs/fs-read-bigint-position.cjs.json b/tests/harness/corpus-costs/fs-read-bigint-position.cjs.json new file mode 100644 index 000000000..9c76a3418 --- /dev/null +++ b/tests/harness/corpus-costs/fs-read-bigint-position.cjs.json @@ -0,0 +1 @@ +{ "plain": 2700, "san": 4500 } diff --git a/tests/harness/corpus-costs/fs-read-sync-bigint-position.cjs.json b/tests/harness/corpus-costs/fs-read-sync-bigint-position.cjs.json new file mode 100644 index 000000000..9c76a3418 --- /dev/null +++ b/tests/harness/corpus-costs/fs-read-sync-bigint-position.cjs.json @@ -0,0 +1 @@ +{ "plain": 2700, "san": 4500 } From 96739536d2d30a9ce450ddb593efc8456e639ec6 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 29 Oct 2026 06:22:18 -0500 Subject: [PATCH 2/2] Read fs.read's position option once and check a bigint against length |= 0 fs.read's options forms read the position option a second time when it was not a bigint, so a getter ran twice and a stateful one could move the read. The option is now read once. A bigint position was checked against the length before `length |= 0`. A fractional or infinite length made the range check build its bound from a null BigInt and crash. The check and the read now take the converted length, so such a read completes with zero or the truncated count of bytes, as in Node. --- packages/runtime/src/scr_bytes_io.c | 45 ++++++++++++------------ tests/corpus/fs-read-bigint-position.cjs | 37 +++++++++++++++++-- 2 files changed, 57 insertions(+), 25 deletions(-) diff --git a/packages/runtime/src/scr_bytes_io.c b/packages/runtime/src/scr_bytes_io.c index 035795619..0d3156c26 100644 --- a/packages/runtime/src/scr_bytes_io.c +++ b/packages/runtime/src/scr_bytes_io.c @@ -167,6 +167,7 @@ static ScrDyn *scr_fs_cb_arg(ScrDyn *const *args, size_t argc, size_t index) { } static bool scr_fs_bigint_position_chk(ScrBigInt *position, double length); +static double scr_fs_int32(double n); static ScrDyn *scr_fs_cb_option(const ScrDyn *options, const char *name) { if (scr_fs_dyn_absent(options)) return scr_dyn_retain(scr_dyn_undefined()); @@ -1131,30 +1132,28 @@ static ScrDyn *scr_fs_cb_invoke(ScrStr *member, ScrDyn *const *args, size_t argc if (ownedBuffer) length = scr_fs_cb_option_number(options, "length", length); else if (readOp && count >= 4) length = readLength; else length = scr_fs_cb_number(ARG(3), "length", length); - /* A bigint read position takes readSync's range check and reads at - * its exact value: a double would round it past 2^53. */ - ScrDyn *bigPosition = NULL; - if (readOp) { - ScrDyn *value = ownedBuffer ? scr_fs_cb_option(options, "position") - : scr_dyn_retain(count > 4 ? ARG(4) : scr_dyn_undefined()); - if (value && value->kind == SCR_DYN_BIGINT) bigPosition = value; - else scr_dyn_release(value); - } - double position = bigPosition ? -1 - : ownedBuffer ? scr_fs_cb_option_number(options, "position", -1) - : scr_fs_cb_number(ARG(4), "position", -1); + /* The position option is read once, so a getter runs once. A bigint + * read position takes readSync's range check against `length |= 0` + * and reads at its exact value: a double would round it past 2^53. */ + ScrDyn *positionValue = ownedBuffer ? scr_fs_cb_option(options, "position") + : scr_dyn_retain(ARG(4)); + bool bigPosition = readOp && positionValue && positionValue->kind == SCR_DYN_BIGINT; + double position = bigPosition || !positionValue + ? -1 + : scr_fs_cb_number(positionValue, "position", -1); if (!scr_exc_pending()) { if (bigPosition) { + double count32 = scr_fs_int32(length); if (scr_fs_read_offset_chk(offset) && - scr_fs_bigint_position_chk(bigPosition->v.bigint, length)) - result = scr_dyn_new_num(scr_fs_read_sync_at(fd, bytes, offset, length, - scr_bigint_to_i64(bigPosition->v.bigint))); + scr_fs_bigint_position_chk(positionValue->v.bigint, count32)) + result = scr_dyn_new_num(scr_fs_read_sync_at(fd, bytes, offset, count32, + scr_bigint_to_i64(positionValue->v.bigint))); } else { result = scr_dyn_new_num(readOp ? scr_fs_read_sync(fd, bytes, offset, length, position) : scr_fs_write_sync(fd, bytes, offset, length, position)); } } - scr_dyn_release(bigPosition); + scr_dyn_release(positionValue); } scr_bytes_release(bytes); /* The completion retains the original buffer, including options.buffer. */ @@ -1938,14 +1937,16 @@ static bool scr_fs_length_int32(const ScrDyn *length, double *out) { } else if (!scr_dyn_number_coerce_js(length, &n)) { return false; } - if (!isfinite(n)) { - *out = 0; - return true; - } + *out = scr_fs_int32(n); + return true; +} + +/* ToInt32 of a number: 0 for NaN and the infinities. */ +static double scr_fs_int32(double n) { + if (!isfinite(n)) return 0; double m = fmod(trunc(n), 4294967296.0); if (m < 0) m += 4294967296.0; - *out = m >= 2147483648.0 ? m - 4294967296.0 : m; - return true; + return m >= 2147483648.0 ? m - 4294967296.0 : m; } /* A bigint read position in [-1n, 2n ** 63n - 1n - BigInt(length)] diff --git a/tests/corpus/fs-read-bigint-position.cjs b/tests/corpus/fs-read-bigint-position.cjs index b54f331cf..424506491 100644 --- a/tests/corpus/fs-read-bigint-position.cjs +++ b/tests/corpus/fs-read-bigint-position.cjs @@ -1,6 +1,8 @@ // fs.read's callback forms with bigint positions read at the exact // position, including past 2^53 (beyond the file's end, so nothing is -// read), and -1n reads at the current offset. +// read), and -1n reads at the current offset. A fractional or infinite +// length takes `length |= 0` before the position check, and a position +// getter in either options form runs once. const fs = require("node:fs"); const os = require("node:os"); const path = require("node:path"); @@ -23,6 +25,35 @@ step("positional", (cb) => fs.read(fd, Buffer.alloc(1), 0, 1, big, cb), () => step("buffer and options", (cb) => fs.read(fd, Buffer.alloc(2), { position: big }, cb), () => step("small", (cb) => fs.read(fd, Buffer.alloc(2), 0, 2, 1n, cb), () => step("current", (cb) => fs.read(fd, Buffer.alloc(1), 0, 1, -1n, cb), () => { - fs.closeSync(fd); - fs.rmSync(dir, { recursive: true }); + fractional(); }))))); + +function counted(values) { + const options = { calls: 0 }; + Object.defineProperty(options, "position", { + get() { + return values[options.calls++]; + }, + }); + return options; +} + +function fractional() { + step("fractional", (cb) => fs.read(fd, { buffer: Buffer.alloc(2), length: 0.5, position: 1n }, cb), () => + step("infinite", (cb) => fs.read(fd, Buffer.alloc(2), { length: Infinity, position: 1n }, cb), () => + step("truncated", (cb) => fs.read(fd, { buffer: Buffer.alloc(3), length: 2.7, position: 1n }, cb), () => { + const first = counted([0, 2]); + first.buffer = Buffer.alloc(1); + first.length = 1; + step("getter options", (cb) => fs.read(fd, first, cb), () => { + console.log("getter options calls", first.calls); + const second = counted([2, 0]); + second.length = 1; + step("getter buffer and options", (cb) => fs.read(fd, Buffer.alloc(1), second, cb), () => { + console.log("getter buffer and options calls", second.calls); + fs.closeSync(fd); + fs.rmSync(dir, { recursive: true }); + }); + }); + }))); +}