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
19 changes: 18 additions & 1 deletion packages/ui/components/html-viewer/bridge-script.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,13 @@ export const BRIDGE_SCRIPT = `(function() {
var MAX_SELECTION_TEXT = 10000;

function capSelectionText(text) {
return text.length > MAX_SELECTION_TEXT ? text.slice(0, MAX_SELECTION_TEXT) : text;
if (text.length <= MAX_SELECTION_TEXT) return text;
var cut = MAX_SELECTION_TEXT;
var last = text.charCodeAt(cut - 1);
// Never split a surrogate pair: a lone high surrogate at the cut point
// becomes U+FFFD the moment the string is UTF-8 encoded downstream.
if (last >= 0xd800 && last <= 0xdbff) cut -= 1;
return text.slice(0, cut);
}

function handleSelection(modeOverride, extras) {
Expand Down Expand Up @@ -1007,10 +1013,21 @@ export const BRIDGE_SCRIPT = `(function() {
return null;
}

// Each ancestor step runs a document-wide uniqueness query against a
// selector that grows with the path, so cost is quadratic in depth. Real
// documents anchor within a few levels; a degenerate deeply-wrapped chain
// (templated exports, generated markup) must not freeze the tab on a
// single pinpoint click. Past the cap the anchor is abandoned (fail
// closed): text-search restoration still works, and no anchor beats one
// that costs seconds of synchronous main-thread time.
var MAX_ANCHOR_PATH_DEPTH = 40;

function buildAnchorSelector(el) {
var path = [];
var current = el;
var depth = 0;
while (current && current.nodeType === 1 && current !== document.body && current !== document.documentElement) {
if (++depth > MAX_ANCHOR_PATH_DEPTH) return null;
var semantic = semanticSelectorFor(current);
if (semantic) {
path.unshift(semantic);
Expand Down
24 changes: 24 additions & 0 deletions packages/ui/components/html-viewer/htmlPinpointProtocol.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,30 @@ describe.if(hasDom)('parseBridgeMessage selection additions', () => {
expect((parsed as { anchor?: unknown }).anchor).toBeUndefined();
});

test('truncation never splits a surrogate pair at the cap boundary', () => {
// An astral character straddling the cut would leave a lone high
// surrogate that turns into U+FFFD once UTF-8-encoded (drafts, feedback,
// share URLs) — the cut must back off one unit instead.
const cap = hookModule!.MAX_SELECTION_TEXT_LENGTH;
const straddling = 'x'.repeat(cap - 1) + '\u{1F600}' + 'tail';
const parsed = hookModule!.parseBridgeMessage({
type: 'plannotator-bridge-selection',
text: straddling,
rect,
}) as { text: string };
expect(parsed.text.length).toBe(cap - 1);
expect(parsed.text.endsWith('x')).toBe(true);
expect(/[\uD800-\uDBFF]$/.test(parsed.text)).toBe(false);
// A pair that fits entirely under the cap is untouched.
const fitting = 'y'.repeat(cap - 2) + '\u{1F600}';
const kept = hookModule!.parseBridgeMessage({
type: 'plannotator-bridge-selection',
text: fitting,
rect,
}) as { text: string };
expect(kept.text).toBe(fitting);
});

test('selection text is truncated at the parse boundary, not rejected', () => {
// The page controls element text entirely, so one pinpoint click on a huge
// <pre> could otherwise ship an unbounded string into React state, drafts,
Expand Down
44 changes: 44 additions & 0 deletions packages/ui/components/html-viewer/srcdoc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,50 @@ describe.if(hasDom)("bridge theme handler (DOM)", () => {
document.body.replaceChildren();
});

test("deeply nested targets get no anchor instead of a quadratic selector walk", async () => {
// Each ancestor step costs a document-wide uniqueness query against a
// growing selector, so unbounded depth freezes the tab on one click
// (measured ~58s at depth 800 pre-cap). Past MAX_ANCHOR_PATH_DEPTH the
// anchor is abandoned and restoration falls back to text search.
// Two structurally identical chains: every positional selector along the
// walk matches both branches, so uniqueness cannot short-circuit before
// the depth cap fires (the branch point sits above it).
const DEPTH = 60;
let chainA = "<p>Deeply buried text</p>";
let chainB = "<p>Other branch text</p>";
for (let i = 0; i < DEPTH; i++) {
chainA = `<div>${chainA}</div>`;
chainB = `<div>${chainB}</div>`;
}
document.body.innerHTML = chainA + chainB;
const target = document.querySelector<HTMLElement>("p");
if (!target || target.textContent !== "Deeply buried text") throw new Error("deep target missing");
postBridge({ type: "plannotator-bridge-set-input-method", method: "pinpoint" });

const messages: Array<Record<string, unknown>> = [];
const collect = (event: MessageEvent) => {
const data = bridgeMessageData(event);
if (data?.type === "plannotator-bridge-selection") messages.push(data);
};
window.addEventListener("message", collect);
const started = performance.now();
target.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true }));
const elapsed = performance.now() - started;
await new Promise((resolve) => setTimeout(resolve, 0));
window.removeEventListener("message", collect);

expect(messages.length).toBe(1);
expect(messages[0]!.text).toBe("Deeply buried text");
expect((messages[0] as { anchor?: unknown }).anchor).toBeUndefined();
// Bounded work: the capped walk must complete in interactive time even
// under happy-dom's slow selector engine.
expect(elapsed).toBeLessThan(2000);

postBridge({ type: "plannotator-bridge-cancel-selection" });
postBridge({ type: "plannotator-bridge-set-input-method", method: "drag" });
document.body.replaceChildren();
});

test("behavioral-attribute anchors never bypass the text check", () => {
// Regenerated page: the button kept its role but its meaning flipped; the
// annotated text moved into a sibling paragraph. The role anchor must NOT
Expand Down
14 changes: 11 additions & 3 deletions packages/ui/components/html-viewer/useHtmlAnnotation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ const MAX_ANCHOR_TEXT_LENGTH = 400;
// MAX_SELECTION_TEXT in bridge-script.ts; this side is the authoritative one.
export const MAX_SELECTION_TEXT_LENGTH = 10000;

/** Truncate to the cap without ever splitting a UTF-16 surrogate pair (a
* lone high surrogate becomes U+FFFD once UTF-8-encoded downstream). */
export function capSelectionText(text: string): string {
if (text.length <= MAX_SELECTION_TEXT_LENGTH) return text;
let cut = MAX_SELECTION_TEXT_LENGTH;
const last = text.charCodeAt(cut - 1);
if (last >= 0xd800 && last <= 0xdbff) cut -= 1;
return text.slice(0, cut);
}

/** Validate a bridge-posted element anchor. Exported for protocol tests. */
export function parseHtmlElementAnchor(value: unknown): HtmlElementAnchor | null {
if (!isRecord(value)) return null;
Expand Down Expand Up @@ -127,9 +137,7 @@ export function parseBridgeMessage(value: unknown): BridgeMessage | null {
if (typeof value.text !== "string" || !rect) return null;
return {
type: value.type,
text: value.text.length > MAX_SELECTION_TEXT_LENGTH
? value.text.slice(0, MAX_SELECTION_TEXT_LENGTH)
: value.text,
text: capSelectionText(value.text),
rect,
modeOverride: parseEditorMode(value.modeOverride),
anchor: parseHtmlElementAnchor(value.anchor) ?? undefined,
Expand Down
8 changes: 8 additions & 0 deletions scripts/install.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -1137,8 +1137,16 @@ if "!SKIP_SKILLS!"=="1" goto skills_checkout_done

set "CLONE_OK=0"
set "SPARSE_CLONE=1"
REM LC_ALL=C pins git's error strings to English for the capability probe
REM below: a localized git would emit a translated "unknown option" message
REM the findstr match misses, sending old-git non-English users to a hard
REM failure instead of the fallback. Saved and restored around the probe.
set "PLANNOTATOR_SAVED_LC_ALL=!LC_ALL!"
set "LC_ALL=C"
git clone --depth 1 --filter=blob:none --sparse "https://github.com/!REPO!.git" --branch "!TAG!" "!SKILLS_TMP!\repo" >nul 2>"!GIT_ERR_FILE!"
if !ERRORLEVEL! equ 0 set "CLONE_OK=1"
set "LC_ALL=!PLANNOTATOR_SAVED_LC_ALL!"
set "PLANNOTATOR_SAVED_LC_ALL="

REM Capability probe, not a version parse (same philosophy as the GitButler
REM flag probing in packages/shared/gitbutler-core.ts): `git clone --sparse`
Expand Down
8 changes: 7 additions & 1 deletion scripts/install.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,13 @@ try {
# $null (#1238) so failures can be diagnosed.
$gitErrFile = Join-Path $skillsTmp "git-stderr.txt"
if (-not $skipSkillsResolved) {
& { $local:ErrorActionPreference = 'Continue'; git clone --depth 1 --filter=blob:none --sparse "https://github.com/$repo.git" --branch $latestTag "$skillsTmp\repo" 2>$gitErrFile }
# LC_ALL=C pins git's error strings to English for the capability
# probe below: a localized git would emit a translated "unknown
# option" message the match misses, sending old-git non-English
# users to a hard failure instead of the fallback. Saved/restored
# around the call because $env: changes are process-wide. Kept on
# one line for the install.test.ts scoped-git-call scanner.
& { $local:ErrorActionPreference = 'Continue'; $prevLcAll = $env:LC_ALL; $env:LC_ALL = 'C'; git clone --depth 1 --filter=blob:none --sparse "https://github.com/$repo.git" --branch $latestTag "$skillsTmp\repo" 2>$gitErrFile; if ($null -eq $prevLcAll) { Remove-Item Env:LC_ALL -ErrorAction SilentlyContinue } else { $env:LC_ALL = $prevLcAll } }
if (-not (Test-Path "$skillsTmp\repo")) {
$cloneErr = ""
if (Test-Path $gitErrFile) { $cloneErr = [System.IO.File]::ReadAllText($gitErrFile) }
Expand Down
7 changes: 6 additions & 1 deletion scripts/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1638,7 +1638,12 @@ checkout_failed=0
tail -n 5 "$git_err" >&2
}
sparse_clone=1
if ! git clone --depth 1 --filter=blob:none --sparse \
# LC_ALL=C pins git's error strings to English: the capability probe below
# matches the literal "unknown option ... sparse" text, and a localized
# git (standard Linux NLS builds) would otherwise emit a translated
# message the match misses, sending old-git non-English users to a hard
# failure instead of the fallback.
if ! LC_ALL=C LANGUAGE=C git clone --depth 1 --filter=blob:none --sparse \
"https://github.com/${REPO}.git" --branch "$latest_tag" repo 2>"$git_err"; then
# Capability probe, not a version parse (same philosophy as the
# GitButler flag probing in packages/shared/gitbutler-core.ts):
Expand Down
5 changes: 4 additions & 1 deletion scripts/install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1909,8 +1909,11 @@ describe("install shared behavior", () => {
// on stderr, so without the scope the skill install failed on the line
// announcing the clone had started. See #1162. Failure detection must
// stay exit-code/Test-Path based, never throw-based.
// The sparse-probe clone also pins LC_ALL=C inside the same scoped
// block (the "unknown option" capability match is English-only), so its
// expected prefix carries the env save alongside the preference.
expect(ps).toContain(
"& { $local:ErrorActionPreference = 'Continue'; git clone",
"& { $local:ErrorActionPreference = 'Continue'; $prevLcAll = $env:LC_ALL; $env:LC_ALL = 'C'; git clone",
);
expect(ps).toContain(
"& { $local:ErrorActionPreference = 'Continue'; git sparse-checkout set",
Expand Down