readFile(fd) does not read the whole file (at least on Windows, possibly other platforms)

[jorangreef]

• Version: v7.1.0
• Platform: Windows 10 64-bit
• Subsystem: fs

I wrote a small test that passes on Mac and Linux but throws on Windows:

var fs = require('fs');
var buffer = Buffer.alloc(8192);
var path = 'test-sparse-read-file';
var fd = fs.openSync(path, 'w+');
console.log('created or truncated, sparse file size is now: ' + fs.fstatSync(fd).size);

console.log('writing 1 byte at offset ' + (8192 - 1));
fs.writeSync(fd, Buffer.alloc(1, 1), 0, 1, 8192 - 1);
console.log('sparse file size is now: ' + fs.fstatSync(fd).size + '\r\n');

console.log('reading 8192 bytes using readSync()...');
var bytesRead = fs.readSync(fd, buffer, 0, 8192, 0);
if (bytesRead !== 8192) {
  throw new Error('readSync() read ' + bytesRead + ' bytes');
} else {
  console.log('readSync() read ' + bytesRead + ' bytes');
}

console.log('\r\nreading entire file using readFileSync()...');
var result = fs.readFileSync(fd);
if (result.length !== buffer.length) {
  throw new Error('readFileSync returned a buffer with length ' + result.length);
} else {
  console.log('readFileSync returned a buffer with length ' + result.length);
}

fs.closeSync(fd);
fs.unlinkSync(path);
console.log('\r\nPASSED');

Linux:

created or truncated, sparse file size is now: 0
writing 1 byte at offset 8191
sparse file size is now: 8192

reading 8192 bytes using readSync()...
readSync() read 8192 bytes

reading entire file using readFileSync()...
readFileSync returned a buffer with length 8192

PASSED

Windows:

created or truncated, sparse file size is now: 0
writing 1 byte at offset 8191
sparse file size is now: 8192

reading 8192 bytes using readSync()...
readSync() read 8192 bytes

reading entire file using readFileSync()...
C:\Users\Joran\test-sparse.js:22
  throw new Error('readFileSync returned a buffer with length ' + result.length);
  ^

Error: readFileSync returned a buffer with length 0
    at Object.<anonymous> (C:\Users\Joran\test-sparse.js:22:9)
    at Module._compile (module.js:573:32)
    at Object.Module._extensions..js (module.js:582:10)
    at Module.load (module.js:490:32)
    at tryModuleLoad (module.js:449:12)
    at Function.Module._load (module.js:441:3)
    at Module.runMain (module.js:607:10)
    at run (bootstrap_node.js:420:7)
    at startup (bootstrap_node.js:139:9)
    at bootstrap_node.js:535:3

The test shows that the problem does not appear to be with read() but only with readFileSync().

readFile() has the same issue.

[vsemozhetbyt]

It seems sparseness is not the cause. This is also throws (Node.js 7.1.0 on Windows 7 x64):

var fs = require('fs');
var buffer = Buffer.alloc(8192);
var path = 'test-read-file';
var fd = fs.openSync(path, 'w+');
console.log('created or truncated, file size is now: ' + fs.fstatSync(fd).size);

console.log('writing 8192 bytes at offset ' + (0));
fs.writeSync(fd, Buffer.alloc(8192, 1), 0, 8192, 0);
console.log('file size is now: ' + fs.fstatSync(fd).size + '\r\n');

console.log('reading 8192 bytes using readSync()...');
var bytesRead = fs.readSync(fd, buffer, 0, 8192, 0);
if (bytesRead !== 8192) {
  throw new Error('readSync() read ' + bytesRead + ' bytes');
} else {
  console.log('readSync() read ' + bytesRead + ' bytes');
}

console.log('\r\nreading entire file using readFileSync()...');
var result = fs.readFileSync(fd);
if (result.length !== buffer.length) {
  throw new Error('readFileSync returned a buffer with length ' + result.length);
} else {
  console.log('readFileSync returned a buffer with length ' + result.length);
}

fs.closeSync(fd);
fs.unlinkSync(path);
console.log('\r\nPASSED');

[vsemozhetbyt]

It seems the cause is this: after writeSync and readSync, the readFileSync reads from their end fd position. See how error message changes in this case:

// ...
console.log('reading 8191 bytes using readSync()...');
var bytesRead = fs.readSync(fd, buffer, 0, 8191, 0);
if (bytesRead !== 8191) {
  throw new Error('readSync() read ' + bytesRead + ' bytes');
} else {
  console.log('readSync() read ' + bytesRead + ' bytes');
}
// ...

Error: readFileSync returned a buffer with length 1

Or in this case:

// ...
console.log('reading 1 bytes using readSync()...');
var bytesRead = fs.readSync(fd, buffer, 0, 1, 0);
if (bytesRead !== 1) {
  throw new Error('readSync() read ' + bytesRead + ' bytes');
} else {
  console.log('readSync() read ' + bytesRead + ' bytes');
}
// ...

Error: readFileSync returned a buffer with length 8191

While if you use path string instead of fd in your initial code all is OK:

// ...
var result = fs.readFileSync('test-sparse-read-file');
// ...

created or truncated, sparse file size is now: 0
writing 1 byte at offset 8191
sparse file size is now: 8192

reading 8192 bytes using readSync()...
readSync() read 8192 bytes

reading entire file using readFileSync()...
readFileSync returned a buffer with length 8192

PASSED

[vsemozhetbyt]

So the bug seems to be this one: readFile() and readFileSync() don't rewind fd position after writeSync and readSync on the same fd on Windows, while they do rewind on Mac and Linux (I can't confirm the last though).

[sam-github]

Your script at #9671 (comment) passes on OS X, I don't have a Windows to try on.

The function used to read files is fs.readSync(fd, buffer, pos, len), it calls http://man7.org/linux/man-pages/man2/pread.2.html, and it reads from a specific offset, and it does not change the file offset (per docs). In other words, no explicit "rewinding" of the fd is needed on Linux, readFile reads from explicit offsets, not the O/S maintained offset.

I wonder, could uv's emulation of pread for Windows not be correct, somehow? Could you try some experiments with fs.read/fs.readSync to see if they work incorrectly, bypassing the readFile wrappers?

[vsemozhetbyt]

@sam-github I'll try, but let's simplify it first. This throws with Node.js 7.1.0 on Windows 7 x64:

'use strict';

const assert = require('assert');
const fs = require('fs');

const filleLength = 2;
const path = 'test-read-file';
const fd = fs.openSync(path, 'w+');

fs.writeSync(fd, Buffer.alloc(filleLength, 1), 0, filleLength, 0);

assert.strictEqual(fs.readFileSync(fd).length, filleLength);

AssertionError: 0 === 2

Does it throw on Linux or Mac?

[sam-github]

Does not throw on unixen, as expected.

Please remove the complex ball of js code that is readFile* from the picture, and replace that last line with assert.equal(filleLength, fs.readSync(fd, buf, 0, 10, 0), which I strongly suspect will not pass on Windows.

[vsemozhetbyt]

So, if I get it right, with some corrections (buf => Buffer.alloc(filleLength, 1) and 10 => filleLength), this is the new test code:

const assert = require('assert');
const fs = require('fs');

const filleLength = 2;
const path = 'test-read-file';
const fd = fs.openSync(path, 'w+');

fs.writeSync(fd, Buffer.alloc(filleLength, 1), 0, filleLength, 0);

assert.equal(filleLength, fs.readSync(fd, Buffer.alloc(filleLength, 1), 0, filleLength, 0));

It passes with Node.js 7.1.0 on Windows 7 x64. But this throws (nota bene position is null, so it is current after writeSync):

const assert = require('assert');
const fs = require('fs');

const filleLength = 2;
const path = 'test-read-file';
const fd = fs.openSync(path, 'w+');

fs.writeSync(fd, Buffer.alloc(filleLength, 1), 0, filleLength, 0);

assert.equal(filleLength, fs.readSync(fd, Buffer.alloc(filleLength, 1), 0, filleLength, null));

AssertionError: 2 == 0

Does this last code throw on Linux or Mac? It should, if I get it right. So maybe we should return to previous one.

[sam-github]

/to @nodejs/platform-windows

[seishun]

and it does not change the file offset (per docs).

Are there any fs functions that are supposed to increment file offset?

[vsemozhetbyt]

It seems I've found the cause.

fs.readFileSync calls tryReadSync() at line 485. tryReadSync() calls fs.readSync() at line 456, but with wrong arguments.

Compare fs.readSync() signature at line 622:

fs.readSync = function(fd, buffer, offset, length, position) {

and the call from tryReadSync() at line 456

bytesRead = fs.readSync(fd, buffer, pos, len);

so position is allways undefined, ie current.

[sam-github]

@seishun fs.read/write[Sync] update the file offset, they use read(2) and write(2).

@vsemozhetbyt got time to PR a fix + regression test? It would be great if you did.

[vsemozhetbyt]

I'm afraid I have not the experience and knowledge required to edit the core and to make appropriate tests(