Skip to content

fix(core): preserve binary output in ProcessOutput.buffer() - #1495

Open
spokodev wants to merge 1 commit into
google:mainfrom
spokodev:fix/process-output-buffer-binary
Open

fix(core): preserve binary output in ProcessOutput.buffer()#1495
spokodev wants to merge 1 commit into
google:mainfrom
spokodev:fix/process-output-buffer-binary

Conversation

@spokodev

Copy link
Copy Markdown

Problem

ProcessOutput.buffer() builds its Buffer from this.stdall:

buffer(): Buffer {
  return Buffer.from(this.stdall)
}

But stdall is a getter that UTF-8 decodes the stored chunks
(bufArrJoinTextDecoder('utf-8').decode). Any byte that isn't valid
UTF-8 has already been replaced with U+FFFD by the time buffer() runs, so
the bytes it returns are corrupted — even though the original, lossless chunks
are still sitting in this._dto.store.stdall.

blob() and text(<encoding>) both delegate to buffer(), so they are
affected too. These are the documented APIs for getting bytes out of a command:

const png = await $`cat logo.png`.buffer()   // mangled: every non-utf8 byte → ef bf bd
await $`cat logo.png`.text('hex')            // same corruption

Reproduction

const o = await $`node -e ${'process.stdout.write(Buffer.from([255,254,129,130]))'}`
o.buffer().toString('hex')   // 'efbfbdefbfbdefbfbdefbfbd'  ← expected 'fffe8182'

Fix

Reconstruct the Buffer from the raw store chunks (which are the original
string | Buffer pieces) instead of the decoded string:

buffer(): Buffer {
  return Buffer.concat(
    [...this._dto.store.stdall].map((chunk) =>
      isString(chunk) ? Buffer.from(chunk) : chunk
    )
  )
}

This also fixes blob() and text('hex' | 'base64' | 'latin1' | …), which go
through buffer(). The plain-text path (toString() / text()) is unchanged.

Test

Added a buffer() case in test/core.test.js asserting that non-UTF-8 bytes
survive buffer()/text('hex'). Fails on main, passes with the fix; the
existing buffer()/text()/blob() string tests still pass.

buffer() built its Buffer from `this.stdall`, which is a UTF-8 decoded
string, so any non-UTF-8 byte in the command output was already replaced
with U+FFFD and the returned bytes were corrupted — even though the raw
chunks are still retained in the store. blob() and text(<encoding>) both
delegate to buffer(), so they were affected too:

  await $`cat image.png`.buffer()  // mangled bytes

Rebuild the Buffer from the lossless store chunks instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant