diff --git a/apps/yaak-client/components/core/Editor/json-lint.test.ts b/apps/yaak-client/components/core/Editor/json-lint.test.ts new file mode 100644 index 000000000..58c5c40f0 --- /dev/null +++ b/apps/yaak-client/components/core/Editor/json-lint.test.ts @@ -0,0 +1,46 @@ +import type { EditorView } from "@codemirror/view"; +import { describe, expect, test } from "vite-plus/test"; +import { jsonParseLinter } from "./json-lint"; + +// The linter only reads the document text off the view +function view(doc: string): EditorView { + return { state: { doc: { toString: () => doc } } } as EditorView; +} + +const lint = jsonParseLinter(); + +describe("jsonParseLinter", () => { + test("reports nothing for an empty document", () => { + expect(lint(view(" "))).toEqual([]); + }); + + test("reports nothing for valid JSON", () => { + expect(lint(view('{"a": 1}'))).toEqual([]); + }); + + test("reports a message for invalid JSON", () => { + expect(lint(view('{"a": }'))).toHaveLength(1); + }); + + test("ignores a template tag standing in for a value", () => { + expect(lint(view('{"a": "${[ my_var ]}"}'))).toEqual([]); + }); + + test("ignores a tag whose quoted argument contains `]}`", () => { + expect(lint(view(`{"a": "\${[ fn(arg='x]}y') ]}"}`))).toEqual([]); + }); + + test("ignores a tag whose quoted argument contains an escaped quote", () => { + expect(lint(view(`{"a": "\${[ fn(arg='it\\'s ]}') ]}"}`))).toEqual([]); + }); + + test("still reports errors around a tag with a quoted `]}`", () => { + expect(lint(view(`{"a": "\${[ fn(arg='x]}y') ]}",}`))).toHaveLength(1); + }); + + test("keeps the tag's length so error offsets stay correct", () => { + const doc = `{"a" "\${[ fn(arg='x]}y') ]}"}`; + const [error] = lint(view(doc)); + expect(error?.from).toEqual(doc.indexOf('"${[')); + }); +}); diff --git a/apps/yaak-client/components/core/Editor/json-lint.ts b/apps/yaak-client/components/core/Editor/json-lint.ts index 1a46bd255..004c12c09 100644 --- a/apps/yaak-client/components/core/Editor/json-lint.ts +++ b/apps/yaak-client/components/core/Editor/json-lint.ts @@ -1,8 +1,7 @@ import type { Diagnostic } from "@codemirror/lint"; import type { EditorView } from "@codemirror/view"; import { type ParseError, parse, printParseErrorCode } from "jsonc-parser"; - -const TEMPLATE_SYNTAX_REGEX = /\$\{\[[\s\S]*?]}/g; +import { replaceTemplateTags } from "../../../lib/templateTags"; // jsonc-parser reports error codes, so these are the words the editor shows for them const MESSAGES: Record = { @@ -36,7 +35,7 @@ export function jsonParseLinter(options?: JsonLintOptions) { // We need lint to not break on stuff like {"foo:" ${[ ... ]}} so we'll replace all template // syntax with repeating `1` characters, so it's valid JSON and the position is still correct. - const escapedDoc = doc.replace(TEMPLATE_SYNTAX_REGEX, (m) => "1".repeat(m.length)); + const escapedDoc = replaceTemplateTags(doc, (m) => "1".repeat(m.end - m.start)); const errors: ParseError[] = []; parse(escapedDoc, errors, { diff --git a/apps/yaak-client/components/core/Editor/twig/highlight.ts b/apps/yaak-client/components/core/Editor/twig/highlight.ts index d3a614ec3..2645ade59 100644 --- a/apps/yaak-client/components/core/Editor/twig/highlight.ts +++ b/apps/yaak-client/components/core/Editor/twig/highlight.ts @@ -4,4 +4,6 @@ export const highlight = styleTags({ TagOpen: t.bracket, TagClose: t.bracket, TagContent: t.keyword, + // Strings are part of the tag body, so they get the same color as the rest of it + TagString: t.keyword, }); diff --git a/apps/yaak-client/components/core/Editor/twig/twig.grammar b/apps/yaak-client/components/core/Editor/twig/twig.grammar index 6ca447c40..fecf4a84f 100644 --- a/apps/yaak-client/components/core/Editor/twig/twig.grammar +++ b/apps/yaak-client/components/core/Editor/twig/twig.grammar @@ -1,13 +1,17 @@ @top Template { (Tag | Text)* } +// `TagString` keeps a quoted argument from closing the tag, so `${[ fn(a='x]}y') ]}` stays a +// single Tag. Only a terminated string matches, so a half-typed `'` degrades to TagContent and +// the tag still closes at the next `]}`. @local tokens { TagClose { "]}" } + TagString { "'" (![\\'] | "\\" _)* "'" } @else TagContent } @skip { } { TagOpen { "${[" } - Tag { TagOpen (TagContent)+ TagClose } + Tag { TagOpen (TagContent | TagString)+ TagClose } } @tokens { diff --git a/apps/yaak-client/components/core/Editor/twig/twig.terms.ts b/apps/yaak-client/components/core/Editor/twig/twig.terms.ts index 7ac3c791f..053c411f1 100644 --- a/apps/yaak-client/components/core/Editor/twig/twig.terms.ts +++ b/apps/yaak-client/components/core/Editor/twig/twig.terms.ts @@ -3,5 +3,6 @@ export const Template = 1, Tag = 2, TagOpen = 3, TagContent = 4, - TagClose = 5, - Text = 6; + TagString = 5, + TagClose = 6, + Text = 7; diff --git a/apps/yaak-client/components/core/Editor/twig/twig.test.ts b/apps/yaak-client/components/core/Editor/twig/twig.test.ts index 2ce95f01f..5d5d3527a 100644 --- a/apps/yaak-client/components/core/Editor/twig/twig.test.ts +++ b/apps/yaak-client/components/core/Editor/twig/twig.test.ts @@ -23,6 +23,10 @@ function hasError(input: string): boolean { return getNodeNames(input).includes("⚠"); } +function countTags(input: string): number { + return getNodeNames(input).filter((n) => n === "Tag").length; +} + describe("twig grammar", () => { describe("${[var]} format (valid template tags)", () => { test("parses simple variable as Tag", () => { @@ -83,6 +87,50 @@ describe("twig grammar", () => { }); }); + describe("quoted strings inside tags", () => { + test("keeps a quoted `]}` inside the tag", () => { + const input = "${[ fn(a='x]}y') ]}"; + expect(countTags(input)).toBe(1); + expect(hasError(input)).toBe(false); + }); + + test("keeps an escaped quote inside the string", () => { + const input = "${[ fn(a='it\\'s ]}') ]}"; + expect(countTags(input)).toBe(1); + expect(hasError(input)).toBe(false); + }); + + test("parses a b64 string as one Tag", () => { + const input = "${[ b64'Zm9v' ]}"; + expect(countTags(input)).toBe(1); + expect(hasError(input)).toBe(false); + }); + + test("closes at the first `]}` when a quote is left unterminated", () => { + // Half-typed strings must not swallow the rest of the document + const input = "${[ fn(a='unterminated ]} after"; + expect(countTags(input)).toBe(1); + expect(hasError(input)).toBe(false); + const tag = parser.parse(input).topNode.getChild("Tag"); + expect(input.slice(tag?.from ?? 0, tag?.to ?? 0)).toBe("${[ fn(a='unterminated ]}"); + }); + + test("handles two tags with quoted `]}` in the same string", () => { + const input = "${[ a(x='1]}') ]} and ${[ b(y='2]}') ]}"; + expect(countTags(input)).toBe(2); + expect(hasError(input)).toBe(false); + }); + + test("keeps JSON around a tag with a quoted `]}` intact", () => { + const tag = "${[ fn(a='x]}y') ]}"; + const json = '{"a": "' + tag + '"}'; + expect(countTags(json)).toBe(1); + expect(hasError(json)).toBe(false); + const node = parser.parse(json).topNode.getChild("Tag"); + expect(json.slice(node?.from ?? 0, node?.to ?? 0)).toBe(tag); + }); + }); + describe("edge cases", () => { test("handles $ at end of string", () => { expect(hasError("hello$")).toBe(false); diff --git a/apps/yaak-client/components/core/Editor/twig/twig.ts b/apps/yaak-client/components/core/Editor/twig/twig.ts index 78af862c7..181356235 100644 --- a/apps/yaak-client/components/core/Editor/twig/twig.ts +++ b/apps/yaak-client/components/core/Editor/twig/twig.ts @@ -4,17 +4,24 @@ import { highlight } from "./highlight"; export const parser = LRParser.deserialize({ version: 14, states: - "!^QQOPOOOOOO'#C_'#C_OYOQO'#C^OOOO'#Cc'#CcQQOPOOOOOO'#Cd'#CdO_OQO,58xOOOO-E6a-E6aOOOO-E6b-E6bOOOO1G.d1G.d", - stateData: "g~OUROYPO~OSTO~OSTOTXO~O", - goto: "nXPPY^PPPbhTROSTQOSQSORVSQUQRWU", - nodeNames: "⚠ Template Tag TagOpen TagContent TagClose Text", - maxTerm: 10, + "!^QQOPOOOOOO'#C_'#C_OYOQO'#C^OOOO'#Cd'#CdQQOPOOOOOO'#Ce'#CeObOQO,58xOOOO-E6b-E6bOOOO-E6c-E6cOOOO1G.d1G.d", + stateData: "m~OVROZPO~OSTOTTO~OSTOTTOUXO~O", + goto: "oYPPZ_PPPPciTROSTQOSQSORVSQUQRWU", + nodeNames: "⚠ Template Tag TagOpen TagContent TagString TagClose Text", + maxTerm: 11, propSources: [highlight], skippedNodes: [0], repeatNodeCount: 2, tokenData: - "#{~RTOtbtu!zu;'Sb;'S;=`!o<%lOb~gTU~Otbtuvu;'Sb;'S;=`!o<%lOb~yVO#ob#o#p!`#p;'Sb;'S;=`!o<%l~b~Ob~~!u~!cSO!}b#O;'Sb;'S;=`!o<%lOb~!rP;=`<%lb~!zOU~~!}VO#ob#o#p#d#p;'Sb;'S;=`!o<%l~b~Ob~~!u~#gTO!}b!}#O#v#O;'Sb;'S;=`!o<%lOb~#{OY~", - tokenizers: [1, new LocalTokenGroup("b~RP#P#QU~XP#q#r[~aOT~~", 17, 4)], + "#{~RTOtbtu!zu;'Sb;'S;=`!o<%lOb~gTV~Otbtuvu;'Sb;'S;=`!o<%lOb~yVO#ob#o#p!`#p;'Sb;'S;=`!o<%l~b~Ob~~!u~!cSO!}b#O;'Sb;'S;=`!o<%lOb~!rP;=`<%lb~!zOV~~!}VO#ob#o#p#d#p;'Sb;'S;=`!o<%l~b~Ob~~!u~#gTO!}b!}#O#v#O;'Sb;'S;=`!o<%lOb~#{OZ~", + tokenizers: [ + 1, + new LocalTokenGroup( + "#R~RQwxX#P#Q!u~[VOwXwxqx#OX#O#Pv#P;'SX;'S;=`!o<%lOX~vOT~~yRO;'SX;'S;=`!S;=`OX~!VWOwXwxqx#OX#O#Pv#P;'SX;'S;=`!o;=`<%lX<%lOX~!rP;=`<%lX~!xP#q#r!{~#QOU~~", + 94, + 4, + ), + ], topRules: { Template: [0, 1] }, tokenPrec: 0, }); diff --git a/apps/yaak-client/lib/resolvedModelName.test.ts b/apps/yaak-client/lib/resolvedModelName.test.ts index a9e8ea12a..55acf6474 100644 --- a/apps/yaak-client/lib/resolvedModelName.test.ts +++ b/apps/yaak-client/lib/resolvedModelName.test.ts @@ -35,6 +35,24 @@ describe("resolvedModelName", () => { ).toEqual("fn(arg='my key')/a/other(b='x y')/b"); }); + test("keeps a quoted `]}` inside the tag", () => { + expect(resolvedModelName(httpRequest("${[ fn(arg='x]}y') ]}/users"))).toEqual( + "fn(arg='x]}y')/users", + ); + }); + + test("keeps an escaped quote inside the tag", () => { + expect(resolvedModelName(httpRequest("${[ fn(arg='it\\'s ]}') ]}/users"))).toEqual( + "fn(arg='it\\'s ]}')/users", + ); + }); + + test("closes at the first `]}` when a quote is left unterminated", () => { + expect(resolvedModelName(httpRequest("${[ fn(arg='oops ]}/users"))).toEqual( + "fn(arg='oops/users", + ); + }); + test("stops each tag at its first closing bracket", () => { expect(resolvedModelName(httpRequest("${[ a ]}${[ b ]}"))).toEqual("ab"); }); diff --git a/apps/yaak-client/lib/resolvedModelName.ts b/apps/yaak-client/lib/resolvedModelName.ts index fe2c23de7..1b7dcfcfd 100644 --- a/apps/yaak-client/lib/resolvedModelName.ts +++ b/apps/yaak-client/lib/resolvedModelName.ts @@ -1,13 +1,7 @@ import type { AnyModel } from "@yaakapp-internal/models"; import { foldersAtom } from "@yaakapp-internal/models"; import { jotaiStore } from "./jotai"; - -/** - * Matches a template tag up to the first `]}`, the same way the Lezer grammar and the JSON - * linter close a tag. Quoted arguments may contain spaces, so this can't stop at whitespace. - * The capture group holds the tag's inner content, trimmed of the padding spaces. - */ -const TEMPLATE_TAG_REGEX = /\$\{\[\s*([\s\S]*?)\s*]}/g; +import { replaceTemplateTags } from "./templateTags"; export function resolvedModelName(r: AnyModel | null): string { if (r == null) return ""; @@ -22,7 +16,7 @@ export function resolvedModelName(r: AnyModel | null): string { } // Replace variable syntax with variable name - const withoutVariables = r.url.replace(TEMPLATE_TAG_REGEX, "$1"); + const withoutVariables = replaceTemplateTags(r.url, (m) => m.inner.trim()); if (withoutVariables.trim() === "") { return r.model === "http_request" ? r.bodyType && r.bodyType === "graphql" diff --git a/apps/yaak-client/lib/templateTags.test.ts b/apps/yaak-client/lib/templateTags.test.ts new file mode 100644 index 000000000..876af2cf4 --- /dev/null +++ b/apps/yaak-client/lib/templateTags.test.ts @@ -0,0 +1,176 @@ +/* oxlint-disable no-template-curly-in-string */ + +import { describe, expect, test } from "vite-plus/test"; +import { findTemplateTags, replaceTemplateTags } from "./templateTags"; + +/** Seeded so a fuzz failure reproduces. */ +function random(seed: number): () => number { + let state = seed; + return () => { + state = (state + 0x6d2b79f5) | 0; + let t = Math.imul(state ^ (state >>> 15), 1 | state); + t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t; + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; +} + +function millis(run: () => void): number { + const started = performance.now(); + run(); + return performance.now() - started; +} + +describe("findTemplateTags", () => { + test("finds a simple tag", () => { + expect(findTemplateTags("${[ my_var ]}")).toEqual([{ start: 0, end: 13, inner: " my_var " }]); + }); + + test("finds a tag surrounded by text", () => { + const text = 'a="${[ my_var ]}"'; + expect(findTemplateTags(text)).toEqual([{ start: 3, end: 16, inner: " my_var " }]); + expect(text.slice(3, 16)).toEqual("${[ my_var ]}"); + }); + + test("finds an empty tag", () => { + expect(findTemplateTags("${[]}")).toEqual([{ start: 0, end: 5, inner: "" }]); + }); + + test("keeps a quoted `]}` inside the tag", () => { + const text = "${[ fn(a='x]}y') ]}"; + expect(findTemplateTags(text)).toEqual([ + { start: 0, end: text.length, inner: " fn(a='x]}y') " }, + ]); + }); + + test("keeps an escaped quote inside the string", () => { + const text = "${[ fn(a='it\\'s ]}') ]}"; + expect(findTemplateTags(text)).toEqual([ + { start: 0, end: text.length, inner: " fn(a='it\\'s ]}') " }, + ]); + }); + + test("keeps a b64 string in one tag", () => { + const text = "${[ b64'Zm9v' ]}"; + expect(findTemplateTags(text)).toEqual([{ start: 0, end: text.length, inner: " b64'Zm9v' " }]); + }); + + test("closes at the first `]}` when a quote is left unterminated", () => { + const text = "${[ fn(a='unterminated ]} rest"; + expect(findTemplateTags(text)).toEqual([{ start: 0, end: 25, inner: " fn(a='unterminated " }]); + expect(text.slice(0, 25)).toEqual("${[ fn(a='unterminated ]}"); + }); + + test("finds two tags with text between them", () => { + expect(findTemplateTags("one ${[ a ]} two ${[ b ]} three")).toEqual([ + { start: 4, end: 12, inner: " a " }, + { start: 17, end: 25, inner: " b " }, + ]); + }); + + test("finds two adjacent tags", () => { + expect(findTemplateTags("${[ a ]} ${[ b ]}")).toEqual([ + { start: 0, end: 8, inner: " a " }, + { start: 9, end: 17, inner: " b " }, + ]); + }); + + test("finds two tags that each quote a `]}`", () => { + const text = "${[ a(x='1]}') ]} and ${[ b(y='2]}') ]}"; + expect(findTemplateTags(text)).toEqual([ + { start: 0, end: 17, inner: " a(x='1]}') " }, + { start: 22, end: 39, inner: " b(y='2]}') " }, + ]); + }); + + test("finds nothing in an unclosed tag", () => { + expect(findTemplateTags("${[ no close")).toEqual([]); + }); + + test("finds nothing in text without tags", () => { + expect(findTemplateTags('{"a": "${b}", "c": 1}')).toEqual([]); + }); + + test("treats a nested `${[` as ordinary content", () => { + expect(findTemplateTags("${[ a ${[ b ]}")).toEqual([{ start: 0, end: 14, inner: " a ${[ b " }]); + }); + + test("finishes fast on a long run of quotes", () => { + const text = `\${[${"'".repeat(20_000)} ]}`; + let matches: unknown; + const elapsed = millis(() => { + matches = findTemplateTags(text); + }); + expect(matches).toEqual([{ start: 0, end: text.length, inner: `${"'".repeat(20_000)} ` }]); + expect(elapsed).toBeLessThan(200); + }); + + test("finishes fast on a long run of unpairable quotes", () => { + // Every quote is escaped, so none of them opens a string that closes + const text = `\${[${"\\'".repeat(20_000)} ]}`; + let matches: unknown; + const elapsed = millis(() => { + matches = findTemplateTags(text); + }); + expect(matches).toHaveLength(1); + expect(elapsed).toBeLessThan(200); + }); + + test("finishes fast on a long run of openers that never close", () => { + const text = "${[".repeat(20_000); + let matches: unknown; + const elapsed = millis(() => { + matches = findTemplateTags(text); + }); + expect(matches).toEqual([]); + expect(elapsed).toBeLessThan(200); + }); + + test("never throws, and always reports real bounds", () => { + const alphabet = ["$", "{", "[", "]", "}", "'", "\\", "a", "b"]; + const next = random(0x5eed); + + for (let n = 0; n < 2_000; n++) { + let text = ""; + const length = Math.floor(next() * 31); + for (let i = 0; i < length; i++) { + text += alphabet[Math.floor(next() * alphabet.length)]; + } + + let matches: ReturnType = []; + expect(() => { + matches = findTemplateTags(text); + }, text).not.toThrow(); + + let previousEnd = 0; + for (const match of matches) { + expect(text.slice(match.start, match.start + 3), text).toEqual("${["); + expect(text.slice(match.end - 2, match.end), text).toEqual("]}"); + expect(text.slice(match.start + 3, match.end - 2), text).toEqual(match.inner); + expect(match.start, text).toBeGreaterThanOrEqual(previousEnd); + previousEnd = match.end; + } + } + }); +}); + +describe("replaceTemplateTags", () => { + test("leaves text without tags alone", () => { + expect(replaceTemplateTags('{"a": 1}', () => "x")).toEqual('{"a": 1}'); + }); + + test("replaces each tag with its trimmed contents", () => { + expect(replaceTemplateTags("${[ a ]}/x/${[ b ]}", (m) => m.inner.trim())).toEqual("a/x/b"); + }); + + test("keeps the replaced length when padding", () => { + const tag = "${[ fn(arg='x]}y') ]}"; + const text = `{"a": "${tag}"}`; + const replaced = replaceTemplateTags(text, (m) => "1".repeat(m.end - m.start)); + expect(replaced).toEqual(`{"a": "${"1".repeat(tag.length)}"}`); + expect(replaced).toHaveLength(text.length); + }); + + test("leaves an unclosed tag in place", () => { + expect(replaceTemplateTags("${[ oops", () => "x")).toEqual("${[ oops"); + }); +}); diff --git a/apps/yaak-client/lib/templateTags.ts b/apps/yaak-client/lib/templateTags.ts new file mode 100644 index 000000000..b832321fc --- /dev/null +++ b/apps/yaak-client/lib/templateTags.ts @@ -0,0 +1,137 @@ +/* + * Finding template tags in text. + * + * A tag closes at `]}`, but a quoted argument is allowed to contain `]}` of its own — the + * printer in `crates/yaak-templates` emits `${[ fn(a='x]}y') ]}` verbatim — so the scan has to + * step over quoted strings. It mirrors the Lezer grammar in + * `components/core/Editor/twig/twig.grammar`: a terminated `'…'` (where `\` escapes whatever + * follows it) is consumed whole, and an unterminated quote is just another character, so a + * half-typed tag still closes at the next `]}` instead of swallowing the rest of the document. + * + * This was a regex until CodeQL flagged it (js/polynomial-redos): matching an unterminated + * quote needed a negative lookahead that rescanned to the end of the input at every unpaired + * quote, which is quadratic on text an attacker picks. The scan below is linear instead — see + * the notes on `closingQuoteTable` and on the dead-end set in `findTemplateTags` for why. + */ + +const QUOTE = 0x27; // ' +const BACKSLASH = 0x5c; // \ +const CLOSE_BRACKET = 0x5d; // ] +const CLOSE_BRACE = 0x7d; // } + +/** The `${[`…`]}` bounds of one tag, and what sits between them. */ +export interface TemplateTagMatch { + /** Index of the `$` that opens the tag. */ + start: number; + /** Index just past the closing `]}`. */ + end: number; + /** Everything between `${[` and `]}`, not trimmed. */ + inner: string; +} + +/** + * For every index `i`, the index of the `'` that closes a string whose body starts at `i`, or + * `-1` when nothing does. + * + * Built right to left in a single pass, which is the trick that replaces the regex lookahead: + * whether a quote opens a terminated string depends only on the text after it, so the answer + * can be computed once for every position instead of re-derived at each quote. Skipping a + * string of any length then costs one lookup. + */ +function closingQuoteTable(text: string): Int32Array { + // Two extra slots so a `\` in the last position can read past the end and find `-1` + const table = new Int32Array(text.length + 2).fill(-1); + for (let i = text.length - 1; i >= 0; i--) { + const c = text.charCodeAt(i); + // A backslash escapes whatever follows it, including a quote + const next = c === BACKSLASH ? table[i + 2] : table[i + 1]; + table[i] = c === QUOTE ? i : (next ?? -1); + } + return table; +} + +/** + * Finds every template tag in `text`, in order and without overlaps. + * + * A `${[` with no `]}` after it isn't a tag, and neither is a `${[` nested inside a tag body — + * both are ordinary content, same as the regex this replaced treated them. + */ +export function findTemplateTags(text: string): TemplateTagMatch[] { + const matches: TemplateTagMatch[] = []; + + // Both built only once a scan actually needs them, so short values allocate nothing + let closingQuotes: Int32Array | null = null; + + /* + * Body positions a previous scan already ran off the end of the text from. Where a scan ends + * up depends only on where it starts, so a scan that walks into one of these would repeat + * that same walk and fail the same way, and can stop right there. + * + * That bound is what keeps the whole pass linear: every position enters this set at most + * once, and every step of every scan either visits a position for the first time or ends + * that scan. + */ + let deadEnds: Set | null = null; + + let searchFrom = 0; + while (searchFrom < text.length) { + const start = text.indexOf("${[", searchFrom); + if (start < 0) break; + + const visited: number[] = []; + let end = -1; + let i = start + 3; + while (i < text.length) { + if (deadEnds?.has(i)) break; + visited.push(i); + + const c = text.charCodeAt(i); + if (c === QUOTE) { + closingQuotes ??= closingQuoteTable(text); + const close = closingQuotes[i + 1] ?? -1; + // An unterminated quote is an ordinary character, so only step over a closed string + i = close < 0 ? i + 1 : close + 1; + continue; + } + + if (c === CLOSE_BRACKET && text.charCodeAt(i + 1) === CLOSE_BRACE) { + end = i + 2; + break; + } + + i += 1; + } + + if (end < 0) { + // Not a tag. Resume past the opener: a real tag may start inside a string this scan + // stepped over. + deadEnds ??= new Set(); + for (const position of visited) deadEnds.add(position); + searchFrom = start + 3; + continue; + } + + matches.push({ start, end, inner: text.slice(start + 3, end - 2) }); + searchFrom = end; + } + + return matches; +} + +/** Rewrites every template tag in `text` with whatever `replace` returns for it. */ +export function replaceTemplateTags( + text: string, + replace: (match: TemplateTagMatch) => string, +): string { + const matches = findTemplateTags(text); + if (matches.length === 0) return text; + + let result = ""; + let copiedTo = 0; + for (const match of matches) { + result += text.slice(copiedTo, match.start) + replace(match); + copiedTo = match.end; + } + + return result + text.slice(copiedTo); +} diff --git a/apps/yaak-client/lib/validateHttpHeader.test.ts b/apps/yaak-client/lib/validateHttpHeader.test.ts index 365592527..8bfc578dd 100644 --- a/apps/yaak-client/lib/validateHttpHeader.test.ts +++ b/apps/yaak-client/lib/validateHttpHeader.test.ts @@ -26,6 +26,18 @@ describe("validateHttpHeader", () => { expect(validateHttpHeader("${[ fn(arg='my key') ]}-${[ other(b='x y') ]}")).toEqual(true); }); + test("allows a tag whose quoted argument contains `]}`", () => { + expect(validateHttpHeader("${[ fn(arg='x]}y') ]}")).toEqual(true); + }); + + test("allows a tag whose quoted argument contains an escaped quote", () => { + expect(validateHttpHeader("${[ fn(arg='it\\'s ]}') ]}")).toEqual(true); + }); + + test("rejects a tag with a quoted `]}` followed by an invalid character", () => { + expect(validateHttpHeader("${[ fn(arg='x]}y') ]} oops")).toEqual(false); + }); + test("rejects a tag followed by an invalid character", () => { expect(validateHttpHeader("${[ my_var ]} oops")).toEqual(false); }); diff --git a/apps/yaak-client/lib/validateHttpHeader.ts b/apps/yaak-client/lib/validateHttpHeader.ts index 1441f9244..beb3cdd1f 100644 --- a/apps/yaak-client/lib/validateHttpHeader.ts +++ b/apps/yaak-client/lib/validateHttpHeader.ts @@ -1,8 +1,4 @@ -/** - * Matches a template tag up to the first `]}`, the same way the Lezer grammar and the JSON - * linter close a tag. Quoted arguments may contain spaces, so this can't stop at whitespace. - */ -const TEMPLATE_TAG_REGEX = /\$\{\[[\s\S]*?]}/g; +import { replaceTemplateTags } from "./templateTags"; export function validateHttpHeader(v: string): boolean { if (v === "") { @@ -10,6 +6,6 @@ export function validateHttpHeader(v: string): boolean { } // Template strings are not allowed so we replace them with a valid example string - const withoutTemplateStrings = v.replace(TEMPLATE_TAG_REGEX, "123"); + const withoutTemplateStrings = replaceTemplateTags(v, () => "123"); return withoutTemplateStrings.match(/^[a-zA-Z0-9-_]+$/) !== null; } diff --git a/crates/yaak-templates/src/format_json.rs b/crates/yaak-templates/src/format_json.rs index 98f1a90e0..8d6b3b7a9 100644 --- a/crates/yaak-templates/src/format_json.rs +++ b/crates/yaak-templates/src/format_json.rs @@ -1,3 +1,5 @@ +use crate::tag_scan::TagStrings; + enum FormatState { TemplateTag, String, @@ -7,6 +9,7 @@ enum FormatState { /// Formats JSON that might contain template tags (skipped entirely) pub fn format_json(text: &str, tab: &str) -> String { let mut chars = text.chars().peekable(); + let mut tag_strings = TagStrings::default(); let mut new_json = "".to_string(); let mut depth = 0; @@ -43,15 +46,22 @@ pub fn format_json(text: &str, tab: &str) -> String { } // Close Template tag states if let FormatState::TemplateTag = state { + // A quoted argument is allowed to contain `]}`, so skip over strings whole + if current_char == '\'' { + new_json.push(current_char); + if let Some(rest) = tag_strings.take(&mut chars) { + new_json.push_str(&rest); + } + continue; + } if rest_of_chars.take(2).collect::() == "]}" { state = FormatState::None; new_json.push_str("]}"); chars.next(); // Skip the second closing bracket continue; - } else { - new_json.push(current_char); - continue; } + new_json.push(current_char); + continue; } if rest_of_chars.take(3).collect::() == "${[" { @@ -315,6 +325,33 @@ mod tests { ); } + #[test] + fn test_skip_template_tags_with_quoted_brackets() { + // The `]}` inside the quotes must not end the tag, or the rest gets re-indented + assert_eq!( + format_json(r#"{"foo":${[ fn(a='x]}y') ]} }"#, " "), + r#" +{ + "foo": ${[ fn(a='x]}y') ]} +} +"# + .trim() + ); + } + + #[test] + fn test_skip_template_tags_with_escaped_quote() { + assert_eq!( + format_json(r#"{"foo":${[ fn(a='it\'s ]}') ]} }"#, " "), + r#" +{ + "foo": ${[ fn(a='it\'s ]}') ]} +} +"# + .trim() + ); + } + #[test] fn test_graphql_response() { assert_eq!( diff --git a/crates/yaak-templates/src/lib.rs b/crates/yaak-templates/src/lib.rs index d97dfb29b..73404b192 100644 --- a/crates/yaak-templates/src/lib.rs +++ b/crates/yaak-templates/src/lib.rs @@ -4,6 +4,7 @@ pub mod format_json; pub mod parser; pub mod renderer; pub mod strip_json_comments; +mod tag_scan; #[cfg(feature = "wasm")] pub mod wasm; diff --git a/crates/yaak-templates/src/parser.rs b/crates/yaak-templates/src/parser.rs index 12230fc33..950b70a11 100644 --- a/crates/yaak-templates/src/parser.rs +++ b/crates/yaak-templates/src/parser.rs @@ -46,12 +46,11 @@ pub enum Val { } /// Strings are printed as plain single-quoted values whenever they can survive a round-trip -/// through both the parser and the client's editor. The editor can't handle `]}` inside a tag -/// (the Lezer grammar and the JSON linter both close a tag at the first one, regardless of -/// quoting), and newlines or other control characters don't belong in a single-line tag, so -/// those cases fall back to base64. +/// through both the parser and the client's editor. A quoted `]}` is fine — the editor's +/// grammar and regexes skip over quoted strings — but newlines and other control characters +/// don't belong in a single-line tag, so those fall back to base64. fn requires_b64(text: &str) -> bool { - text.contains("]}") || text.chars().any(char::is_control) + text.chars().any(char::is_control) } /// Escape the only two characters the parser treats specially inside a single-quoted string. @@ -872,14 +871,14 @@ mod tests { assert_eq!(Val::Str { text: r#"{"a":1}"#.to_string() }.to_string(), r#"'{"a":1}'"#); assert_eq!(Val::Str { text: "héllo".to_string() }.to_string(), "'héllo'"); + // Quoting is enough to keep `]}` from closing the tag early in the client editor + assert_eq!(Val::Str { text: "a]}b".to_string() }.to_string(), "'a]}b'"); + Ok(()) } #[test] fn token_display_str_b64_fallback() -> Result<()> { - // `]}` would close the tag early in the client editor - assert_eq!(Val::Str { text: "a]}b".to_string() }.to_string(), "b64'YV19Yg'"); - // Control characters don't belong in a single-line tag assert_eq!(Val::Str { text: "line\nbreak".to_string() }.to_string(), "b64'bGluZQpicmVhaw'"); assert_eq!(Val::Str { text: "line\rbreak".to_string() }.to_string(), "b64'bGluZQ1icmVhaw'"); diff --git a/crates/yaak-templates/src/strip_json_comments.rs b/crates/yaak-templates/src/strip_json_comments.rs index 5941f10e5..18ebb48ff 100644 --- a/crates/yaak-templates/src/strip_json_comments.rs +++ b/crates/yaak-templates/src/strip_json_comments.rs @@ -1,3 +1,5 @@ +use crate::tag_scan::TagStrings; + /// Strips JSON comments only if the result is valid JSON. If stripping comments /// produces invalid JSON, the original text is returned unchanged. pub fn maybe_strip_json_comments(text: &str) -> String { @@ -17,6 +19,7 @@ pub fn maybe_strip_json_comments(text: &str) -> String { /// - Comments inside strings and template tags are left alone pub fn strip_json_comments(text: &str) -> String { let mut chars = text.chars().peekable(); + let mut tag_strings = TagStrings::default(); let mut result = String::with_capacity(text.len()); let mut in_string = false; let mut in_template_tag = false; @@ -45,6 +48,13 @@ pub fn strip_json_comments(text: &str) -> String { // Handle template tags if in_template_tag { result.push(current_char); + // A quoted argument is allowed to contain `]}`, so skip over strings whole + if current_char == '\'' { + if let Some(rest) = tag_strings.take(&mut chars) { + result.push_str(&rest); + } + continue; + } if current_char == ']' && chars.peek() == Some(&'}') { result.push(chars.next().unwrap()); in_template_tag = false; @@ -280,6 +290,34 @@ mod tests { assert_eq!(strip_json_comments(input), input); } + #[test] + fn test_quoted_tag_close_inside_template_tag() { + // The `]}` is quoted, so the tag runs to the real close and the `//` stays put + let input = r#"{ + "foo": ${[ fn(a='x]}y // not a comment') ]} +}"#; + assert_eq!(strip_json_comments(input), input); + } + + #[test] + fn test_escaped_quote_inside_template_tag() { + let input = r#"{ + "foo": ${[ fn(a='it\'s ]} // still fine') ]} +}"#; + assert_eq!(strip_json_comments(input), input); + } + + #[test] + fn test_unterminated_quote_closes_tag_at_first_bracket() { + // A half-typed string must not swallow the rest of the document + assert_eq!( + strip_json_comments(r#"{"foo": ${[ fn(a='oops ]}, "bar": 1 // gone +}"#), + r#"{"foo": ${[ fn(a='oops ]}, "bar": 1 +}"# + ); + } + #[test] fn test_multiple_comments() { assert_eq!( diff --git a/crates/yaak-templates/src/tag_scan.rs b/crates/yaak-templates/src/tag_scan.rs new file mode 100644 index 000000000..bc622a3fb --- /dev/null +++ b/crates/yaak-templates/src/tag_scan.rs @@ -0,0 +1,114 @@ +use std::iter::Peekable; + +/// Reads the single-quoted strings inside template tags, moving left to right through one text. +/// +/// Kept as a struct rather than a free function because it remembers one thing between calls; +/// see [`TagStrings::take`]. +#[derive(Default)] +pub(crate) struct TagStrings { + /// Set once a scan has run off the end of the text, after which no string can close. + none_left: bool, +} + +impl TagStrings { + /// Consumes the body of a single-quoted string, assuming the opening quote has already been + /// read, and returns it (including the closing quote). + /// + /// Returns `None` when the quote is never closed, leaving the iterator untouched. The parser + /// and the editor's Lezer grammar both only recognize a terminated `'…'`, so an unterminated + /// quote has to stay an ordinary character and let the tag close at the next `]}`. + /// + /// Scanning forward from every quote would be quadratic on text full of unpaired quotes, so + /// a failed scan is remembered: once one runs off the end, no later quote can open a + /// terminated string either. Any unescaped quote after it would have closed the failed scan, + /// so every remaining quote must be one the failed scan consumed as an escaped character — + /// and a scan starting there continues from the same place, in the same state, to the same + /// end. With that, every character of the text is read a bounded number of times. + pub(crate) fn take(&mut self, chars: &mut Peekable) -> Option + where + I: Iterator + Clone, + { + if self.none_left { + return None; + } + + let mut lookahead = chars.clone(); + let mut consumed = String::new(); + while let Some(c) = lookahead.next() { + consumed.push(c); + match c { + // A backslash escapes whatever follows it, including a quote + '\\' => match lookahead.next() { + Some(escaped) => consumed.push(escaped), + None => break, + }, + '\'' => { + *chars = lookahead; + return Some(consumed); + } + _ => {} + } + } + + self.none_left = true; + None + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::time::{Duration, Instant}; + + fn take(text: &str) -> (Option, String) { + let mut chars = text.chars().peekable(); + let taken = TagStrings::default().take(&mut chars); + (taken, chars.collect()) + } + + #[test] + fn takes_a_terminated_string() { + assert_eq!(take("abc' rest"), (Some("abc'".to_string()), " rest".to_string())); + } + + #[test] + fn takes_a_string_containing_a_tag_close() { + assert_eq!(take("a]}b') ]}"), (Some("a]}b'".to_string()), ") ]}".to_string())); + } + + #[test] + fn takes_a_string_with_an_escaped_quote() { + assert_eq!(take(r"it\'s' rest"), (Some(r"it\'s'".to_string()), " rest".to_string())); + } + + #[test] + fn leaves_an_unterminated_string_alone() { + assert_eq!(take("oops ]}"), (None, "oops ]}".to_string())); + } + + #[test] + fn leaves_a_trailing_backslash_alone() { + assert_eq!(take(r"oops\"), (None, r"oops\".to_string())); + } + + #[test] + fn stays_linear_on_many_unpaired_quotes() { + // Every quote here is escaped, so none of them can ever close a string. Rescanning to + // the end from each one would be quadratic, so this should finish in no time. + let text = r"\'".repeat(20_000); + let mut strings = TagStrings::default(); + let mut chars = text.chars().peekable(); + let started = Instant::now(); + + let mut quotes = 0; + while let Some(c) = chars.next() { + if c == '\'' { + quotes += 1; + assert_eq!(strings.take(&mut chars), None); + } + } + + assert_eq!(quotes, 20_000); + assert!(started.elapsed() < Duration::from_secs(1), "took {:?}", started.elapsed()); + } +} diff --git a/plugins/template-function-request/src/index.ts b/plugins/template-function-request/src/index.ts index 51de19012..a602b5249 100755 --- a/plugins/template-function-request/src/index.ts +++ b/plugins/template-function-request/src/index.ts @@ -252,6 +252,85 @@ export const plugin: PluginDefinition = { ], }; +const QUOTE = 0x27; // ' +const BACKSLASH = 0x5c; // \ +const CLOSE_BRACKET = 0x5d; // ] +const CLOSE_BRACE = 0x7d; // } + +/** + * For every index `i`, the index of the `'` that closes a string whose body starts at `i`, or + * `-1` when nothing does. One right-to-left pass, so skipping a string costs one lookup. + */ +function closingQuoteTable(text: string): Int32Array { + // Two extra slots so a `\` in the last position can read past the end and find `-1` + const table = new Int32Array(text.length + 2).fill(-1); + for (let i = text.length - 1; i >= 0; i--) { + const c = text.charCodeAt(i); + // A backslash escapes whatever follows it, including a quote + const next = c === BACKSLASH ? table[i + 2] : table[i + 1]; + table[i] = c === QUOTE ? i : (next ?? -1); + } + return table; +} + +/** + * Rewrites every template tag in `text` with whatever `replace` returns for its inner content. + * + * A quoted argument may contain `]}`, so the scan steps over strings instead of stopping at the + * first one, and an unterminated quote stays an ordinary character. Kept in sync by hand with + * `apps/yaak-client/lib/templateTags.ts`, which carries the full explanation — including why + * this is a linear hand-written scan rather than a regex. + */ +function replaceTemplateTags(text: string, replace: (inner: string) => string): string { + let closingQuotes: Int32Array | null = null; + let deadEnds: Set | null = null; + + let result = ""; + let copiedTo = 0; + let searchFrom = 0; + while (searchFrom < text.length) { + const start = text.indexOf("${[", searchFrom); + if (start < 0) break; + + const visited: number[] = []; + let end = -1; + let i = start + 3; + while (i < text.length) { + if (deadEnds?.has(i)) break; + visited.push(i); + + const c = text.charCodeAt(i); + if (c === QUOTE) { + closingQuotes ??= closingQuoteTable(text); + const close = closingQuotes[i + 1] ?? -1; + i = close < 0 ? i + 1 : close + 1; + continue; + } + + if (c === CLOSE_BRACKET && text.charCodeAt(i + 1) === CLOSE_BRACE) { + end = i + 2; + break; + } + + i += 1; + } + + if (end < 0) { + // Not a tag, but a real one may start inside a string this scan stepped over + deadEnds ??= new Set(); + for (const position of visited) deadEnds.add(position); + searchFrom = start + 3; + continue; + } + + result += text.slice(copiedTo, start) + replace(text.slice(start + 3, end - 2)); + copiedTo = end; + searchFrom = end; + } + + return result + text.slice(copiedTo); +} + // TODO: Use a common function for this, but it fails to build on windows during CI if I try importing it here export function resolvedModelName(r: AnyModel | null): string { if (r == null) return ""; @@ -266,7 +345,7 @@ export function resolvedModelName(r: AnyModel | null): string { } // Replace variable syntax with variable name - const withoutVariables = r.url.replace(/\$\{\[\s*([^\]\s]+)\s*]}/g, "$1"); + const withoutVariables = replaceTemplateTags(r.url, (inner) => inner.trim()); if (withoutVariables.trim() === "") { return r.model === "http_request" ? r.bodyType && r.bodyType === "graphql"