GeezerVison is showing github.com — fs: harden fs.readSync(buffer, options) typecheck · nodejs/node@41a6d82
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/fs.js +9-7Lines changed: 9 additions & 7 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -701,26 +701,28 @@ ObjectDefineProperty(read, kCustomPromisifyArgsSymbol, | 701 | 701 | * offset?: number; | 702 | 702 | * length?: number; | 703 | 703 | * position?: number | bigint | null; | 704 | | - * }} [offset] | | 704 | + * }} [offsetOrOptions] | 705 | 705 | * @returns {number} | 706 | 706 | */ | 707 | | -function readSync(fd, buffer, offset, length, position) { | | 707 | +function readSync(fd, buffer, offsetOrOptions, length, position) { | 708 | 708 | fd = getValidatedFd(fd); | 709 | 709 |
| 710 | 710 | validateBuffer(buffer); | 711 | 711 |
| 712 | | - if (arguments.length <= 3) { | 713 | | - // Assume fs.readSync(fd, buffer, options) | 714 | | - const options = offset || kEmptyObject; | | 712 | + let offset = offsetOrOptions; | | 713 | + if (arguments.length <= 3 || typeof offsetOrOptions === 'object') { | | 714 | + if (offsetOrOptions !== undefined) { | | 715 | + validateObject(offsetOrOptions, 'options', { nullable: true }); | | 716 | + } | 715 | 717 |
| 716 | 718 | ({ | 717 | 719 | offset = 0, | 718 | 720 | length = buffer.byteLength - offset, | 719 | 721 | position = null, | 720 | | - } = options); | | 722 | + } = offsetOrOptions ?? kEmptyObject); | 721 | 723 | } | 722 | 724 |
| 723 | | - if (offset == null) { | | 725 | + if (offset === undefined) { | 724 | 726 | offset = 0; | 725 | 727 | } else { | 726 | 728 | validateInteger(offset, 'offset', 0); | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-fs-readSync-optional-params.js +25-9Lines changed: 25 additions & 9 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -8,13 +8,20 @@ const filepath = fixtures.path('x.txt'); | 8 | 8 |
| 9 | 9 | const expected = Buffer.from('xyz\n'); | 10 | 10 |
| 11 | | -function runTest(defaultBuffer, options) { | | 11 | +function runTest(defaultBuffer, options, errorCode = false) { | 12 | 12 | let fd; | 13 | 13 | try { | 14 | 14 | fd = fs.openSync(filepath, 'r'); | 15 | | - const result = fs.readSync(fd, defaultBuffer, options); | 16 | | - assert.strictEqual(result, expected.length); | 17 | | - assert.deepStrictEqual(defaultBuffer, expected); | | 15 | + if (errorCode) { | | 16 | + assert.throws( | | 17 | + () => fs.readSync(fd, defaultBuffer, options), | | 18 | + { code: errorCode } | | 19 | + ); | | 20 | + } else { | | 21 | + const result = fs.readSync(fd, defaultBuffer, options); | | 22 | + assert.strictEqual(result, expected.length); | | 23 | + assert.deepStrictEqual(defaultBuffer, expected); | | 24 | + } | 18 | 25 | } finally { | 19 | 26 | if (fd != null) fs.closeSync(fd); | 20 | 27 | } | @@ -31,7 +38,6 @@ for (const options of [ | 31 | 38 | { length: expected.length, position: 0 }, | 32 | 39 | { offset: 0, length: expected.length, position: 0 }, | 33 | 40 |
| 34 | | - { offset: null }, | 35 | 41 | { position: null }, | 36 | 42 | { position: -1 }, | 37 | 43 | { position: 0n }, | @@ -41,17 +47,27 @@ for (const options of [ | 41 | 47 | null, | 42 | 48 | undefined, | 43 | 49 |
| 44 | | - // Test if bad params are interpreted as default (not mandatory) | | 50 | + // Test malicious corner case: it works as {length: 4} but not intentionally | | 51 | + new String('4444'), | | 52 | +]) { | | 53 | + runTest(Buffer.allocUnsafe(expected.length), options); | | 54 | +} | | 55 | + | | 56 | +for (const options of [ | | 57 | + | | 58 | + // Test various invalid options | 45 | 59 | false, | 46 | 60 | true, | 47 | 61 | Infinity, | 48 | 62 | 42n, | 49 | 63 | Symbol(), | | 64 | + 'amString', | | 65 | + [], | | 66 | + () => {}, | 50 | 67 |
| 51 | | - // Test even more malicious corner cases | | 68 | + // Test if arbitrary entity with expected .length is not mistaken for options | 52 | 69 | '4'.repeat(expected.length), | 53 | | - new String('4444'), | 54 | 70 | [4, 4, 4, 4], | 55 | 71 | ]) { | 56 | | - runTest(Buffer.allocUnsafe(expected.length), mustNotMutateObjectDeep(options)); | | 72 | + runTest(Buffer.allocUnsafe(expected.length), mustNotMutateObjectDeep(options), 'ERR_INVALID_ARG_TYPE'); | 57 | 73 | } |
|