Fix the encoding option and line splitting in transforms

Author: sindresorhus

docs/api.md

@@ -988,6 +988,8 @@ If it outputs binary data instead, this should be either:
 
 The output is available with [`result.stdout`](#resultstdout), [`result.stderr`](#resultstderr) and [`result.stdio`](#resultstdio).
 
+String input passed with the [`input`](#optionsinput) or [`stdin`](#optionsstdin) option is always encoded with UTF-8, whatever this option is. With a text encoding, [`stdin` transforms](transform.md) also decode their input with UTF-8.
+
 [More info.](binary.md)
 
 ### options.lines

docs/binary.md

@@ -61,6 +61,9 @@ const transform = function * (data) {
 await execa({stdout: {transform, binary: true}})`zip -r - input.txt`;
 ```
 
+> [!NOTE]
+> With the [`binary`](api.md#transformoptionsbinary) transform option, a `string` a transform yields is always converted to bytes using UTF-8, since the transform deals with raw bytes. Therefore, if the [`encoding`](api.md#optionsencoding) option is a different text encoding like `'utf16le'`, those bytes are then decoded with that encoding, which corrupts the text. Either `yield` an `Uint8Array` instead, or do not use the `binary` transform option.
+
 ## Streams
 
 [Streams produced](streams.md#converting-a-subprocess-to-a-stream) by [`subprocess.readable()`](api.md#subprocessreadablereadableoptions) and [`subprocess.duplex()`](api.md#subprocessduplexduplexoptions) are binary by default, which means they iterate over arbitrary [`Buffer`](https://nodejs.org/api/buffer.html#class-buffer) chunks. However, if the [`binary`](api.md#readableoptionsbinary) option is `false`, they iterate over line strings instead, and the stream is [in object mode](https://nodejs.org/api/stream.html#object-mode).

lib/arguments/encoding-option.js

@@ -19,6 +19,16 @@ const TEXT_ENCODINGS = new Set(['utf8', 'utf16le']);
 export const BINARY_ENCODINGS = new Set(['buffer', 'hex', 'base64', 'base64url', 'latin1', 'ascii']);
 const ENCODINGS = TEXT_ENCODINGS.union(BINARY_ENCODINGS);
 
+/*
+Encoding to use when converting a transform's string to bytes.
+Text encodings describe how text is represented as bytes, so those must be used, otherwise the string would be decoded with a different encoding than the one it was encoded with, which corrupts it.
+Binary encodings instead describe how bytes are serialized as text, so the string's UTF-8 bytes are used. The same applies with the `binary` transform option, since the transform then deals with raw bytes.
+*/
+export const getStringEncoding = (binary, encoding) => binary || !TEXT_ENCODINGS.has(encoding) ? 'utf8' : encoding;
+
+// Text encodings only apply to the subprocess' output. Its input is always text encoded with UTF-8, so input transforms decode and encode strings with UTF-8 too, whichever text encoding the `encoding` option uses.
+export const getInputEncoding = encoding => TEXT_ENCODINGS.has(encoding) ? 'utf8' : encoding;
+
 const getCorrectEncoding = encoding => {
 	if (encoding === null) {
 		return 'buffer';

lib/io/input-sync.js

@@ -1,6 +1,7 @@
 import {runGeneratorsSync} from '../transform/generator.js';
 import {joinToUint8Array, isUint8Array} from '../utils/uint-array.js';
 import {TYPE_TO_MESSAGE} from '../stdio/type.js';
+import {getInputEncoding} from '../arguments/encoding-option.js';
 
 // Apply `stdin`/`input`/`inputFile` options, before spawning, in sync mode, by converting it to the `input` option
 export const addInputOptionsSync = (fileDescriptors, options) => {
@@ -26,12 +27,13 @@ const addInputOptionSync = (fileDescriptors, fdNumber, options) => {
 	}
 
 	const allContents = allStdioItems.map(({contents}) => contents);
-	const transformedContents = allContents.map(contents => applySingleInputGeneratorsSync(contents, stdioItems));
+	const transformedContents = allContents.map(contents => applySingleInputGeneratorsSync(contents, stdioItems, options.encoding));
 	options.input = joinToUint8Array(transformedContents);
 };
 
-const applySingleInputGeneratorsSync = (contents, stdioItems) => {
-	const newContents = runGeneratorsSync(contents, stdioItems, 'utf8', true);
+const applySingleInputGeneratorsSync = (contents, stdioItems, encoding) => {
+	// Input transforms use UTF-8 with text encodings, since those only apply to the output
+	const newContents = runGeneratorsSync(contents, stdioItems, getInputEncoding(encoding), true);
 	validateSerializable(newContents);
 	return joinToUint8Array(newContents);
 };

lib/stdio/handle-async.js

@@ -2,6 +2,7 @@ import {createReadStream, createWriteStream} from 'node:fs';
 import {Buffer} from 'node:buffer';
 import {Readable, Writable, Duplex} from 'node:stream';
 import {generatorToStream} from '../transform/generator.js';
+import {getInputEncoding} from '../arguments/encoding-option.js';
 import {handleStdio} from './handle.js';
 import {TYPE_TO_MESSAGE} from './type.js';
 
@@ -28,9 +29,14 @@ const addProperties = {
 	native() {},
 };
 
+// Input transforms use UTF-8 with text encodings, since those only apply to the output
+const generatorToInputStream = (stdioItem, {encoding}) => generatorToStream(stdioItem, {encoding: getInputEncoding(encoding)});
+
 const addPropertiesAsync = {
 	input: {
 		...addProperties,
+		generator: generatorToInputStream,
+		asyncGenerator: generatorToInputStream,
 		fileUrl: ({value}) => ({stream: createReadStream(value)}),
 		filePath: ({value: {file}}) => ({stream: createReadStream(file)}),
 		webStream: ({value}) => ({stream: Readable.fromWeb(value)}),

lib/transform/encoding-transform.js

@@ -1,5 +1,6 @@
 import {Buffer} from 'node:buffer';
 import {StringDecoder} from 'node:string_decoder';
+import {getStringEncoding} from '../arguments/encoding-option.js';
 import {isUint8Array, bufferToUint8Array} from '../utils/uint-array.js';
 
 /*
@@ -49,3 +50,17 @@ const encodingStringFinal = function * (stringDecoder) {
 		yield lastChunk;
 	}
 };
+
+/*
+Once the user's transform has run, its string chunks are converted to bytes: by `Transform.push()` with the asynchronous methods, and by `joinToString()` with the synchronous ones. Both use UTF-8, yet the result is then decoded with the `encoding` option, which would corrupt the text when that option is a different text encoding.
+Therefore, those chunks are converted using the `encoding` option beforehand.
+With the `binary` transform option, they are converted with UTF-8 even when the `encoding` option is another text encoding. Otherwise, with synchronous methods, the next transform would receive the string itself, while with asynchronous methods it receives those UTF-8 bytes decoded with the `encoding` option.
+When the `encoding` option is UTF-8, strings round-trip unchanged, so there is nothing to do.
+*/
+export const getStringToBytesGenerator = (binary, encoding, skipped) => skipped || encoding === 'utf8'
+	? undefined
+	: {transform: stringToBytesGenerator.bind(undefined, getStringEncoding(binary, encoding))};
+
+const stringToBytesGenerator = function * (stringEncoding, chunk) {
+	yield typeof chunk === 'string' ? bufferToUint8Array(Buffer.from(chunk, stringEncoding)) : chunk;
+};

lib/transform/generator.js

@@ -1,8 +1,9 @@
 import {Transform, getDefaultHighWaterMark} from 'node:stream';
+import {getStringEncoding} from '../arguments/encoding-option.js';
 import {isAsyncGenerator} from '../stdio/type.js';
 import {getSplitLinesGenerator, getAppendNewlineGenerator} from './split.js';
 import {getValidateTransformInput, getValidateTransformReturn} from './validate.js';
-import {getEncodingTransformGenerator} from './encoding-transform.js';
+import {getEncodingTransformGenerator, getStringToBytesGenerator} from './encoding-transform.js';
 import {
 	pushChunks,
 	transformChunk,
@@ -37,7 +38,13 @@ Transform an array of generator functions into a `Transform` stream.
 */
 export const generatorToStream = ({
 	value,
-	value: {transform, final, writableObjectMode, readableObjectMode},
+	value: {
+		transform,
+		final,
+		binary,
+		writableObjectMode,
+		readableObjectMode,
+	},
 	optionName,
 }, {encoding}) => {
 	const state = {};
@@ -60,6 +67,8 @@ export const generatorToStream = ({
 		writableHighWaterMark: getDefaultHighWaterMark(writableObjectMode),
 		readableObjectMode,
 		readableHighWaterMark: getDefaultHighWaterMark(readableObjectMode),
+		// A string written to the stream is converted to bytes with this encoding, which `getEncodingTransformGenerator()` then decodes with the `encoding` option
+		defaultEncoding: getStringEncoding(binary, encoding),
 		transform(chunk, encoding, done) {
 			transformMethod([chunk, generators, 0], this, done);
 		},
@@ -102,6 +111,8 @@ const addInternalGenerators = (
 			preserveNewlines,
 			readableObjectMode,
 			state,
+			encoding,
 		}),
+		getStringToBytesGenerator(binary, encoding, readableObjectMode),
 	].filter(Boolean);
 };

lib/transform/split.js

@@ -1,3 +1,5 @@
+import {getStringEncoding} from '../arguments/encoding-option.js';
+
 // Split chunks line-wise for generators passed to the `std*` options
 export const getSplitLinesGenerator = (binary, preserveNewlines, skipped, state) => binary || skipped
 	? undefined
@@ -36,15 +38,16 @@ const splitGenerator = function * (state, preserveNewlines, chunk) {
 			continue;
 		}
 
-		const newlineLength = getNewlineLength(chunk, end, preserveNewlines, state);
-		let line = chunk.slice(start + 1, end + 1 - newlineLength);
+		// The `\r` of a `\r\n` sequence is stripped further below, once `previousChunks` has been prepended.
+		// Otherwise, a `\r\n` sequence split across two chunks would not be detected.
+		let line = chunk.slice(start + 1, preserveNewlines ? end + 1 : end);
 
 		if (previousChunks.length > 0) {
 			line = concatString(previousChunks, line);
 			previousChunks = '';
 		}
 
-		yield line;
+		yield preserveNewlines ? line : stripCarriageReturn(line, state);
 		start = end;
 	}
 
@@ -55,13 +58,9 @@ const splitGenerator = function * (state, preserveNewlines, chunk) {
 	state.previousChunks = previousChunks;
 };
 
-const getNewlineLength = (chunk, end, preserveNewlines, state) => {
-	if (preserveNewlines) {
-		return 0;
-	}
-
-	state.isWindowsNewline = end !== 0 && chunk[end - 1] === '\r';
-	return state.isWindowsNewline ? 2 : 1;
+const stripCarriageReturn = (line, state) => {
+	state.isWindowsNewline = line.endsWith('\r');
+	return state.isWindowsNewline ? line.slice(0, -1) : line;
 };
 
 const linesFinal = function * ({previousChunks}) {
@@ -72,29 +71,36 @@ const linesFinal = function * ({previousChunks}) {
 
 // Unless `preserveNewlines: true` is used, we strip the newline of each line.
 // This re-adds them after the user `transform` code has run.
-export const getAppendNewlineGenerator = ({binary, preserveNewlines, readableObjectMode, state}) => binary || preserveNewlines || readableObjectMode
+// The re-added newline must be encoded with the `encoding` option when the chunk is bytes: a UTF-8 newline would misalign `utf16le` bytes.
+export const getAppendNewlineGenerator = ({binary, preserveNewlines, readableObjectMode, state, encoding}) => binary || preserveNewlines || readableObjectMode
 	? undefined
-	: {transform: appendNewlineGenerator.bind(undefined, state)};
+	: {transform: appendNewlineGenerator.bind(undefined, state, getStringEncoding(binary, encoding))};
 
-const appendNewlineGenerator = function * ({isWindowsNewline = false}, chunk) {
-	const {unixNewline, windowsNewline, LF, concatBytes} = typeof chunk === 'string' ? linesStringInfo : linesUint8ArrayInfo;
+const appendNewlineGenerator = function * ({isWindowsNewline = false}, stringEncoding, chunk) {
+	const {unixNewline, windowsNewline, concat} = typeof chunk === 'string'
+		? linesStringInfo
+		: LINES_BYTES_INFO[stringEncoding];
 
-	if (chunk.at(-1) === LF) {
+	if (endsWithNewline(chunk, unixNewline)) {
 		yield chunk;
 		return;
 	}
 
 	const newline = isWindowsNewline ? windowsNewline : unixNewline;
-	yield concatBytes(chunk, newline);
+	yield concat(chunk, newline);
 };
 
+const endsWithNewline = (chunk, unixNewline) => typeof chunk === 'string'
+	? chunk.endsWith(unixNewline)
+	: chunk.length >= unixNewline.length
+		&& chunk.subarray(-unixNewline.length).every((byte, index) => byte === unixNewline[index]);
+
 const concatString = (firstChunk, secondChunk) => `${firstChunk}${secondChunk}`;
 
 const linesStringInfo = {
 	windowsNewline: '\r\n',
 	unixNewline: '\n',
-	LF: '\n',
-	concatBytes: concatString,
+	concat: concatString,
 };
 
 const concatUint8Array = (firstChunk, secondChunk) => {
@@ -104,9 +110,17 @@ const concatUint8Array = (firstChunk, secondChunk) => {
 	return chunk;
 };
 
-const linesUint8ArrayInfo = {
-	windowsNewline: new Uint8Array([0x0D, 0x0A]),
-	unixNewline: new Uint8Array([0x0A]),
-	LF: 0x0A,
-	concatBytes: concatUint8Array,
+// Only indexed with text encodings, since the `binary` option and binary encodings disable this generator
+const LINES_BYTES_INFO = {
+	__proto__: null,
+	utf8: {
+		windowsNewline: new Uint8Array([0x0D, 0x0A]),
+		unixNewline: new Uint8Array([0x0A]),
+		concat: concatUint8Array,
+	},
+	utf16le: {
+		windowsNewline: new Uint8Array([0x0D, 0x00, 0x0A, 0x00]),
+		unixNewline: new Uint8Array([0x0A, 0x00]),
+		concat: concatUint8Array,
+	},
 };

test/helpers/lines.js

@@ -20,9 +20,7 @@ export const simpleFullUtf16Inverted = simpleFullBuffer.toString('utf16le');
 const simpleFullUtf16Buffer = Buffer.from(simpleFull, 'utf16le');
 export const simpleFullUtf16Uint8Array = new Uint8Array(simpleFullUtf16Buffer);
 export const simpleFullEnd = `${simpleFull}\n`;
-const simpleFullEndBuffer = Buffer.from(simpleFullEnd);
 export const simpleFullEndChunks = [simpleFullEnd];
-export const simpleFullEndUtf16Inverted = simpleFullEndBuffer.toString('utf16le');
 export const simpleLines = ['aaa\n', 'bbb\n', 'ccc'];
 export const simpleFullEndLines = ['aaa\n', 'bbb\n', 'ccc\n'];
 export const noNewlinesFull = 'aaabbbccc';

test/io/input-option.js

@@ -1,3 +1,4 @@
+import {Buffer} from 'node:buffer';
 import {Writable} from 'node:stream';
 import test from 'ava';
 import {execa, execaSync} from '../../index.js';
@@ -16,6 +17,7 @@ import {
 	foobarUint16Array,
 	foobarDataView,
 } from '../helpers/input.js';
+import {noopGenerator} from '../helpers/generator.js';
 
 setFixtureDirectory();
 
@@ -67,3 +69,58 @@ test('input option cannot be 0 - sync', testInvalidInput, 0, execaSync);
 test('input option cannot be false - sync', testInvalidInput, false, execaSync);
 test('input option cannot be null - sync', testInvalidInput, null, execaSync);
 test('input option cannot be a non-Readable stream - sync', testInvalidInput, new Writable(), execaSync);
+
+/*
+String input is encoded with the `encoding` option, so it round-trips with subprocesses echoing it back.
+This must not depend on whether a `stdin` transform happens to be present.
+*/
+const nonAsciiString = '🦄🦄';
+
+const testStringEncoding = async (t, options, execaMethod) => {
+	const {stdout} = await execaMethod('stdin.js', {...options, input: nonAsciiString, encoding: 'utf16le'});
+	t.is(stdout, nonAsciiString);
+};
+
+test('input option string is encoded with encoding "utf16le" without transforms', testStringEncoding, {}, execa);
+test('input option string is encoded with encoding "utf16le" without transforms - sync', testStringEncoding, {}, execaSync);
+test('input option string is encoded with encoding "utf16le" with transforms', testStringEncoding, {stdin: noopGenerator()}, execa);
+test('input option string is encoded with encoding "utf16le" with transforms - sync', testStringEncoding, {stdin: noopGenerator()}, execaSync);
+
+/*
+Sources other than the `input` option also pass strings to the subprocess, which must be encoded with the `encoding` option too.
+*/
+const testIterableInputEncoding = async (t, execaMethod) => {
+	const {stdout} = await execaMethod('stdin.js', {
+		stdin: [[`${nonAsciiString}aaa\n`], [`${nonAsciiString}bbb\n`]],
+		encoding: 'utf16le',
+		stripFinalNewline: false,
+	});
+	t.is(stdout, `${nonAsciiString}aaa\n${nonAsciiString}bbb\n`);
+};
+
+test('Iterable string input is encoded with encoding "utf16le"', testIterableInputEncoding, execa);
+test('Iterable string input is encoded with encoding "utf16le" - sync', testIterableInputEncoding, execaSync);
+
+const testAsyncIterableInputEncoding = async t => {
+	const chunks = async function * () {
+		yield nonAsciiString;
+	};
+
+	const {stdout} = await execa('stdin.js', {stdin: [chunks()], encoding: 'utf16le'});
+	t.is(stdout, nonAsciiString);
+};
+
+test('Async iterable string input is encoded with encoding "utf16le"', testAsyncIterableInputEncoding);
+
+/*
+The `encoding` option does not change how string input is encoded when it is a binary encoding: those describe how output bytes are serialized, so input keeps using UTF-8.
+*/
+const testBinaryEncodingInput = async (t, encoding, execaMethod) => {
+	const {stdout} = await execaMethod('stdin.js', {input: nonAsciiString, encoding});
+	t.is(stdout, Buffer.from(nonAsciiString).toString(encoding));
+};
+
+test('String input stays UTF-8 with encoding "latin1"', testBinaryEncodingInput, 'latin1', execa);
+test('String input stays UTF-8 with encoding "latin1" - sync', testBinaryEncodingInput, 'latin1', execaSync);
+test('String input stays UTF-8 with encoding "hex"', testBinaryEncodingInput, 'hex', execa);
+test('String input stays UTF-8 with encoding "hex" - sync', testBinaryEncodingInput, 'hex', execaSync);