diff --git a/node/_fs/_fs_read.ts b/node/_fs/_fs_read.ts index a18b49210c86..bab584b632d2 100644 --- a/node/_fs/_fs_read.ts +++ b/node/_fs/_fs_read.ts @@ -1,10 +1,11 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. import { Buffer } from "../buffer.ts"; -import { assert } from "../../testing/asserts.ts"; +import { ERR_INVALID_ARG_TYPE } from "../internal/errors.ts"; import { - ERR_INVALID_ARG_TYPE, - ERR_INVALID_ARG_VALUE, -} from "../internal/errors.ts"; + validateOffsetLengthRead, + validatePosition, +} from "../internal/fs/utils.mjs"; +import { validateBuffer, validateInteger } from "../internal/validators.mjs"; type readOptions = { buffer: Buffer | Uint8Array; @@ -66,11 +67,10 @@ export function read( cb = optOrBufferOrCb; } else { offset = offsetOrCallback as number; + validateInteger(offset, "offset", 0); cb = callback; } - if (!cb) throw new ERR_INVALID_ARG_TYPE("cb", "Callback", cb); - if ( optOrBufferOrCb instanceof Buffer || optOrBufferOrCb instanceof Uint8Array ) { @@ -82,28 +82,32 @@ export function read( position = null; } else { const opt = optOrBufferOrCb as readOptions; + if ( + !(opt.buffer instanceof Buffer) && !(opt.buffer instanceof Uint8Array) + ) { + if (opt.buffer === null) { + // @ts-ignore: Intentionally create TypeError for passing test-fs-read.js#L87 + length = opt.buffer.byteLength; + } + throw new ERR_INVALID_ARG_TYPE("buffer", [ + "Buffer", + "TypedArray", + "DataView", + ], optOrBufferOrCb); + } offset = opt.offset ?? 0; buffer = opt.buffer ?? Buffer.alloc(16384); length = opt.length ?? buffer.byteLength; position = opt.position ?? null; } - assert(offset >= 0, "offset should be greater or equal to 0"); - assert( - offset + length <= buffer.byteLength, - `buffer doesn't have enough data: byteLength = ${buffer.byteLength}, offset + length = ${ - offset + length - }`, - ); - - if (buffer.byteLength == 0) { - throw new ERR_INVALID_ARG_VALUE( - "buffer", - buffer, - "is empty and cannot be written", - ); + if (position == null) { + position = -1; } + validatePosition(position); + validateOffsetLengthRead(offset, length, buffer.byteLength); + let err: Error | null = null, numberOfBytesRead: number | null = null; @@ -123,6 +127,8 @@ export function read( err = error instanceof Error ? error : new Error("[non-error thrown]"); } + if (!cb) throw new ERR_INVALID_ARG_TYPE("cb", "Callback", cb); + if (err) { (callback as (err: Error) => void)(err); } else { @@ -154,20 +160,19 @@ export function readSync( ): number { let offset = 0; - if (length == null) { - length = 0; + if (typeof fd !== "number") { + throw new ERR_INVALID_ARG_TYPE("fd", "number", fd); } - if (buffer.byteLength == 0) { - throw new ERR_INVALID_ARG_VALUE( - "buffer", - buffer, - "is empty and cannot be written", - ); + validateBuffer(buffer); + + if (length == null) { + length = 0; } if (typeof offsetOrOpt === "number") { offset = offsetOrOpt; + validateInteger(offset, "offset", 0); } else { const opt = offsetOrOpt as readSyncOptions; offset = opt.offset ?? 0; @@ -175,13 +180,12 @@ export function readSync( position = opt.position ?? null; } - assert(offset >= 0, "offset should be greater or equal to 0"); - assert( - offset + length <= buffer.byteLength, - `buffer doesn't have enough data: byteLength = ${buffer.byteLength}, offset + length = ${ - offset + length - }`, - ); + if (position == null) { + position = -1; + } + + validatePosition(position); + validateOffsetLengthRead(offset, length, buffer.byteLength); let currentPosition = 0; if (typeof position === "number" && position >= 0) { diff --git a/node/_tools/config.json b/node/_tools/config.json index 0daf7cfd8f07..2fb0778128fc 100644 --- a/node/_tools/config.json +++ b/node/_tools/config.json @@ -45,7 +45,6 @@ "test-fs-append-file.js", "test-fs-chmod-mask.js", "test-fs-chmod.js", - "test-fs-read.js", "test-fs-rmdir-recursive.js", "test-fs-write-file.js", "test-fs-write.js", @@ -306,6 +305,8 @@ "test-fs-read-stream-resume.js", "test-fs-read-stream-throw-type-error.js", "test-fs-read-stream.js", + "test-fs-read-type.js", + "test-fs-read-zero-length.js", "test-fs-read.js", "test-fs-readdir-stack-overflow.js", "test-fs-readdir.js", diff --git a/node/_tools/test/parallel/test-fs-read-type.js b/node/_tools/test/parallel/test-fs-read-type.js new file mode 100644 index 000000000000..31efad6c3ffe --- /dev/null +++ b/node/_tools/test/parallel/test-fs-read-type.js @@ -0,0 +1,250 @@ +// deno-fmt-ignore-file +// deno-lint-ignore-file + +// Copyright Joyent and Node contributors. All rights reserved. MIT license. +// Taken from Node 18.8.0 +// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually + +'use strict'; +const common = require('../common'); +const fs = require('fs'); +const assert = require('assert'); +const fixtures = require('../common/fixtures'); + +const filepath = fixtures.path('x.txt'); +const fd = fs.openSync(filepath, 'r'); +const expected = 'xyz\n'; + + +// Error must be thrown with string +assert.throws( + () => fs.read(fd, expected.length, 0, 'utf-8', common.mustNotCall()), + { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError', + message: 'The "buffer" argument must be an instance of Buffer, ' + + 'TypedArray, or DataView. Received type number (4)' + } +); + +[true, null, undefined, () => {}, {}].forEach((value) => { + assert.throws(() => { + fs.read(value, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 0, + common.mustNotCall()); + }, { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' + }); +}); + +assert.throws(() => { + fs.read(fd, + Buffer.allocUnsafe(expected.length), + -1, + expected.length, + 0, + common.mustNotCall()); +}, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', +}); + +assert.throws(() => { + fs.read(fd, + Buffer.allocUnsafe(expected.length), + NaN, + expected.length, + 0, + common.mustNotCall()); +}, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "offset" is out of range. It must be an integer. ' + + 'Received NaN' +}); + +assert.throws(() => { + fs.read(fd, + Buffer.allocUnsafe(expected.length), + 0, + -1, + 0, + common.mustNotCall()); +}, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "length" is out of range. ' + + 'It must be >= 0. Received -1' +}); + +[true, () => {}, {}, ''].forEach((value) => { + assert.throws(() => { + fs.read(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + value, + common.mustNotCall()); + }, { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' + }); +}); + +[0.5, 2 ** 53, 2n ** 63n].forEach((value) => { + assert.throws(() => { + fs.read(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + value, + common.mustNotCall()); + }, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError' + }); +}); + +fs.read(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 0n, + common.mustSucceed()); + +fs.read(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 2n ** 53n - 1n, + common.mustCall((err) => { + if (err) { + if (common.isIBMi) + assert.strictEqual(err.code, 'EOVERFLOW'); + else + assert.strictEqual(err.code, 'EFBIG'); + } + })); + +assert.throws( + () => fs.readSync(fd, expected.length, 0, 'utf-8'), + { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError', + message: 'The "buffer" argument must be an instance of Buffer, ' + + 'TypedArray, or DataView. Received type number (4)' + } +); + +[true, null, undefined, () => {}, {}].forEach((value) => { + assert.throws(() => { + fs.readSync(value, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 0); + }, { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' + }); +}); + +assert.throws(() => { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + -1, + expected.length, + 0); +}, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', +}); + +assert.throws(() => { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + NaN, + expected.length, + 0); +}, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "offset" is out of range. It must be an integer. ' + + 'Received NaN' +}); + +assert.throws(() => { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + 0, + -1, + 0); +}, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "length" is out of range. ' + + 'It must be >= 0. Received -1' +}); + +assert.throws(() => { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length + 1, + 0); +}, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError', + message: 'The value of "length" is out of range. ' + + 'It must be <= 4. Received 5' +}); + +[true, () => {}, {}, ''].forEach((value) => { + assert.throws(() => { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + value); + }, { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' + }); +}); + +[0.5, 2 ** 53, 2n ** 63n].forEach((value) => { + assert.throws(() => { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + value); + }, { + code: 'ERR_OUT_OF_RANGE', + name: 'RangeError' + }); +}); + +fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 0n); + +try { + fs.readSync(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 2n ** 53n - 1n); +} catch (err) { + // On systems where max file size is below 2^53-1, we'd expect a EFBIG error. + // This is not using `assert.throws` because the above call should not raise + // any error on systems that allows file of that size. + if (err.code !== 'EFBIG' && !(common.isIBMi && err.code === 'EOVERFLOW')) + throw err; +} diff --git a/node/_tools/test/parallel/test-fs-read-zero-length.js b/node/_tools/test/parallel/test-fs-read-zero-length.js new file mode 100644 index 000000000000..f643b8498bea --- /dev/null +++ b/node/_tools/test/parallel/test-fs-read-zero-length.js @@ -0,0 +1,25 @@ +// deno-fmt-ignore-file +// deno-lint-ignore-file + +// Copyright Joyent and Node contributors. All rights reserved. MIT license. +// Taken from Node 18.8.0 +// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually + +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const fs = require('fs'); +const filepath = fixtures.path('x.txt'); +const fd = fs.openSync(filepath, 'r'); +const bufferAsync = Buffer.alloc(0); +const bufferSync = Buffer.alloc(0); + +fs.read(fd, bufferAsync, 0, 0, 0, common.mustCall((err, bytesRead) => { + assert.strictEqual(bytesRead, 0); + assert.deepStrictEqual(bufferAsync, Buffer.alloc(0)); +})); + +const r = fs.readSync(fd, bufferSync, 0, 0, 0); +assert.deepStrictEqual(bufferSync, Buffer.alloc(0)); +assert.strictEqual(r, 0); diff --git a/node/_tools/test/parallel/test-fs-read.js b/node/_tools/test/parallel/test-fs-read.js index 09e18f70de84..f3f72e73cca3 100644 --- a/node/_tools/test/parallel/test-fs-read.js +++ b/node/_tools/test/parallel/test-fs-read.js @@ -84,22 +84,21 @@ assert.throws( } ); -// TODO(wafuwafu13): Enable this -// assert.throws( -// () => fs.read(fd, { buffer: null }, common.mustNotCall()), -// /TypeError: Cannot read properties of null \(reading 'byteLength'\)/, -// 'throws when options.buffer is null' -// ); +assert.throws( + () => fs.read(fd, { buffer: null }, common.mustNotCall()), + /TypeError: Cannot read properties of null \(reading 'byteLength'\)/, + 'throws when options.buffer is null' +); -// assert.throws( -// () => fs.readSync(fd, { buffer: null }), -// { -// name: 'TypeError', -// message: 'The "buffer" argument must be an instance of Buffer, ' + -// 'TypedArray, or DataView. Received an instance of Object', -// }, -// 'throws when options.buffer is null' -// ); +assert.throws( + () => fs.readSync(fd, { buffer: null }), + { + name: 'TypeError', + message: 'The "buffer" argument must be an instance of Buffer, ' + + 'TypedArray, or DataView. Received an instance of Object', + }, + 'throws when options.buffer is null' +); assert.throws( () => fs.read(null, Buffer.alloc(1), 0, 1, 0),