Skip to content

fix(templates): strip backslash from escaped \{ in token interpolation - #308

Open
shuvamk wants to merge 1 commit into
CoreBunch:mainfrom
shuvamk:fix/token-escape-backslash-leak
Open

fix(templates): strip backslash from escaped \{ in token interpolation#308
shuvamk wants to merge 1 commit into
CoreBunch:mainfrom
shuvamk:fix/token-escape-backslash-leak

Conversation

@shuvamk

@shuvamk shuvamk commented Jul 29, 2026

Copy link
Copy Markdown

Summary

An escaped \{ in a string-typed prop value leaks its backslash into published output. The module documents the escape as "emits a literal {not-a-token} with no replacement" (tokenInterpolation.ts:22-23) and parseTokenString implements it correctly — but interpolateTokens never reaches the parser for this input.

The culprit is the containsTokens fast-path heuristic, which deliberately excluded escaped braces:

export function containsTokens(value: string): boolean {
  if (value.length < 5) return false
  for (let i = 0; i < value.length; i++) {
    const c = value[i]
    if (c === '{' && (i === 0 || value[i - 1] !== '\\')) return true  // skips \{
  }
  return false
}

interpolateTokens then short-circuits on the false result:

if (!containsTokens(input)) return input   // returns input verbatim, backslash intact

So "Literal \{not-a-token}" is returned unchanged instead of being parsed to "Literal {not-a-token}". parseTokenString on the same input already yields the stripped literal, so the heuristic and the parser disagree.

The existing test crystallized the bug — its title states the correct behavior while the assertion encoded the wrong one:

it('returns true even for escaped { (the parser will resolve it correctly)', () => {
  expect(containsTokens('Literal \\{not-a-token}')).toBe(false)   // contradicts the title
})

Reproduction

interpolateTokens('Literal \{not-a-token}', ctx)
  before: 'Literal \{not-a-token}'   ← backslash leaks
  after : 'Literal {not-a-token}'    ← literal, as documented

Fix

Any { — escaped or not — needs a parse (the parser is what strips the backslash), so containsTokens now returns true whenever a brace is present.

Safe at every call site:

  • publisher/dynamicDetection.ts — an escape-only string parses to text-only segments (no kind: 'token'), so the node correctly stays static; no page is newly promoted to a dynamic hole.
  • templates/dynamicBindings.ts (both sites) — now strip the backslash as documented instead of leaking it.

The < 5 length floor is dropped with it: a real token's source (page/site/route/currentEntry/parentEntry) already makes it far longer than 5 chars, but a short escape like \{x} is only 4 and was wrongly skipped.

Verification

  • bun run build
  • bun test (6333 pass, 0 fail; the flipped assertion + two new interpolateTokens escape tests fail on main and pass with the fix)
  • bun run lint

Checklist

  • Tests cover behavior changes — corrected the self-contradictory containsTokens assertion and added interpolateTokens escape cases (with and without a real token alongside).
  • Docs were updated when behavior, config, deployment, or public surfaces changed — n/a (internal engine; the escape was already documented).
  • No compatibility shim was added for old pre-release behavior.
  • No secrets, local databases, uploads, or generated artifacts are included.

`containsTokens` excluded escaped `\{`, so `interpolateTokens` took its
no-token fast path and returned the string verbatim — the backslash of an
escaped `\{not-a-token}` leaked into published output instead of being
consumed. `parseTokenString` already implements the documented escape
(emit a literal `{`), but the heuristic gated it out, and its own unit
test asserted the buggy `false` under a title claiming the opposite.

Any `{` — escaped or not — needs a parse, so `containsTokens` now reports
true whenever a brace is present. Escaped-only strings stay static in
`dynamicDetection` (they parse to text-only segments), and the binding
paths now strip the backslash as documented.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant