From a0353fdbe2ce725e06dab0acbd0326d1c163667c Mon Sep 17 00:00:00 2001 From: ptaylor Date: Sun, 31 Mar 2019 17:26:06 -0700 Subject: [PATCH] fs: align fs.ReadStream buffer pool writes to 8-byte boundary Prevents alignment issues when creating a typed array from a buffer. Fixes: https://github.com/nodejs/node/issues/24817 PR-URL: https://github.com/nodejs/node/pull/24838 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- lib/internal/fs/streams.js | 23 ++++++++++++++++++----- test/parallel/test-fs-read-stream.js | 1 + 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js index 97a8856797ac52..dfff08dbbd1d2a 100644 --- a/lib/internal/fs/streams.js +++ b/lib/internal/fs/streams.js @@ -51,6 +51,10 @@ function checkPosition(pos, name) { } } +function roundUpToMultipleOf8(n) { + return (n + 7) & ~7; // Align to 8 byte boundary. +} + function ReadStream(path, options) { if (!(this instanceof ReadStream)) return new ReadStream(path, options); @@ -172,10 +176,18 @@ ReadStream.prototype._read = function(n) { // Now that we know how much data we have actually read, re-wind the // 'used' field if we can, and otherwise allow the remainder of our // reservation to be used as a new pool later. - if (start + toRead === thisPool.used && thisPool === pool) - thisPool.used += bytesRead - toRead; - else if (toRead - bytesRead > kMinPoolSpace) - poolFragments.push(thisPool.slice(start + bytesRead, start + toRead)); + if (start + toRead === thisPool.used && thisPool === pool) { + const newUsed = thisPool.used + bytesRead - toRead; + thisPool.used = roundUpToMultipleOf8(newUsed); + } else { + // Round down to the next lowest multiple of 8 to ensure the new pool + // fragment start and end positions are aligned to an 8 byte boundary. + const alignedEnd = (start + toRead) & ~7; + const alignedStart = roundUpToMultipleOf8(start + bytesRead); + if (alignedEnd - alignedStart >= kMinPoolSpace) { + poolFragments.push(thisPool.slice(alignedStart, alignedEnd)); + } + } if (bytesRead > 0) { this.bytesRead += bytesRead; @@ -189,7 +201,8 @@ ReadStream.prototype._read = function(n) { // Move the pool positions, and internal position for reading. if (this.pos !== undefined) this.pos += toRead; - pool.used += toRead; + + pool.used = roundUpToMultipleOf8(pool.used + toRead); }; ReadStream.prototype._destroy = function(err, cb) { diff --git a/test/parallel/test-fs-read-stream.js b/test/parallel/test-fs-read-stream.js index b1559ed9ea8c04..e33c6dec4ee264 100644 --- a/test/parallel/test-fs-read-stream.js +++ b/test/parallel/test-fs-read-stream.js @@ -55,6 +55,7 @@ const rangeFile = fixtures.path('x.txt'); file.on('data', function(data) { assert.ok(data instanceof Buffer); + assert.ok(data.byteOffset % 8 === 0); assert.ok(!paused); file.length += data.length;