Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fs/promises] FileHandle.read does not respect reading from position=0 #40715

Closed
mihilmy opened this issue Nov 3, 2021 · 0 comments
Closed
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@mihilmy
Copy link
Contributor

mihilmy commented Nov 3, 2021

Version

v14.18.1

Platform

Linux 5.4.141-78.230.amzn2int.x86_64 Fri Aug 27 01:18:39 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

Code

const fs = require('fs/promises');

let handle;

async function main() {
  handle = await fs.open(__filename, 'r');
  await read(0, 5);  // Should get const ✅
  await read(11, 7); // Should get require  ✅
  await read(0, 5);  // Should get const again; zero is not honored ❌ 
  await read(11, 7); // Should get require ✅
}

async function read(start, length) {
  const buffer = Buffer.alloc(length);
  await handle.read({ buffer, position: start, length });
  console.log(buffer.toString());
}

Console Output

const
require
 fs =
require

How often does it reproduce? Is there a required condition?

It's reproduced whenever you try to use 0 as the seek position

What is the expected behavior?

I would expect to be able to navigate back and forth using different positions of the file..

What do you see instead?

I am able to do that only if the position > 0 (STRICTLY greater than)

Additional information

I believe the cause might be this line here:

position = bufferOrOptions.position || null;

Since 0 is falsy in javascript this might lead to this behavior. I am happy to put in a pull request to fix if indeed this is the cause.

@mihilmy mihilmy changed the title fs.read does not respect reading from position=0 [fs/promises] FileHandle.read does not respect reading from position=0 Nov 3, 2021
@VoltrexKeyva VoltrexKeyva added the fs Issues and PRs related to the fs subsystem / file system. label Nov 3, 2021
@VoltrexKeyva VoltrexKeyva linked a pull request Nov 3, 2021 that will close this issue
mihilmy added a commit to mihilmy/node that referenced this issue Nov 4, 2021
When the file read position is moved passing zero is
not respected and `null` is used instead. PR fixes the
issues by using nullish coalescing which will return
the rhs only when the lhs is `null` or `undefined`;
respecting the zero.

Fixes: nodejs#40715
Refs: nodejs#40716
Trott pushed a commit to mihilmy/node that referenced this issue Nov 9, 2021
When the file read position is moved passing zero is
not respected and `null` is used instead. PR fixes the
issues by using nullish coalescing which will return
the rhs only when the lhs is `null` or `undefined`;
respecting the zero.

Fixes: nodejs#40715
Refs: nodejs#40716
@Trott Trott closed this as completed in 2037ee8 Nov 12, 2021
targos pushed a commit that referenced this issue Nov 21, 2021
When the file read position is moved passing zero is
not respected and `null` is used instead. PR fixes the
issues by using nullish coalescing which will return
the rhs only when the lhs is `null` or `undefined`;
respecting the zero.

Fixes: #40715

PR-URL: #40716
Fixes: #40699
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
richardlau pushed a commit that referenced this issue Jan 25, 2022
When the file read position is moved passing zero is
not respected and `null` is used instead. PR fixes the
issues by using nullish coalescing which will return
the rhs only when the lhs is `null` or `undefined`;
respecting the zero.

Fixes: #40715

PR-URL: #40716
Fixes: #40699
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
danielleadams pushed a commit that referenced this issue Jan 30, 2022
When the file read position is moved passing zero is
not respected and `null` is used instead. PR fixes the
issues by using nullish coalescing which will return
the rhs only when the lhs is `null` or `undefined`;
respecting the zero.

Fixes: #40715

PR-URL: #40716
Fixes: #40699
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
danielleadams pushed a commit that referenced this issue Feb 1, 2022
When the file read position is moved passing zero is
not respected and `null` is used instead. PR fixes the
issues by using nullish coalescing which will return
the rhs only when the lhs is `null` or `undefined`;
respecting the zero.

Fixes: #40715

PR-URL: #40716
Fixes: #40699
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants