Skip to content

Commit

Permalink
fs: expand known issue to AIX and smartOS
Browse files Browse the repository at this point in the history
  • Loading branch information
refack committed Nov 16, 2018
1 parent cfb7408 commit c99e3d1
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 7 deletions.
6 changes: 5 additions & 1 deletion doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -632,13 +632,15 @@ This method is only valid when using [`fs.lstat()`][].

The numeric identifier of the device containing the file.

*Note*: The `number` version is unreliable as values often overflow.

### stats.ino

* {number|bigint}

The file system specific "Inode" number for the file.

*Note*: The `number` version is unreliable on Windows as values often overflow.
*Note*: The `number` version is unreliable as values often overflow.

### stats.mode

Expand Down Expand Up @@ -670,6 +672,8 @@ The numeric group identifier of the group that owns the file (POSIX).

A numeric device identifier if the file is considered "special".

*Note*: The `number` version is unreliable as values often overflow.

### stats.size

* {number|bigint}
Expand Down
6 changes: 4 additions & 2 deletions src/node_file.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,14 @@ constexpr uint64_t ToNative(uv_timespec_t ts) {
template <typename NativeT, typename V8T>
constexpr void FillStatsArray(AliasedBuffer<NativeT, V8T>* fields,
const uv_stat_t* s, const size_t offset = 0) {
fields->SetValue(offset + 0, gsl::narrow<NativeT>(s->st_dev));
// Using the noop `narrow_cast` since this overflows.
fields->SetValue(offset + 0, gsl::narrow_cast<NativeT>(s->st_dev));
fields->SetValue(offset + 1, gsl::narrow<NativeT>(s->st_mode));
fields->SetValue(offset + 2, gsl::narrow<NativeT>(s->st_nlink));
fields->SetValue(offset + 3, gsl::narrow<NativeT>(s->st_uid));
fields->SetValue(offset + 4, gsl::narrow<NativeT>(s->st_gid));
fields->SetValue(offset + 5, gsl::narrow<NativeT>(s->st_rdev));
// Using the noop `narrow_cast` since this overflows.
fields->SetValue(offset + 5, gsl::narrow_cast<NativeT>(s->st_rdev));
#if defined(__POSIX__)
fields->SetValue(offset + 6, gsl::narrow<NativeT>(s->st_blksize));
fields->SetValue(offset + 7, gsl::narrow<NativeT>(s->st_ino));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@
const common = require('../common');
const assert = require('assert');

if (!common.isWindows)
if (!(common.isWindows || common.isSunOS || common.isAIX))
assert.fail('Code should fail only on Windows.');

const fs = require('fs');
const promiseFs = require('fs').promises;
const path = require('path');
const tmpdir = require('../common/tmpdir');
const { isDate } = require('util').types;

tmpdir.refresh();

Expand All @@ -23,8 +22,30 @@ function getFilename() {
}

function verifyStats(bigintStats, numStats) {
assert.ok(Number.isSafeInteger(numStats.ino));
assert.strictEqual(bigintStats.ino, BigInt(numStats.ino));
const keys = [
'mode', 'nlink', 'uid', 'gid', 'size',
// `rdev` can overflow on AIX and smartOS
'rdev',
// `dev` can overflow on AIX
'dev',
// `ino` can overflow on Windows
'ino',
];
const nStats = keys.reduce(
(s, k) => Object.assign(s, { [k]: String(numStats[k]) }),
{}
);
const bStats = keys.reduce(
(s, k) => Object.assign(s, { [k]: String(bigintStats[k]) }),
{}
);
assert.deepStrictEqual(nStats, bStats);
for (const key of keys) {
assert.ok(
Number.isSafeInteger(numStats[key]),
`numStats.${key}: ${numStats[key]} is not a safe integer`
);
}
}

{
Expand Down
117 changes: 117 additions & 0 deletions test/parallel/test-fs-stat-overflow.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
'use strict';

const common = require('../common');
const assert = require('assert');

const fs = require('fs');
const promiseFs = require('fs').promises;
const path = require('path');
const tmpdir = require('../common/tmpdir');

tmpdir.refresh();

let testIndex = 0;

function getFilename() {
const filename = path.join(tmpdir.path, `test-file-${++testIndex}`);
fs.writeFileSync(filename, 'test');
return filename;
}

function verifyStats(bigintStats, numStats) {
const keys = [
'mode', 'nlink', 'uid', 'gid', 'size',
];
if (!(common.isAIX || common.isSunOS)) {
// `rdev` can overflow.
keys.push('rdev');
}
if (!common.isAIX) {
keys.push('dev');
}
if (!common.isWindows) {
// These two are not defined on Windows.
keys.push('blocks', 'blksize');
// `ino` can overflow.
keys.push('ino');
}
for (const key of keys) {
const nVal = numStats[key];
const bVal = bigintStats[key];
assert.strictEqual(
// coerce to string to get a common type to test
String(bVal), String(nVal),
`bigintStats.${key}: ${bVal} is not equal to numStats.${key}: ${nVal}`
);
assert.ok(
Number.isSafeInteger(nVal),
`numStats.${key}: ${nVal} is not a safe integer`
);
}
}

{
const filename = getFilename();
const bigintStats = fs.statSync(filename, { bigint: true });
const numStats = fs.statSync(filename);
verifyStats(bigintStats, numStats);
}

{
const filename = __filename;
const bigintStats = fs.statSync(filename, { bigint: true });
const numStats = fs.statSync(filename);
verifyStats(bigintStats, numStats);
}

{
const filename = __dirname;
const bigintStats = fs.statSync(filename, { bigint: true });
const numStats = fs.statSync(filename);
verifyStats(bigintStats, numStats);
}

{
const filename = getFilename();
const fd = fs.openSync(filename, 'r');
const bigintStats = fs.fstatSync(fd, { bigint: true });
const numStats = fs.fstatSync(fd);
verifyStats(bigintStats, numStats);
fs.closeSync(fd);
}

{
const filename = getFilename();
fs.stat(filename, { bigint: true }, (err, bigintStats) => {
fs.stat(filename, (err, numStats) => {
verifyStats(bigintStats, numStats);
});
});
}

{
const filename = getFilename();
const fd = fs.openSync(filename, 'r');
fs.fstat(fd, { bigint: true }, (err, bigintStats) => {
fs.fstat(fd, (err, numStats) => {
verifyStats(bigintStats, numStats);
fs.closeSync(fd);
});
});
}

(async function() {
const filename = getFilename();
const bigintStats = await promiseFs.stat(filename, { bigint: true });
const numStats = await promiseFs.stat(filename);
verifyStats(bigintStats, numStats);
})();

(async function() {
const filename = getFilename();
const handle = await promiseFs.open(filename, 'r');
const bigintStats = await handle.stat({ bigint: true });
const numStats = await handle.stat();
verifyStats(bigintStats, numStats);
await handle.close();
})();

0 comments on commit c99e3d1

Please sign in to comment.