Skip to content

Commit 05bbfe5

Browse files
committed
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>
1 parent 7d9410c commit 05bbfe5

2 files changed

Lines changed: 45 additions & 2 deletions

File tree

‎clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,30 @@ describe("ResourceTemplatePanel", () => {
318318
expect(screen.getByText("Still needed: a")).toBeInTheDocument();
319319
});
320320

321+
it("renders an autocomplete field for a name that collides with Object.prototype", async () => {
322+
// `toString` is a valid RFC 6570 varname and `completions` starts empty,
323+
// so a bare `completions[varName] ?? []` handed Mantine the prototype's
324+
// *function* as its options array (`??` catches only null/undefined) and
325+
// the field crashed on first render.
326+
const user = userEvent.setup();
327+
const onCompleteArgument = vi.fn().mockResolvedValue(["a", "b"]);
328+
const onReadResource = vi.fn();
329+
renderWithMantine(
330+
<ResourceTemplatePanel
331+
template={{ name: "Proto", uriTemplate: "x://{toString}" }}
332+
onReadResource={onReadResource}
333+
onCompleteArgument={onCompleteArgument}
334+
completionsSupported
335+
/>,
336+
);
337+
// By role, not label: "toString" also appears in the "Still needed" line.
338+
const input = screen.getByRole("textbox", { name: "toString" });
339+
expect(input).toBeInTheDocument();
340+
await user.type(input, "v");
341+
await user.click(screen.getByRole("button", { name: "Read Resource" }));
342+
expect(onReadResource).toHaveBeenCalledWith("x://v");
343+
});
344+
321345
it("shows the malformed template unexpanded in the preview", () => {
322346
renderWithMantine(
323347
<ResourceTemplatePanel

‎clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,24 @@ export interface ResourceTemplatePanelProps {
4747

4848
const COMPLETION_DEBOUNCE_MS = 300;
4949

50+
/**
51+
* The completions fetched for one variable, ignoring anything inherited from
52+
* `Object.prototype`.
53+
*
54+
* `toString`, `constructor` and `__proto__` are valid RFC 6570 variable names,
55+
* and this map starts empty — so a bare `completions[varName] ?? []` returned
56+
* the prototype's *function* for such a name (`??` only catches null and
57+
* undefined) and handed it to Mantine as its `data` array, crashing the field
58+
* on first render. Same hazard the expansion path fixed for values; this is
59+
* the one place the component reads a name-keyed map it did not seed.
60+
*/
61+
function completionsFor(
62+
completions: Record<string, string[]>,
63+
varName: string,
64+
): string[] {
65+
return Object.hasOwn(completions, varName) ? completions[varName] : [];
66+
}
67+
5068
const HeaderRow = Group.withProps({
5169
justify: "space-between",
5270
wrap: "nowrap",
@@ -206,7 +224,8 @@ export function ResourceTemplatePanel({
206224
// show ghost suggestions from the old keystroke while the new
207225
// request is in flight (300ms debounce + network latency).
208226
setCompletions((prev) => {
209-
if (prev[varName] === undefined) return prev;
227+
// Own-property, as above: an inherited member is not a stale dropdown.
228+
if (!Object.hasOwn(prev, varName)) return prev;
210229
const next = { ...prev };
211230
delete next[varName];
212231
return next;
@@ -328,7 +347,7 @@ export function ResourceTemplatePanel({
328347
description={description}
329348
placeholder={`Enter ${varName}`}
330349
value={fieldValue}
331-
data={completions[varName] ?? []}
350+
data={completionsFor(completions, varName)}
332351
// The server already filtered the values for the typed
333352
// prefix; passing options through verbatim avoids hiding
334353
// valid suggestions when the input is empty or doesn't

0 commit comments

Comments
 (0)