Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions apps/app/src/components/plugin/ExperimentalFileLink.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,28 @@ describe("ExperimentalFileLink", () => {
expect(openFilePreview).not.toHaveBeenCalled();
});

it("does not dispatch a malformed target supplied across a JavaScript boundary", () => {
it("uses a scheme-safe href for a valid scheme-like file name", () => {
const openFilePreview = vi.fn(() => true);
render(
<MemoryRouter>
<AppNavigationHostProvider capabilities={{ openFilePreview }}>
<ExperimentalFileLink target={{ ...target, path: "vscode:foo" }}>
vscode:foo
</ExperimentalFileLink>
</AppNavigationHostProvider>
</MemoryRouter>,
);
const link = screen.getByRole("link", { name: "vscode:foo" });
expect(link.getAttribute("href")).toBe("./vscode%3Afoo");

fireEvent.click(link);
expect(openFilePreview).toHaveBeenCalledWith({
target: { ...target, path: "vscode:foo" },
location: null,
});
});

it("renders a malformed target supplied across a JavaScript boundary as inert", () => {
const openFilePreview = vi.fn(() => true);
render(
<MemoryRouter>
Expand All @@ -67,7 +88,30 @@ describe("ExperimentalFileLink", () => {
</AppNavigationHostProvider>
</MemoryRouter>,
);
fireEvent.click(screen.getByRole("link", { name: "invalid" }));
const invalid = screen.getByText("invalid");
expect(screen.queryByRole("link", { name: "invalid" })).toBeNull();
expect(invalid.getAttribute("href")).toBeNull();
fireEvent.click(invalid);
expect(openFilePreview).not.toHaveBeenCalled();
});

it("renders a path with an unpaired UTF-16 surrogate as inert", () => {
const openFilePreview = vi.fn(() => true);
render(
<MemoryRouter>
<AppNavigationHostProvider capabilities={{ openFilePreview }}>
<ExperimentalFileLink
target={{ ...target, path: String.fromCharCode(0xd800) }}
>
invalid Unicode
</ExperimentalFileLink>
</AppNavigationHostProvider>
</MemoryRouter>,
);
const invalid = screen.getByText("invalid Unicode");
expect(screen.queryByRole("link", { name: "invalid Unicode" })).toBeNull();
expect(invalid.getAttribute("href")).toBeNull();
fireEvent.click(invalid);
expect(openFilePreview).not.toHaveBeenCalled();
});
});
4 changes: 3 additions & 1 deletion apps/app/src/components/plugin/ExperimentalFileLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,10 @@ export function ExperimentalFileLink({
},
[intent, navigation, onClick],
);
const href =
intent === null ? undefined : `./${encodeURIComponent(intent.target.path)}`;
Comment thread
ymichael marked this conversation as resolved.
Resolved
const anchor = (
<RouteAnchor {...anchorProps} href={target.path} onClick={handleClick} />
<RouteAnchor {...anchorProps} href={href} onClick={handleClick} />
);

if (intent === null) return anchor;
Expand Down
93 changes: 93 additions & 0 deletions apps/app/src/components/plugin/ExperimentalUrlLink.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,74 @@ describe("ExperimentalUrlLink", () => {
expect(openUrl).not.toHaveBeenCalled();
});

it.each(["_blank", "preview-pane"])(
"leaves the explicit %s target native when the URL host would accept it",
(target) => {
const openUrl = vi.fn(() => true);
render(
<MemoryRouter>
<AppNavigationHostProvider capabilities={{ openUrl }}>
<ExperimentalUrlLink href="https://example.com" target={target}>
Example
</ExperimentalUrlLink>
</AppNavigationHostProvider>
</MemoryRouter>,
);
const link = screen.getByRole("link", { name: "Example" });
expect(link.getAttribute("target")).toBe(target);
expect(link.getAttribute("rel")).toBe("noopener noreferrer");
expect(fireEvent.click(link)).toBe(true);
expect(openUrl).not.toHaveBeenCalled();
},
);

it("preserves an explicit rel for a named target", () => {
render(
<MemoryRouter>
<ExperimentalUrlLink
href="https://example.com"
target="preview-pane"
rel="opener"
>
Example
</ExperimentalUrlLink>
</MemoryRouter>,
);
expect(
screen.getByRole("link", { name: "Example" }).getAttribute("rel"),
).toBe("opener");
});

it("preserves rel tokens without sacrificing named-target isolation", () => {
render(
<MemoryRouter>
<ExperimentalUrlLink
href="https://example.com"
target="preview-pane"
rel="nofollow"
>
Example
</ExperimentalUrlLink>
</MemoryRouter>,
);
expect(
screen.getByRole("link", { name: "Example" }).getAttribute("rel"),
).toBe("nofollow noopener noreferrer");
});

it("does not add new-context rel tokens to a same-context target", () => {
render(
<MemoryRouter>
<ExperimentalUrlLink href="https://example.com" target="_self">
Example
</ExperimentalUrlLink>
</MemoryRouter>,
);
expect(
screen.getByRole("link", { name: "Example" }).getAttribute("rel"),
).toBeNull();
});

it("routes internal links through browser history before URL preferences", () => {
const openUrl = vi.fn(() => true);
render(
Expand All @@ -62,4 +130,29 @@ describe("ExperimentalUrlLink", () => {
expect(screen.getByText("Settings route")).toBeTruthy();
expect(openUrl).not.toHaveBeenCalled();
});

it("keeps an explicit target on an internal route native", () => {
const openUrl = vi.fn(() => true);
render(
<MemoryRouter initialEntries={["/"]}>
<RouteNavigationProvider>
<AppNavigationHostProvider capabilities={{ openUrl }}>
<ExperimentalUrlLink href="/settings" target="_blank">
Settings in new context
</ExperimentalUrlLink>
<Routes>
<Route path="/settings" element={<div>Settings route</div>} />
</Routes>
</AppNavigationHostProvider>
</RouteNavigationProvider>
</MemoryRouter>,
);
const link = screen.getByRole("link", {
name: "Settings in new context",
});
expect(link.getAttribute("target")).toBe("_blank");
expect(fireEvent.click(link)).toBe(true);
expect(screen.queryByText("Settings route")).toBeNull();
expect(openUrl).not.toHaveBeenCalled();
});
});
35 changes: 32 additions & 3 deletions apps/app/src/components/plugin/ExperimentalUrlLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ function shouldHandleUrlClick(
event.ctrlKey ||
event.metaKey ||
event.shiftKey ||
event.currentTarget.hasAttribute("download")
event.currentTarget.hasAttribute("download") ||
event.currentTarget.hasAttribute("target")
) {
return false;
}
Expand Down Expand Up @@ -51,12 +52,40 @@ export function ExperimentalUrlLink({
},
[href, navigation, onClick],
);
const normalizedTarget = target?.toLowerCase();
const opensNewBrowsingContext =
normalizedTarget !== undefined &&
normalizedTarget !== "" &&
normalizedTarget !== "_self" &&
normalizedTarget !== "_parent" &&
normalizedTarget !== "_top" &&
normalizedTarget !== "_unfencedtop";
const relTokens = rel?.split(/\s+/u).filter(Boolean) ?? [];
const normalizedRelTokens = relTokens.map((token) => token.toLowerCase());
const resolvedRel =
opensNewBrowsingContext && !normalizedRelTokens.includes("opener")
? [
...relTokens,
...(normalizedRelTokens.includes("noopener") ? [] : ["noopener"]),
...(normalizedRelTokens.includes("noreferrer") ? [] : ["noreferrer"]),
].join(" ")
: rel;
if (target !== undefined) {
return (
<a
{...anchorProps}
href={href}
target={target}
rel={resolvedRel}
onClick={handleClick}
/>
);
}
return (
<RouteAnchor
{...anchorProps}
href={href}
target={target}
rel={target === "_blank" ? (rel ?? "noopener noreferrer") : rel}
rel={resolvedRel}
onClick={handleClick}
/>
);
Expand Down
24 changes: 24 additions & 0 deletions apps/app/src/lib/live-file-navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,30 @@ describe("normalizeExperimentalLiveFileTarget", () => {
])("rejects ambiguous or non-exact target %#", (target) => {
expect(normalizeExperimentalLiveFileTarget(target)).toBeNull();
});

it("accepts valid surrogate pairs and rejects unpaired surrogates", () => {
expect(
normalizeExperimentalLiveFileTarget({
kind: "workspace",
environmentId: "env_1",
path: `reports/${String.fromCodePoint(0x1f4c4)}.md`,
}),
).not.toBeNull();

for (const path of [
String.fromCharCode(0xd800),
String.fromCharCode(0xdc00),
`a${String.fromCharCode(0xd800)}b`,
]) {
expect(
normalizeExperimentalLiveFileTarget({
kind: "workspace",
environmentId: "env_1",
path,
}),
).toBeNull();
}
});
});

describe("normalizeExperimentalFileOpenOptions", () => {
Expand Down
Loading
Loading