From 752b7887779a89ca964ea335fe5f7eb568d43a43 Mon Sep 17 00:00:00 2001 From: Zach Leventer Date: Sun, 26 Apr 2026 17:46:58 -0400 Subject: [PATCH] fix: buffer ReplacementStream chunks before applying replacements ReplacementStream ran the replacement regex against each chunk independently, so any token that was split across a chunk boundary (e.g. `#SOME` + `_REPLACEMENT#`) would fail to match and survive into the converted output unmodified. Buffer all chunks and run the replacement once in `_flush`. Metadata files are bounded in size so the memory cost is negligible compared to the correctness win, and the warning bookkeeping for `singleFile` replacements collapses naturally to a single pass. Adds a regression test that splits a token at its midpoint across two chunks via `Readable.from` and asserts the token is replaced and no "not found" warning is emitted. Refs forcedotcom/cli#3461 --- src/convert/replacements.ts | 49 ++++++++++++++++++------------- test/convert/replacements.test.ts | 36 +++++++++++++++++++++++ 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/src/convert/replacements.ts b/src/convert/replacements.ts index f6234695d7..c6cc6ad334 100644 --- a/src/convert/replacements.ts +++ b/src/convert/replacements.ts @@ -55,37 +55,46 @@ export class ReplacementStream extends Transform { private readonly foundReplacements = new Set(); private readonly allReplacements: MarkedReplacement[]; private readonly lifecycleInstance = Lifecycle.getInstance(); + private readonly chunks: Buffer[] = []; public constructor(private readonly replacements: MarkedReplacement[]) { super({ objectMode: true }); this.allReplacements = replacements; } - public async _transform( - chunk: Buffer, - encoding: string, - callback: (error?: Error, data?: Buffer) => void - ): Promise { - let error: Error | undefined; - const { output, found } = await replacementIterations(chunk.toString(), this.replacements); - for (const foundKey of found) { - this.foundReplacements.add(foundKey); - } - callback(error, Buffer.from(output)); + public _transform(chunk: Buffer | string, encoding: string, callback: (error?: Error, data?: Buffer) => void): void { + // Buffer the whole file before running replacements. Running per-chunk + // missed any token that straddled a chunk boundary, leaving the original + // token in the output (forcedotcom/cli#3461). Metadata files are bounded + // in size, so the memory cost is negligible compared to the correctness + // win. + this.chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + callback(); } public async _flush(callback: (error?: Error) => void): Promise { - // At the end of the stream, emit warnings for replacements not found - for (const replacement of this.allReplacements) { - const key = replacement.toReplace.toString(); - if (replacement.singleFile && !this.foundReplacements.has(key)) { - // eslint-disable-next-line no-await-in-loop - await this.lifecycleInstance.emitWarning( - `Your sfdx-project.json specifies that ${key} should be replaced in ${replacement.matchedFilename}, but it was not found.` - ); + try { + const input = Buffer.concat(this.chunks).toString(); + const { output, found } = await replacementIterations(input, this.replacements); + for (const foundKey of found) { + this.foundReplacements.add(foundKey); } + this.push(Buffer.from(output)); + + // At the end of the stream, emit warnings for replacements not found + for (const replacement of this.allReplacements) { + const key = replacement.toReplace.toString(); + if (replacement.singleFile && !this.foundReplacements.has(key)) { + // eslint-disable-next-line no-await-in-loop + await this.lifecycleInstance.emitWarning( + `Your sfdx-project.json specifies that ${key} should be replaced in ${replacement.matchedFilename}, but it was not found.` + ); + } + } + callback(); + } catch (e) { + callback(e instanceof Error ? e : new Error(String(e))); } - callback(); } } diff --git a/test/convert/replacements.test.ts b/test/convert/replacements.test.ts index 386fcf4034..136a3db2f7 100644 --- a/test/convert/replacements.test.ts +++ b/test/convert/replacements.test.ts @@ -488,4 +488,40 @@ describe('executes replacements on a string', () => { expect(warnSpy.callCount).to.equal(0); warnSpy.restore(); }); + + it('replaces a token that straddles a chunk boundary', async () => { + // Regression test for forcedotcom/cli#3461. The previous implementation + // ran each chunk through the regex independently, so a token that was + // split across chunks (e.g. `#SOME` + `_REPLACEMENT#`) would not be + // matched and would survive into the converted file. + const token = '#SOME_REPLACEMENT#'; + const prefix = '\n'; + const suffix = '\n'; + const fullText = prefix + token + suffix; + + // Split the input so the token is sliced in half across two chunks. + const splitAt = prefix.length + Math.floor(token.length / 2); + const chunk1 = fullText.slice(0, splitAt); + const chunk2 = fullText.slice(splitAt); + + const stream = new ReplacementStream([ + { + toReplace: stringToRegex(token), + replaceWith: 'REPLACED', + singleFile: true, + matchedFilename: 'straddle.xml', + }, + ]); + const warnSpy = Sinon.spy(Lifecycle.getInstance(), 'emitWarning'); + let result = ''; + stream.on('data', (chunk) => { + result += chunk.toString(); + }); + + await pipeline(Readable.from([chunk1, chunk2]), stream); + + expect(result).to.equal(prefix + 'REPLACED' + suffix); + expect(warnSpy.callCount).to.equal(0); + warnSpy.restore(); + }); });