fs: keep fs.promises.readFile read until EOF is reached

[kylo5aby]

• For cases where the size==0 of the file to read, ensure it ends upon reaching EOF.

Fixes: #52155

[H4ad]

The PR looks great, can you add a test just to ensure the issue is fixed?

[BridgeAR]

This is looking good. I guess we can improve the conditions a tad with my suggestions and I wonder if we have to do the same for the non-encoding situation. Is that also impacted? Let's add regression tests for both to be safe.

[BridgeAR]

What about changing the chunkedRead above instead:

const chunkedRead = length > kReadFileBufferLength || size === 0;

That would automatically check for it and it's one boolean check in the loop less.

[kylo5aby]

I believe the premise here for chunkedRead is already knowing the size of the file, meaning size === 0 cannot imply the need for chunkedRead, have I misunderstand something?

[BridgeAR]

Suggested change

	result += decoder.write(noSize && bytesRead !== kReadFileUnknownBufferLength ?
	buffer.subarray(0, bytesRead) : buffer);
	const writeBuffer = bytesRead !== buffer.length ?
	buffer.subarray(0, bytesRead) :
	buffer;
	result += decoder.write(writeBuffer);

[kylo5aby]

This is looking good. I guess we can improve the conditions a tad with my suggestions and I wonder if we have to do the same for the non-encoding situation. Is that also impacted? Let's add regression tests for both to be safe.

[kylo5aby]

This is looking good. I guess we can improve the conditions a tad with my suggestions and I wonder if we have to do the same for the non-encoding situation. Is that also impacted? Let's add regression tests for both to be safe.

Thanks for the reminder :)

[kylo5aby]

I have added some regression tests. PTAL @H4ad @BridgeAR

[kylo5aby]

could you take a look? @legendecas

[legendecas]

If I understand correctly, this is not a buffer for a "write" operation. Instead, it is a result of "read".

Suggested change

	const writeBuffer = bytesRead !== buffer.length ?
	const readBuffer = bytesRead !== buffer.length ?

[kylo5aby]

Resolved. Thanks!

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/58480/

[nodejs-github-bot]

Landed in ff7910b

[orgads]

Can this be backported to v20?