fix: read the completions map by own property, not through the prototype
Addresses Copilot's round-17 review on #2035. A real crash, and a class the earlier prototype work missed rather than a narrower version of it. `toString`, `constructor` and `__proto__` are valid RFC 6570 variable names, and `completions` starts as `{}`. So for such a name: completions["toString"] ?? [] => the prototype's FUNCTION `??` catches only null and undefined, so that function went to Mantine's Autocomplete as its `data`, and the field crashed on first render with `TypeError: data.map is not a function` -- reproduced by reverting the fix under the new test. The earlier rounds fixed this for the *values* map, which is seeded with every declared variable and so always has the key as an own property. The completions map is the one name-keyed map this component reads WITHOUT having seeded it, which is what made it the surviving instance. Both reads now go through `Object.hasOwn` -- the render, and the stale-dropdown check that clears a variable's options on the next keystroke. Only the autocomplete branch could reach it, so the regression drives that branch: completions supported, a `{toString}` template, type and submit. npm run ci passes. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
7d9410cbe0
commit
05bbfe5074
2 changed files with 45 additions and 2 deletions
|
|
@ -318,6 +318,30 @@ describe("ResourceTemplatePanel", () => {
|
|||
expect(screen.getByText("Still needed: a")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders an autocomplete field for a name that collides with Object.prototype", async () => {
|
||||
// `toString` is a valid RFC 6570 varname and `completions` starts empty,
|
||||
// so a bare `completions[varName] ?? []` handed Mantine the prototype's
|
||||
// *function* as its options array (`??` catches only null/undefined) and
|
||||
// the field crashed on first render.
|
||||
const user = userEvent.setup();
|
||||
const onCompleteArgument = vi.fn().mockResolvedValue(["a", "b"]);
|
||||
const onReadResource = vi.fn();
|
||||
renderWithMantine(
|
||||
<ResourceTemplatePanel
|
||||
template={{ name: "Proto", uriTemplate: "x://{toString}" }}
|
||||
onReadResource={onReadResource}
|
||||
onCompleteArgument={onCompleteArgument}
|
||||
completionsSupported
|
||||
/>,
|
||||
);
|
||||
// By role, not label: "toString" also appears in the "Still needed" line.
|
||||
const input = screen.getByRole("textbox", { name: "toString" });
|
||||
expect(input).toBeInTheDocument();
|
||||
await user.type(input, "v");
|
||||
await user.click(screen.getByRole("button", { name: "Read Resource" }));
|
||||
expect(onReadResource).toHaveBeenCalledWith("x://v");
|
||||
});
|
||||
|
||||
it("shows the malformed template unexpanded in the preview", () => {
|
||||
renderWithMantine(
|
||||
<ResourceTemplatePanel
|
||||
|
|
|
|||
|
|
@ -47,6 +47,24 @@ export interface ResourceTemplatePanelProps {
|
|||
|
||||
const COMPLETION_DEBOUNCE_MS = 300;
|
||||
|
||||
/**
|
||||
* The completions fetched for one variable, ignoring anything inherited from
|
||||
* `Object.prototype`.
|
||||
*
|
||||
* `toString`, `constructor` and `__proto__` are valid RFC 6570 variable names,
|
||||
* and this map starts empty — so a bare `completions[varName] ?? []` returned
|
||||
* the prototype's *function* for such a name (`??` only catches null and
|
||||
* undefined) and handed it to Mantine as its `data` array, crashing the field
|
||||
* on first render. Same hazard the expansion path fixed for values; this is
|
||||
* the one place the component reads a name-keyed map it did not seed.
|
||||
*/
|
||||
function completionsFor(
|
||||
completions: Record<string, string[]>,
|
||||
varName: string,
|
||||
): string[] {
|
||||
return Object.hasOwn(completions, varName) ? completions[varName] : [];
|
||||
}
|
||||
|
||||
const HeaderRow = Group.withProps({
|
||||
justify: "space-between",
|
||||
wrap: "nowrap",
|
||||
|
|
@ -206,7 +224,8 @@ export function ResourceTemplatePanel({
|
|||
// show ghost suggestions from the old keystroke while the new
|
||||
// request is in flight (300ms debounce + network latency).
|
||||
setCompletions((prev) => {
|
||||
if (prev[varName] === undefined) return prev;
|
||||
// Own-property, as above: an inherited member is not a stale dropdown.
|
||||
if (!Object.hasOwn(prev, varName)) return prev;
|
||||
const next = { ...prev };
|
||||
delete next[varName];
|
||||
return next;
|
||||
|
|
@ -328,7 +347,7 @@ export function ResourceTemplatePanel({
|
|||
description={description}
|
||||
placeholder={`Enter ${varName}`}
|
||||
value={fieldValue}
|
||||
data={completions[varName] ?? []}
|
||||
data={completionsFor(completions, varName)}
|
||||
// The server already filtered the values for the typed
|
||||
// prefix; passing options through verbatim avoids hiding
|
||||
// valid suggestions when the input is empty or doesn't
|
||||
|
|
|
|||
Loading…
Reference in a new issue