From 7a4c44564a828e643fb954a8245c7bf9d0bf64a5 Mon Sep 17 00:00:00 2001
From: UnZan Chuang <55685430+unzan401@users.noreply.github.com>
Date: Mon, 8 Jan 2024 01:58:07 +0800
Subject: [PATCH] Convert writingBuf to Buffer to compute the original string
 length (#188)

* Convert writingBuf to Buffer to compute the original string length

* Add jsdoc

* Compare by Buffer.byteLength instead of String.length
---
 index.js           | 47 +++++++++++++++++++++++++++-------------------
 test/write.test.js | 40 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 67 insertions(+), 20 deletions(-)

diff --git a/index.js b/index.js
index 7b20ab7..5c45b4d 100644
--- a/index.js
+++ b/index.js
@@ -179,21 +179,9 @@ function SonicBoom (opts) {
     }
 
     this.emit('write', n)
-
-    this._len -= n
-    // In case of multi-byte characters, the length of the written buffer
-    // may be different from the length of the string. Let's make sure
-    // we do not have an accumulated string with a negative length.
-    // This also mean that ._len is not precise, but it's not a problem as some
-    // writes might be triggered earlier than ._minLength.
-    if (this._len < 0) {
-      this._len = 0
-    }
-
-    // TODO if we have a multi-byte character in the buffer, we need to
-    // n might not be the same as this._writingBuf.length, so we might loose
-    // characters here. The solution to this problem is to use a Buffer for _writingBuf.
-    this._writingBuf = this._writingBuf.slice(n)
+    const releasedBufObj = releaseWritingBuf(this._writingBuf, this._len, n)
+    this._len = releasedBufObj.len
+    this._writingBuf = releasedBufObj.writingBuf
 
     if (this._writingBuf.length) {
       if (!this.sync) {
@@ -204,8 +192,9 @@ function SonicBoom (opts) {
       try {
         do {
           const n = fsWriteSync()
-          this._len -= n
-          this._writingBuf = this._writingBuf.slice(n)
+          const releasedBufObj = releaseWritingBuf(this._writingBuf, this._len, n)
+          this._len = releasedBufObj.len
+          this._writingBuf = releasedBufObj.writingBuf
         } while (this._writingBuf.length)
       } catch (err) {
         this.release(err)
@@ -251,6 +240,25 @@ function SonicBoom (opts) {
   })
 }
 
+/**
+ * Release the writingBuf after fs.write n bytes data
+ * @param {string | Buffer} writingBuf - currently writing buffer, usually be instance._writingBuf.
+ * @param {number} len - currently buffer length, usually be instance._len.
+ * @param {number} n - number of bytes fs already written
+ * @returns {{writingBuf: string | Buffer, len: number}} released writingBuf and length
+ */
+function releaseWritingBuf (writingBuf, len, n) {
+  // if Buffer.byteLength is equal to n, that means writingBuf contains no multi-byte character
+  if (typeof writingBuf === 'string' && Buffer.byteLength(writingBuf) !== n) {
+    // Since the fs.write callback parameter `n` means how many bytes the passed of string
+    // We calculate the original string length for avoiding the multi-byte character issue
+    n = Buffer.from(writingBuf).subarray(0, n).toString().length
+  }
+  len = Math.max(len - n, 0)
+  writingBuf = writingBuf.slice(n)
+  return { writingBuf, len }
+}
+
 function emitDrain (sonic) {
   const hasListeners = sonic.listenerCount('drain') > 0
   if (!hasListeners) return
@@ -523,8 +531,9 @@ function flushSync () {
     }
     try {
       const n = fs.writeSync(this.fd, buf, 'utf8')
-      buf = buf.slice(n)
-      this._len = Math.max(this._len - n, 0)
+      const releasedBufObj = releaseWritingBuf(buf, this._len, n)
+      buf = releasedBufObj.writingBuf
+      this._len = releasedBufObj.len
       if (buf.length <= 0) {
         this._bufs.shift()
       }
diff --git a/test/write.test.js b/test/write.test.js
index 1c0913b..b619507 100644
--- a/test/write.test.js
+++ b/test/write.test.js
@@ -78,7 +78,7 @@ function buildTests (test, sync) {
     const dest = file()
     const fd = fs.openSync(dest, 'w')
     const stream = new SonicBoom({ fd, sync })
-    const source = fs.createReadStream(__filename)
+    const source = fs.createReadStream(__filename, { encoding: 'utf8' })
 
     source.pipe(stream)
 
@@ -247,6 +247,44 @@ function buildTests (test, sync) {
       t.pass('close emitted')
     })
   })
+
+  test('write multi-byte characters string over than maxWrite', (t) => {
+    const fakeFs = Object.create(fs)
+    const MAX_WRITE = 65535
+    fakeFs.write = function (fd, buf, ...args) {
+      // only write byteLength === MAX_WRITE
+      const _buf = Buffer.from(buf).subarray(0, MAX_WRITE).toString()
+      fs.write(fd, _buf, ...args)
+      setImmediate(args[args.length - 1], null, MAX_WRITE)
+      fakeFs.write = function (fd, buf, ...args) {
+        fs.write(fd, buf, ...args)
+      }
+    }
+    const SonicBoom = proxyquire('../', {
+      fs: fakeFs
+    })
+    const dest = file()
+    const fd = fs.openSync(dest, 'w')
+    const stream = new SonicBoom({ fd, minLength: 0, sync, maxWrite: MAX_WRITE })
+    let buf = Buffer.alloc(MAX_WRITE).fill('x')
+    buf = '🌲' + buf.toString()
+    stream.write(buf)
+    stream.end()
+
+    stream.on('finish', () => {
+      fs.readFile(dest, 'utf8', (err, data) => {
+        t.error(err)
+        t.equal(data, buf)
+        t.end()
+      })
+    })
+    stream.on('close', () => {
+      t.pass('close emitted')
+    })
+    stream.on('error', () => {
+      t.pass('error emitted')
+    })
+  })
 }
 
 test('write buffers that are not totally written', (t) => {