fix(cli): strip trailing sentence punctuation from URL matches in normalizeUrls

\S+ was greedily consuming trailing punctuation (. ! , ;) into the URL
match, causing new URL() to mutate clean ASCII URLs — e.g.
"see https://example.com." became "see https://example.com./".

Fix: strip trailing sentence punctuation before parsing, extract and
replace only the hostname (not href) to avoid adding trailing slashes to
bare origins, and restore the stripped tail afterward.
This commit is contained in:
Catriel Müller
2026-05-11 13:18:49 -03:00
parent 9064478949
commit a0dcf76177
2 changed files with 67 additions and 12 deletions
+21 -1
View File
@@ -6,11 +6,31 @@
*
* Example: "curl https://аpitest.com/status" (Cyrillic а)
* → "curl https://xn--pitest-2nf.com/status"
*
* Trailing sentence punctuation (. , ! ? ; :) that \S+ would otherwise consume
* into the URL match is stripped before parsing and left in place afterward, so
* plain-text prose like "see https://example.com." is returned unchanged.
*
* Only the hostname is replaced, not the full href, to avoid side-effects such
* as new URL() appending a trailing slash to bare origins.
*/
export function normalizeUrls(text: string) {
return text.replace(/https?:\/\/\S+/g, (match) => {
// Strip trailing sentence punctuation that \S+ greedily consumes but that
// is almost certainly not part of the URL (e.g. "visit https://x.com.").
const stripped = match.replace(/[.,!?;:)"'\]>]+$/, "")
const tail = match.slice(stripped.length)
try {
return new URL(match).href
const parsed = new URL(stripped)
// Extract the raw hostname from the stripped string so we can replace
// only that part — using href would add a trailing slash to bare origins.
const afterScheme = stripped.indexOf("//") + 2
const slashPos = stripped.indexOf("/", afterScheme)
const rawHost = slashPos === -1 ? stripped.slice(afterScheme) : stripped.slice(afterScheme, slashPos)
const colon = rawHost.indexOf(":")
const rawHostname = colon === -1 ? rawHost : rawHost.slice(0, colon)
if (rawHostname === parsed.hostname) return match // plain ASCII — nothing to change
return stripped.replace(rawHostname, parsed.hostname) + tail
} catch {
return match
}
@@ -14,7 +14,7 @@ describe("normalizeUrls", () => {
test("converts mixed-script hostname to punycode", () => {
// Mix of Latin and Cyrillic in the same label
const input = "https://\u0430pitest.com"
const input = "https://\u0430pitest.com/path"
expect(normalizeUrls(input)).not.toContain("\u0430")
})
@@ -33,20 +33,57 @@ describe("normalizeUrls", () => {
})
describe("plain ASCII URLs are unchanged", () => {
test("leaves a clean https URL untouched", () => {
test("leaves a URL with a path untouched", () => {
const url = "https://apitest.com/status"
expect(normalizeUrls(url)).toBe(url)
})
test("leaves a clean http URL untouched", () => {
test("leaves a URL with path and query string untouched", () => {
const url = "http://example.com/foo?bar=1&baz=2"
expect(normalizeUrls(url)).toBe(url)
})
test("leaves localhost URL untouched", () => {
test("leaves a localhost URL with port untouched", () => {
const url = "http://localhost:3000/api"
expect(normalizeUrls(url)).toBe(url)
})
test("leaves a bare origin untouched (no trailing slash added)", () => {
// Regression: new URL("https://example.com").href === "https://example.com/"
// The old implementation using href would mutate bare origins by adding "/".
const url = "https://example.com"
expect(normalizeUrls(url)).toBe(url)
})
})
describe("trailing sentence punctuation is not consumed into the URL", () => {
test("period at end of sentence is not consumed (was: adds trailing slash)", () => {
// "see https://example.com." — the period ends the sentence, not the URL.
// Old behaviour (bug): returned "see https://example.com./"
expect(normalizeUrls("see https://example.com.")).toBe("see https://example.com.")
})
test("exclamation mark at end of sentence is not consumed", () => {
expect(normalizeUrls("visit https://example.com!")).toBe("visit https://example.com!")
})
test("comma after URL in a list is not consumed", () => {
expect(normalizeUrls("check https://example.com, then continue")).toBe(
"check https://example.com, then continue",
)
})
test("closing parenthesis after URL is not consumed", () => {
expect(normalizeUrls("(see https://example.com)")).toBe("(see https://example.com)")
})
test("trailing punctuation after an IDN URL is stripped correctly and punycode applied", () => {
// Trailing period on a homograph URL: period is sentence punctuation, not part of the URL.
const input = "see https://\u0430pitest.com."
const result = normalizeUrls(input)
expect(result).toBe("see https://xn--pitest-2nf.com.")
expect(result).not.toContain("\u0430")
})
})
describe("URL embedded in a bash command string", () => {
@@ -57,7 +94,7 @@ describe("normalizeUrls", () => {
expect(result).not.toContain("\u0430")
})
test("preserves the non-URL parts of the command", () => {
test("preserves flags and pipe around the URL", () => {
const input = "curl -sSf https://\u0430pitest.com/status | bash"
const result = normalizeUrls(input)
expect(result).toMatch(/^curl -sSf /)
@@ -71,7 +108,7 @@ describe("normalizeUrls", () => {
expect(result).not.toContain("\u0430")
})
test("leaves plain-ASCII command entirely unchanged", () => {
test("leaves a plain-ASCII command entirely unchanged", () => {
const input = "curl -sSf https://kilo.ai/update.sh | bash"
expect(normalizeUrls(input)).toBe(input)
})
@@ -92,18 +129,16 @@ describe("normalizeUrls", () => {
expect(normalizeUrls(text)).toBe(text)
})
test("handles a URL with a path, query, and fragment", () => {
test("preserves path, query string, and fragment after IDN conversion", () => {
const input = "https://\u0430pitest.com/path?q=1#anchor"
const result = normalizeUrls(input)
expect(result).toMatch(/xn--/)
expect(result).toContain("/path?q=1#anchor")
})
test("preserves a URL that fails to parse (e.g. malformed) verbatim", () => {
// A URL that new URL() cannot parse should pass through untouched.
test("preserves a URL that fails to parse verbatim", () => {
const malformed = "https://[unclosed"
const result = normalizeUrls(malformed)
expect(result).toBe(malformed)
expect(normalizeUrls(malformed)).toBe(malformed)
})
})
})