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 <cliff@futurescale.com>
This commit is contained in:
parent
c6fb627142
commit
6a345ae77c
2 changed files with 56 additions and 11 deletions
|
|
@ -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",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue