fs: align fs.ReadStream buffer pool writes to 8-byte boundary

Prevents alignment issues when creating a typed array from a buffer. Fixes: #24817 PR-URL: #24838 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
nodejs · May 4, 2019 · a0353fd · a0353fd
1 parent a278814
commit a0353fd
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 5 deletions.
diff --git 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
@@ -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;