From 6a345ae77c3c467b6525c62554b3567f0e614337 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 17 Aug 2026 00:22:09 -0400 Subject: [PATCH] fix: keep the URI preview from throwing or promising an unsendable URI Addresses Copilot's round-12 review on #2035. Both halves are regressions from my own round-10 rework, which replaced "expand a rewritten template through the lenient expandUriTemplate" with a part-by-part assembly and so lost two properties that implementation had for free. Reproduced on c6fb6271: previewUriTemplate("x://{v}", { v: "\ud800" }) => throws URIError previewUriTemplate("x://{a,}", { a: "1" }) => "x://1" The first is the worse one: the preview runs during render, so an escaping throw unmounts the whole panel rather than disabling its button -- and an unpaired surrogate is real input, since `encodeURIComponent` raises URIError on one and a paste can deliver it. The panel already reports that case properly through `tryExpandUriTemplate`; the display path now degrades to the raw template instead of taking the panel down with it. The second promised a URI the user could never send: `{a,}` is an invalid template whose submission is refused, but `a` survives into the expression's varspecs, so filling it previewed `x://1`. An invalid expression is now left standing as written, which is what the rest of the panel already says about it. Tests for both, plus one asserting the valid expressions around an invalid one still preview. npm run ci passes. Signed-off-by: cliffhall --- clients/web/src/utils/uriTemplate.test.ts | 27 +++++++++++++++ clients/web/src/utils/uriTemplate.ts | 40 ++++++++++++++++------- 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/clients/web/src/utils/uriTemplate.test.ts b/clients/web/src/utils/uriTemplate.test.ts index f25c9e73..72e5f824 100644 --- a/clients/web/src/utils/uriTemplate.test.ts +++ b/clients/web/src/utils/uriTemplate.test.ts @@ -99,3 +99,30 @@ describe("previewUriTemplate - literals", () => { ); }); }); + +describe("previewUriTemplate - it must never throw or over-promise", () => { + // Both of these were free when the preview expanded a rewritten template + // through the lenient `expandUriTemplate`; assembling part by part has to + // reinstate them explicitly. + it("falls back to the raw template when a value cannot be encoded", () => { + // An unpaired surrogate has no UTF-8 encoding, so `encodeURIComponent` + // raises URIError -- and this runs during render, where an escaping throw + // unmounts the panel rather than disabling its button. A paste delivers it. + expect(previewUriTemplate("x://{v}", { v: "\ud800" })).toBe("x://{v}"); + }); + + it("leaves an invalid expression standing rather than expanding it", () => { + // `{a,}` keeps `a` in its varspecs, so expanding it would preview a URI + // for a template whose submission is refused outright. + expect(previewUriTemplate("x://{a,}", { a: "1" })).toBe("x://{a,}"); + expect(previewUriTemplate("x://{id:abc}", { "id:abc": "1" })).toBe( + "x://{id:abc}", + ); + }); + + it("still previews the valid expressions around an invalid one", () => { + expect(previewUriTemplate("x://{a,}/{b}", { a: "1", b: "2" })).toBe( + "x://{a,}/2", + ); + }); +}); diff --git a/clients/web/src/utils/uriTemplate.ts b/clients/web/src/utils/uriTemplate.ts index 78db1bc9..918f5907 100644 --- a/clients/web/src/utils/uriTemplate.ts +++ b/clients/web/src/utils/uriTemplate.ts @@ -50,6 +50,19 @@ export type { * Each half still comes from the shared expander -- `encodeLiteral` for literal * runs, `expandTemplateExpression` for filled ones -- so the preview cannot * promise a URI that submitting would not send. + * + * Two guards keep that promise, both of them things the previous + * expand-the-whole-template implementation got for free from the lenient + * `expandUriTemplate` it called: + * + * - **An invalid expression is left standing.** `{a,}` still parses `a` into + * its varspecs, so filling `a` would preview `x://1` for a template whose + * submission is refused outright -- a URI the user could never send. + * - **The whole thing is wrapped.** This runs during render, where a throw + * unmounts the panel instead of disabling its button, and encoding can throw + * for real input: `encodeURIComponent` raises `URIError` on an unpaired + * surrogate, which a paste can deliver. The panel already reports that case + * through `tryExpandUriTemplate`; here it degrades to the raw template. */ export function previewUriTemplate( uriTemplate: string, @@ -57,15 +70,20 @@ export function previewUriTemplate( ): string { const defined = definedValues(values); - return parseUriTemplate(uriTemplate) - .map((part) => { - if (part.kind === "literal") return encodeLiteral(part.text); - // `Object.hasOwn`, not a bare lookup: `toString` and `constructor` are - // valid RFC 6570 variable names, and a plain lookup would find - // `Object.prototype`'s member and treat a blank field as filled. - return part.names.some((name) => Object.hasOwn(defined, name)) - ? expandTemplateExpression(part, defined) - : part.source; - }) - .join(""); + try { + return parseUriTemplate(uriTemplate) + .map((part) => { + if (part.kind === "literal") return encodeLiteral(part.text); + if (part.invalid) return part.source; + // `Object.hasOwn`, not a bare lookup: `toString` and `constructor` are + // valid RFC 6570 variable names, and a plain lookup would find + // `Object.prototype`'s member and treat a blank field as filled. + return part.names.some((name) => Object.hasOwn(defined, name)) + ? expandTemplateExpression(part, defined) + : part.source; + }) + .join(""); + } catch { + return uriTemplate; + } }