Skip to content

Commit

Permalink
fix fs-read-empty-buffer.test.js (#14316)
Browse files Browse the repository at this point in the history
  • Loading branch information
nektro authored Oct 8, 2024
1 parent c41ff9d commit 1ce2d0e
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 6 deletions.
21 changes: 16 additions & 5 deletions src/bun.js/node/node_fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3065,7 +3065,8 @@ pub const Arguments = struct {

if (exception.* != null) return null;

const buffer = Buffer.fromJS(ctx.ptr(), arguments.next() orelse {
const buffer_value = arguments.next();
const buffer: Buffer = Buffer.fromJS(ctx.ptr(), buffer_value orelse {
if (exception.* == null) {
JSC.throwInvalidArguments(
"buffer is required",
Expand Down Expand Up @@ -3096,6 +3097,7 @@ pub const Arguments = struct {
.buffer = buffer,
};

var defined_length = false;
if (arguments.next()) |current| {
arguments.eat();
if (current.isNumber() or current.isBigInt()) {
Expand All @@ -3108,14 +3110,11 @@ pub const Arguments = struct {

const arg_length = arguments.next().?;
arguments.eat();
defined_length = true;

if (arg_length.isNumber() or arg_length.isBigInt()) {
args.length = arg_length.to(u52);
}
if (args.length == 0) {
JSC.throwInvalidArguments("length must be greater than 0", .{}, ctx, exception);
return null;
}

if (arguments.next()) |arg_position| {
arguments.eat();
Expand All @@ -3134,6 +3133,7 @@ pub const Arguments = struct {
if (num.isNumber() or num.isBigInt()) {
args.length = num.to(u52);
}
defined_length = true;
}

if (current.getTruthy(ctx.ptr(), "position")) |num| {
Expand All @@ -3144,6 +3144,12 @@ pub const Arguments = struct {
}
}

if (defined_length and args.length > 0 and buffer.slice().len == 0) {
var formatter = bun.JSC.ConsoleObject.Formatter{ .globalThis = ctx };
ctx.ERR_INVALID_ARG_VALUE("The argument 'buffer' is empty and cannot be written. Received {}", .{buffer_value.?.toFmt(&formatter)}).throw();
return null;
}

return args;
}
};
Expand Down Expand Up @@ -5147,6 +5153,11 @@ pub const NodeFS = struct {
}

pub fn read(this: *NodeFS, args: Arguments.Read, comptime flavor: Flavor) Maybe(Return.Read) {
const len1 = args.buffer.slice().len;
const len2 = args.length;
if (len1 == 0 or len2 == 0) {
return Maybe(Return.Read).initResult(.{ .bytes_read = 0 });
}
return if (args.position != null)
this._pread(
args,
Expand Down
7 changes: 6 additions & 1 deletion test/js/node/fs/fs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,11 @@ describe("readSync", () => {
}
closeSync(fd);
});

it("works with invalid fd but zero length",()=>{
expect(readSync(2147483640, Buffer.alloc(0))).toBe(0);
expect(readSync(2147483640, Buffer.alloc(10), 0, 0, 0)).toBe(0);
})
});

it("writevSync", () => {
Expand Down Expand Up @@ -3167,7 +3172,7 @@ it("new Stats", () => {
it("test syscall errno, issue#4198", () => {
const path = `${tmpdir()}/non-existent-${Date.now()}.txt`;
expect(() => openSync(path, "r")).toThrow("No such file or directory");
expect(() => readSync(2147483640, Buffer.alloc(0))).toThrow("Bad file descriptor");
expect(() => readSync(2147483640, Buffer.alloc(1))).toThrow("Bad file descriptor");
expect(() => readlinkSync(path)).toThrow("No such file or directory");
expect(() => realpathSync(path)).toThrow("No such file or directory");
expect(() => readFileSync(path)).toThrow("No such file or directory");
Expand Down
47 changes: 47 additions & 0 deletions test/js/node/test/parallel/fs-read-empty-buffer.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//#FILE: test-fs-read-empty-buffer.js
//#SHA1: a2dc2c25e5a712b62c41298f885df24dd6106646
//-----------------
'use strict';
const fs = require('fs');
const path = require('path');

const filepath = path.resolve(__dirname, 'x.txt');
let fd;

beforeAll(() => {
// Create a test file
fs.writeFileSync(filepath, 'test content');
fd = fs.openSync(filepath, 'r');
});

afterAll(() => {
fs.closeSync(fd);
fs.unlinkSync(filepath);
});

const buffer = new Uint8Array();

test('fs.readSync throws ERR_INVALID_ARG_VALUE for empty buffer', () => {
expect(() => fs.readSync(fd, buffer, 0, 10, 0)).toThrow(expect.objectContaining({
code: 'ERR_INVALID_ARG_VALUE',
message: expect.stringContaining('The argument \'buffer\' is empty and cannot be written')
}));
});

test('fs.read throws ERR_INVALID_ARG_VALUE for empty buffer', () => {
expect(() => fs.read(fd, buffer, 0, 1, 0, () => {})).toThrow(expect.objectContaining({
code: 'ERR_INVALID_ARG_VALUE',
message: expect.stringContaining('The argument \'buffer\' is empty and cannot be written')
}));
});

test('fsPromises.filehandle.read rejects with ERR_INVALID_ARG_VALUE for empty buffer', async () => {
const filehandle = await fs.promises.open(filepath, 'r');
await expect(filehandle.read(buffer, 0, 1, 0)).rejects.toThrow(expect.objectContaining({
code: 'ERR_INVALID_ARG_VALUE',
message: expect.stringContaining('The argument \'buffer\' is empty and cannot be written')
}));
await filehandle.close();
});

//<#END_FILE: test-fs-read-empty-buffer.js

0 comments on commit 1ce2d0e

Please sign in to comment.