From 29e1cca941571e2e6ba6ed36ffe176d90d50cfdc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Jul 2026 02:52:25 +0000 Subject: [PATCH 1/3] fix: normalize every CRLF when parsing a RELEASES manifest String.replace with a string pattern only replaces the first occurrence, so all but the first line of a CRLF manifest kept their trailing \r. The line regex tolerated the stray \r (making this harmless in practice), but the normalization now does what it says via replaceAll. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01AtqCsX7ptP36t5rdyaJJhM --- src/utils/win-releases.ts | 3 ++- test/unit/win-releases.spec.ts | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/utils/win-releases.ts b/src/utils/win-releases.ts index e63d520..9d8cfd3 100644 --- a/src/utils/win-releases.ts +++ b/src/utils/win-releases.ts @@ -72,7 +72,8 @@ export async function parseRELEASES( content: string, ): Promise { const stripped = stripBom(content); - const normalizedEOL = stripped.replace("\r\n", "\n"); + // replaceAll: replace with a string pattern only replaces the first CRLF + const normalizedEOL = stripped.replaceAll("\r\n", "\n"); const lines = normalizedEOL.split("\n"); const goodlines = lines.filter((line) => !!releaseRe.exec(line)); diff --git a/test/unit/win-releases.spec.ts b/test/unit/win-releases.spec.ts index 44fc13a..2b157d5 100644 --- a/test/unit/win-releases.spec.ts +++ b/test/unit/win-releases.spec.ts @@ -44,6 +44,18 @@ describe("Windows RELEASES", function () { expect(releases.length).toBe(5); }); + it("should parse a multi-line file with CRLF line endings", async function () { + const crlfReleases = await parseRELEASES( + "62E8BF432F29E8E08240910B85EDBF2D1A41EDF2 atom-0.178.0-full.nupkg 81272434\r\n" + + "5D754139E89802E88984185D2276B54DB730CD5E atom-0.178.1-delta.nupkg 8938535\r\n" + + "DD48D16EE177DD278F0A82CDDB72EBD043C767D2 atom-0.178.1-full.nupkg 81293415", + ); + expect(crlfReleases.length).toBe(3); + // every CRLF is normalized, not just the first one + expect(crlfReleases[2].filename).toBe("atom-0.178.1-full.nupkg"); + expect(crlfReleases[2].size).toBe(81293415); + }); + it("should parse a one-line file (with utf-8 BOM)", async function () { const oneRelease = await parseRELEASES( "\uFEFF24182FAD211FB9EB72610B1C086810FE37F70AE3 gitbook-editor-4.0.0-full.nupkg 46687158", From 48e4490d6dd89dfaf854e4af4ca186abf6b1ed24 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Jul 2026 03:07:58 +0000 Subject: [PATCH 2/3] fix: verify full CRLF normalization and correct the comment Copilot review follow-up: the line regex tolerates a trailing CR, so the CRLF test now captures the lines handed to the regex and asserts none still carries one (it fails against first-only String#replace), and the comment no longer reads as if replaceAll were the single-replacement variant. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01AtqCsX7ptP36t5rdyaJJhM --- src/utils/win-releases.ts | 3 ++- test/unit/win-releases.spec.ts | 21 +++++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/utils/win-releases.ts b/src/utils/win-releases.ts index 9d8cfd3..22b160b 100644 --- a/src/utils/win-releases.ts +++ b/src/utils/win-releases.ts @@ -72,7 +72,8 @@ export async function parseRELEASES( content: string, ): Promise { const stripped = stripBom(content); - // replaceAll: replace with a string pattern only replaces the first CRLF + // String#replace with a string pattern would only replace the first + // CRLF; replaceAll normalizes every line ending const normalizedEOL = stripped.replaceAll("\r\n", "\n"); const lines = normalizedEOL.split("\n"); const goodlines = lines.filter((line) => !!releaseRe.exec(line)); diff --git a/test/unit/win-releases.spec.ts b/test/unit/win-releases.spec.ts index 2b157d5..bf99992 100644 --- a/test/unit/win-releases.spec.ts +++ b/test/unit/win-releases.spec.ts @@ -44,16 +44,33 @@ describe("Windows RELEASES", function () { expect(releases.length).toBe(5); }); - it("should parse a multi-line file with CRLF line endings", async function () { + it("should normalize every CRLF, not just the first one", async function () { + // the line regex tolerates a trailing \r ([\r]*$), so parsed output + // alone can't distinguish full normalization from first-only + // String#replace; capture the lines actually handed to the regex and + // assert none still carries a \r + const seenLines: string[] = []; + const originalExec = RegExp.prototype.exec; + const spy = vi + .spyOn(RegExp.prototype, "exec") + .mockImplementation(function (this: RegExp, str: string) { + seenLines.push(str); + return originalExec.call(this, str); + }); const crlfReleases = await parseRELEASES( "62E8BF432F29E8E08240910B85EDBF2D1A41EDF2 atom-0.178.0-full.nupkg 81272434\r\n" + "5D754139E89802E88984185D2276B54DB730CD5E atom-0.178.1-delta.nupkg 8938535\r\n" + "DD48D16EE177DD278F0A82CDDB72EBD043C767D2 atom-0.178.1-full.nupkg 81293415", ); + spy.mockRestore(); + expect(crlfReleases.length).toBe(3); - // every CRLF is normalized, not just the first one expect(crlfReleases[2].filename).toBe("atom-0.178.1-full.nupkg"); expect(crlfReleases[2].size).toBe(81293415); + // with first-only replacement the second and third lines would still + // end in \r when they reach the regex + expect(seenLines.length).toBeGreaterThan(0); + expect(seenLines.some((line) => line.includes("\r"))).toBe(false); }); it("should parse a one-line file (with utf-8 BOM)", async function () { From c259927a22e0e7301381bdf2b4e8a48e4733d7c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Jul 2026 03:14:52 +0000 Subject: [PATCH 3/3] fix: restore the exec spy in finally and correct test comments Copilot review follow-up: the RegExp.prototype.exec spy now restores in a finally block so a throwing parseRELEASES can't leak it into later tests; the comments now say only the second (CRLF-terminated) line retains a CR under first-only replacement and that replaceAll targets CRLF occurrences, not every line-ending flavor. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01AtqCsX7ptP36t5rdyaJJhM --- src/utils/win-releases.ts | 2 +- test/unit/win-releases.spec.ts | 23 +++++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/utils/win-releases.ts b/src/utils/win-releases.ts index 22b160b..ad2efd3 100644 --- a/src/utils/win-releases.ts +++ b/src/utils/win-releases.ts @@ -73,7 +73,7 @@ export async function parseRELEASES( ): Promise { const stripped = stripBom(content); // String#replace with a string pattern would only replace the first - // CRLF; replaceAll normalizes every line ending + // CRLF; replaceAll normalizes every CRLF occurrence const normalizedEOL = stripped.replaceAll("\r\n", "\n"); const lines = normalizedEOL.split("\n"); const goodlines = lines.filter((line) => !!releaseRe.exec(line)); diff --git a/test/unit/win-releases.spec.ts b/test/unit/win-releases.spec.ts index bf99992..a2d131a 100644 --- a/test/unit/win-releases.spec.ts +++ b/test/unit/win-releases.spec.ts @@ -57,18 +57,25 @@ describe("Windows RELEASES", function () { seenLines.push(str); return originalExec.call(this, str); }); - const crlfReleases = await parseRELEASES( - "62E8BF432F29E8E08240910B85EDBF2D1A41EDF2 atom-0.178.0-full.nupkg 81272434\r\n" + - "5D754139E89802E88984185D2276B54DB730CD5E atom-0.178.1-delta.nupkg 8938535\r\n" + - "DD48D16EE177DD278F0A82CDDB72EBD043C767D2 atom-0.178.1-full.nupkg 81293415", - ); - spy.mockRestore(); + let crlfReleases; + try { + crlfReleases = await parseRELEASES( + "62E8BF432F29E8E08240910B85EDBF2D1A41EDF2 atom-0.178.0-full.nupkg 81272434\r\n" + + "5D754139E89802E88984185D2276B54DB730CD5E atom-0.178.1-delta.nupkg 8938535\r\n" + + "DD48D16EE177DD278F0A82CDDB72EBD043C767D2 atom-0.178.1-full.nupkg 81293415", + ); + } finally { + // restore even if parseRELEASES throws, or the global spy would + // leak into subsequent tests + spy.mockRestore(); + } expect(crlfReleases.length).toBe(3); expect(crlfReleases[2].filename).toBe("atom-0.178.1-full.nupkg"); expect(crlfReleases[2].size).toBe(81293415); - // with first-only replacement the second and third lines would still - // end in \r when they reach the regex + // with first-only replacement the second line (any CRLF-terminated + // line after the first) would still end in \r when it reaches the + // regex; the last line has no CRLF terminator expect(seenLines.length).toBeGreaterThan(0); expect(seenLines.some((line) => line.includes("\r"))).toBe(false); });