Skip to content

Commit adab365

Browse files
authored
fix: prevent VALUE/MVALUE self-recursion
Fixes #920 Prevents the formatter from hanging when user input contains `{{value}}` / `{{mvalue}}` by making VALUE/MVALUE replacement non-recursive and single-pass. - Adds regression tests for self-referential VALUE/MVALUE input
1 parent 41744eb commit adab365

2 files changed

Lines changed: 144 additions & 10 deletions

File tree

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { describe, it, expect, beforeEach } from "vitest";
2+
import { Formatter } from "./formatter";
3+
4+
// Regression tests for issue #920:
5+
// - Entering `{{value}}` (or text containing it) into the VALUE prompt caused an infinite loop.
6+
// - Entering `{{mvalue}}` into the math modal caused repeated prompting / non-termination.
7+
class Issue920TestFormatter extends Formatter {
8+
private valueResponse = "";
9+
private mathResponse = "";
10+
11+
constructor() {
12+
super();
13+
}
14+
15+
public setValueResponse(value: string): void {
16+
this.valueResponse = value;
17+
}
18+
19+
public setMathResponse(value: string): void {
20+
this.mathResponse = value;
21+
}
22+
23+
protected async format(input: string): Promise<string> {
24+
let output = input;
25+
output = await this.replaceValueInString(output);
26+
output = await this.replaceMathValueInString(output);
27+
return output;
28+
}
29+
30+
protected promptForValue(): string {
31+
return this.valueResponse;
32+
}
33+
34+
protected getCurrentFileLink(): string | null {
35+
return null;
36+
}
37+
38+
protected getCurrentFileName(): string | null {
39+
return null;
40+
}
41+
42+
protected getVariableValue(variableName: string): string {
43+
return (this.variables.get(variableName) as string) ?? "";
44+
}
45+
46+
protected suggestForValue(): string {
47+
return "";
48+
}
49+
50+
protected suggestForField(): Promise<string> {
51+
return Promise.resolve("");
52+
}
53+
54+
protected promptForMathValue(): Promise<string> {
55+
return Promise.resolve(this.mathResponse);
56+
}
57+
58+
protected getMacroValue(): string {
59+
return "";
60+
}
61+
62+
protected promptForVariable(): Promise<string> {
63+
return Promise.resolve("");
64+
}
65+
66+
protected getTemplateContent(): Promise<string> {
67+
return Promise.resolve("");
68+
}
69+
70+
protected getSelectedText(): Promise<string> {
71+
return Promise.resolve("");
72+
}
73+
74+
protected getClipboardContent(): Promise<string> {
75+
return Promise.resolve("");
76+
}
77+
78+
protected isTemplatePropertyTypesEnabled(): boolean {
79+
return false;
80+
}
81+
82+
public async testFormat(input: string): Promise<string> {
83+
return await this.format(input);
84+
}
85+
}
86+
87+
describe("Issue #920: VALUE/MVALUE self-references should not hang", () => {
88+
let formatter: Issue920TestFormatter;
89+
90+
beforeEach(() => {
91+
formatter = new Issue920TestFormatter();
92+
});
93+
94+
it("treats {{VALUE}} returned from the VALUE prompt as literal (no recursion)", async () => {
95+
formatter.setValueResponse("{{VALUE}}");
96+
const result = await formatter.testFormat("Start {{VALUE}} End");
97+
expect(result).toBe("Start {{VALUE}} End");
98+
});
99+
100+
it("does not recursively expand {{VALUE}} inside user-provided VALUE input", async () => {
101+
formatter.setValueResponse("prefix {{VALUE}}");
102+
const result = await formatter.testFormat("A {{VALUE}} B {{VALUE}} C");
103+
expect(result).toBe("A prefix {{VALUE}} B prefix {{VALUE}} C");
104+
});
105+
106+
it("treats {{MVALUE}} returned from the math prompt as literal (no recursion)", async () => {
107+
formatter.setMathResponse("{{MVALUE}}");
108+
const result = await formatter.testFormat("Start {{MVALUE}} End");
109+
expect(result).toBe("Start {{MVALUE}} End");
110+
});
111+
112+
it("does not recursively expand {{MVALUE}} inside user-provided math input", async () => {
113+
formatter.setMathResponse("prefix {{MVALUE}}");
114+
const result = await formatter.testFormat("A {{MVALUE}} B");
115+
expect(result).toBe("A prefix {{MVALUE}} B");
116+
});
117+
});
118+

src/formatters/formatter.ts

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -130,16 +130,24 @@ export abstract class Formatter {
130130
protected async replaceValueInString(input: string): Promise<string> {
131131
let output: string = input;
132132

133-
if (this.variables.has("value")) {
134-
this.value = this.variables.get("value") as string;
135-
}
133+
// Fast path: nothing to do.
134+
if (!NAME_VALUE_REGEX.test(output)) return output;
136135

137-
while (NAME_VALUE_REGEX.test(output)) {
138-
if (!this.value) this.value = await this.promptForValue();
136+
// Preserve programmatic VALUE injection via reserved variable name `value`.
137+
if (this.hasConcreteVariable("value")) {
138+
this.value = String(this.variables.get("value"));
139+
}
139140

140-
output = this.replacer(output, NAME_VALUE_REGEX, this.value);
141+
// Prompt only once per formatter run (empty string is a valid value).
142+
if (this.value === undefined) {
143+
this.value = await this.promptForValue();
141144
}
142145

146+
// Replace all occurrences in a single non-recursive pass.
147+
// Important: use a replacer function so `$` in user input is treated literally.
148+
const regex = new RegExp(NAME_VALUE_REGEX.source, "gi");
149+
output = output.replace(regex, () => this.value);
150+
143151
return output;
144152
}
145153

@@ -358,13 +366,21 @@ export abstract class Formatter {
358366
protected abstract promptForMathValue(): Promise<string>;
359367

360368
protected async replaceMathValueInString(input: string) {
361-
let output: string = input;
369+
// Build the output by scanning the current input once.
370+
// This avoids infinite replacement loops when the provided math input contains {{MVALUE}}.
371+
const regex = new RegExp(MATH_VALUE_REGEX.source, "gi");
372+
373+
let output = "";
374+
let lastIndex = 0;
375+
let match: RegExpExecArray | null;
362376

363-
while (MATH_VALUE_REGEX.test(output)) {
364-
const mathstr = await this.promptForMathValue();
365-
output = this.replacer(output, MATH_VALUE_REGEX, mathstr);
377+
while ((match = regex.exec(input)) !== null) {
378+
output += input.slice(lastIndex, match.index);
379+
output += await this.promptForMathValue();
380+
lastIndex = match.index + match[0].length;
366381
}
367382

383+
output += input.slice(lastIndex);
368384
return output;
369385
}
370386

0 commit comments

Comments
 (0)