Unverified 提交 23b45a9b authored 作者: Will Chen's avatar Will Chen 提交者: GitHub

Fix HTML attribute and content escaping for dyad-tags (#2266)

When serializing dyad-tags, special characters (& < > ") are escaped. When deserializing/parsing, these need to be unescaped to get the original values. This fixes issues where escaped characters like &lt; would appear literally in the UI or processed values. Changes: - Add shared/xmlEscape.ts with escape/unescape utilities - Update DyadMarkdownParser.tsx to unescape attributes and content - Update dyad_tag_parser.ts to unescape all parsed values - Consolidate duplicate escape functions to use shared utilities - Add comprehensive tests for the escape/unescape roundtrip <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Fixes incorrect literal XML entities by unescaping dyad-tag attributes and inner content during parse, and centralizes escape helpers. > > - **Shared utils**: Add `shared/xmlEscape.ts` with `escapeXmlAttr|Content` and `unescapeXmlAttr|Content`; remove duplicate implementations and re-export in local agent tools > - **Parsing/rendering**: Update `DyadMarkdownParser` and `dyad_tag_parser` to unescape attributes/content for all relevant tags (write/rename/delete/search-replace/execute-sql/chat-summary/command) > - **Problem reports**: Generate `<dyad-problem-report>` using `escapeXmlAttr` for attributes and `escapeXmlContent` for messages > - **Local agent**: Use shared escaping in `xml_tool_translator` and related tooling > - **Tests/E2E**: Add comprehensive `xmlEscape` unit tests; update grep snapshots to expect raw JSX/HTML characters > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 0e1be9ad5d5cf0300cc1cf5bd361b1fd44f61ad7. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Fixes XML escaping for dyad-tags by unescaping attributes and content during parsing so original values render and process correctly. Prevents literal entities like &lt; showing in the UI and tools. - **Bug Fixes** - Unescape tag attributes and inner content in DyadMarkdownParser and dyad_tag_parser. - Correct attribute and content escaping in dyad-problem-report generation. - Add roundtrip tests for attribute and content escape/unescape. - Update local agent grep E2E snapshots to expect raw JSX/HTML characters. - **Refactors** - Introduce shared/xmlEscape with escape/unescape helpers for attributes and content. - Remove duplicate implementations and update IPC/local agent callers to use shared utilities. <sup>Written for commit 0e1be9ad5d5cf0300cc1cf5bd361b1fd44f61ad7. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: 's avatarClaude <noreply@anthropic.com> Co-authored-by: 's avatargithub-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
上级 71812e65
...@@ -5,4 +5,4 @@ ...@@ -5,4 +5,4 @@
- button "Copy": - button "Copy":
- img - img
- text: log - text: log
- code: "src/main.tsx:1: import { createRoot } from \"react-dom/client\"; src/main.tsx:4: createRoot(document.getElementById(\"root\")!).render(&lt;App /&gt;);" - code: "src/main.tsx:1: import { createRoot } from \"react-dom/client\"; src/main.tsx:4: createRoot(document.getElementById(\"root\")!).render(<App />);"
\ No newline at end of file \ No newline at end of file
...@@ -5,4 +5,4 @@ ...@@ -5,4 +5,4 @@
- button "Copy": - button "Copy":
- img - img
- text: log - text: log
- code: "src/main.tsx:2: import App from \"./App.tsx\"; src/main.tsx:4: createRoot(document.getElementById(\"root\")!).render(&lt;App /&gt;); src/App.tsx:1: const App = () =&gt; &lt;div&gt;Minimal imported app&lt;/div&gt;; src/App.tsx:3: export default App;" - code: "src/main.tsx:2: import App from \"./App.tsx\"; src/main.tsx:4: createRoot(document.getElementById(\"root\")!).render(<App />); src/App.tsx:1: const App = () => <div>Minimal imported app</div>; src/App.tsx:3: export default App;"
\ No newline at end of file \ No newline at end of file
/**
* XML escape/unescape utilities for dyad tags.
*
* When serializing dyad tags, we escape special characters to prevent
* breaking the tag structure. When deserializing (parsing), we need
* to unescape these characters to get the original values.
*/
/**
* Escapes special characters in XML attribute values.
* Handles: & " < >
*/
export function escapeXmlAttr(str: string): string {
return str
.replace(/&/g, "&amp;")
.replace(/"/g, "&quot;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;");
}
/**
* Unescapes XML attribute values.
* Note: Order matters - &amp; must be unescaped last to avoid double-unescaping.
*/
export function unescapeXmlAttr(str: string): string {
return str
.replace(/&lt;/g, "<")
.replace(/&gt;/g, ">")
.replace(/&quot;/g, '"')
.replace(/&amp;/g, "&");
}
/**
* Escapes special characters in XML content (text between tags).
* Handles: & < >
*/
export function escapeXmlContent(str: string): string {
return str.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
}
/**
* Unescapes XML content values.
* Note: Order matters - &amp; must be unescaped last to avoid double-unescaping.
*/
export function unescapeXmlContent(str: string): string {
return str.replace(/&lt;/g, "<").replace(/&gt;/g, ">").replace(/&amp;/g, "&");
}
import { describe, it, expect } from "vitest";
import {
escapeXmlAttr,
unescapeXmlAttr,
escapeXmlContent,
unescapeXmlContent,
} from "../../shared/xmlEscape";
describe("xmlEscape", () => {
describe("escapeXmlAttr", () => {
it("should escape ampersands", () => {
expect(escapeXmlAttr("foo & bar")).toBe("foo &amp; bar");
});
it("should escape double quotes", () => {
expect(escapeXmlAttr('foo "bar" baz')).toBe("foo &quot;bar&quot; baz");
});
it("should escape angle brackets", () => {
expect(escapeXmlAttr("foo <bar> baz")).toBe("foo &lt;bar&gt; baz");
});
it("should escape all special characters together", () => {
expect(escapeXmlAttr('use <a> and "b" & c')).toBe(
"use &lt;a&gt; and &quot;b&quot; &amp; c",
);
});
it("should handle empty strings", () => {
expect(escapeXmlAttr("")).toBe("");
});
it("should not modify strings without special characters", () => {
expect(escapeXmlAttr("hello world")).toBe("hello world");
});
});
describe("unescapeXmlAttr", () => {
it("should unescape ampersands", () => {
expect(unescapeXmlAttr("foo &amp; bar")).toBe("foo & bar");
});
it("should unescape double quotes", () => {
expect(unescapeXmlAttr("foo &quot;bar&quot; baz")).toBe('foo "bar" baz');
});
it("should unescape angle brackets", () => {
expect(unescapeXmlAttr("foo &lt;bar&gt; baz")).toBe("foo <bar> baz");
});
it("should unescape all special characters together", () => {
expect(unescapeXmlAttr("use &lt;a&gt; and &quot;b&quot; &amp; c")).toBe(
'use <a> and "b" & c',
);
});
it("should handle empty strings", () => {
expect(unescapeXmlAttr("")).toBe("");
});
it("should not modify strings without escaped characters", () => {
expect(unescapeXmlAttr("hello world")).toBe("hello world");
});
});
describe("escapeXmlContent", () => {
it("should escape ampersands", () => {
expect(escapeXmlContent("foo & bar")).toBe("foo &amp; bar");
});
it("should escape angle brackets", () => {
expect(escapeXmlContent("foo <bar> baz")).toBe("foo &lt;bar&gt; baz");
});
it("should NOT escape double quotes (content doesn't need it)", () => {
expect(escapeXmlContent('foo "bar" baz')).toBe('foo "bar" baz');
});
it("should handle empty strings", () => {
expect(escapeXmlContent("")).toBe("");
});
});
describe("unescapeXmlContent", () => {
it("should unescape ampersands", () => {
expect(unescapeXmlContent("foo &amp; bar")).toBe("foo & bar");
});
it("should unescape angle brackets", () => {
expect(unescapeXmlContent("foo &lt;bar&gt; baz")).toBe("foo <bar> baz");
});
it("should handle empty strings", () => {
expect(unescapeXmlContent("")).toBe("");
});
});
describe("roundtrip", () => {
it("should roundtrip attribute values correctly", () => {
const original = 'path with <special> "chars" & ampersand';
const escaped = escapeXmlAttr(original);
const unescaped = unescapeXmlAttr(escaped);
expect(unescaped).toBe(original);
});
it("should roundtrip content correctly", () => {
const original = "content with <tags> & ampersand";
const escaped = escapeXmlContent(original);
const unescaped = unescapeXmlContent(escaped);
expect(unescaped).toBe(original);
});
it("should handle complex nested escapes correctly", () => {
// Test that &amp;lt; doesn't get double-unescaped
const original = "literal &lt; should stay as &lt;";
const escaped = escapeXmlContent(original);
expect(escaped).toBe("literal &amp;lt; should stay as &amp;lt;");
const unescaped = unescapeXmlContent(escaped);
expect(unescaped).toBe(original);
});
});
});
...@@ -36,6 +36,7 @@ import { DyadStatus } from "./DyadStatus"; ...@@ -36,6 +36,7 @@ import { DyadStatus } from "./DyadStatus";
import { mapActionToButton } from "./ChatInput"; import { mapActionToButton } from "./ChatInput";
import { SuggestedAction } from "@/lib/schemas"; import { SuggestedAction } from "@/lib/schemas";
import { FixAllErrorsButton } from "./FixAllErrorsButton"; import { FixAllErrorsButton } from "./FixAllErrorsButton";
import { unescapeXmlAttr, unescapeXmlContent } from "../../../shared/xmlEscape";
const DYAD_CUSTOM_TAGS = [ const DYAD_CUSTOM_TAGS = [
"dyad-write", "dyad-write",
...@@ -274,25 +275,25 @@ function parseCustomTags(content: string): ContentPiece[] { ...@@ -274,25 +275,25 @@ function parseCustomTags(content: string): ContentPiece[] {
}); });
} }
// Parse attributes // Parse attributes and unescape values
const attributes: Record<string, string> = {}; const attributes: Record<string, string> = {};
const attrPattern = /([\w-]+)="([^"]*)"/g; const attrPattern = /([\w-]+)="([^"]*)"/g;
let attrMatch; let attrMatch;
while ((attrMatch = attrPattern.exec(attributesStr)) !== null) { while ((attrMatch = attrPattern.exec(attributesStr)) !== null) {
attributes[attrMatch[1]] = attrMatch[2]; attributes[attrMatch[1]] = unescapeXmlAttr(attrMatch[2]);
} }
// Check if this tag was marked as in progress // Check if this tag was marked as in progress
const tagInProgressSet = inProgressTags.get(tag); const tagInProgressSet = inProgressTags.get(tag);
const isInProgress = tagInProgressSet?.has(startIndex); const isInProgress = tagInProgressSet?.has(startIndex);
// Add the tag info // Add the tag info with unescaped content
contentPieces.push({ contentPieces.push({
type: "custom-tag", type: "custom-tag",
tagInfo: { tagInfo: {
tag, tag,
attributes, attributes,
content: tagContent, content: unescapeXmlContent(tagContent),
fullMatch, fullMatch,
inProgress: isInProgress || false, inProgress: isInProgress || false,
}, },
......
...@@ -67,6 +67,7 @@ import { cleanFullResponse } from "../utils/cleanFullResponse"; ...@@ -67,6 +67,7 @@ import { cleanFullResponse } from "../utils/cleanFullResponse";
import { generateProblemReport } from "../processors/tsc"; import { generateProblemReport } from "../processors/tsc";
import { createProblemFixPrompt } from "@/shared/problem_prompt"; import { createProblemFixPrompt } from "@/shared/problem_prompt";
import { AsyncVirtualFileSystem } from "../../../shared/VirtualFilesystem"; import { AsyncVirtualFileSystem } from "../../../shared/VirtualFilesystem";
import { escapeXmlAttr, escapeXmlContent } from "../../../shared/xmlEscape";
import { import {
getDyadAddDependencyTags, getDyadAddDependencyTags,
getDyadWriteTags, getDyadWriteTags,
...@@ -125,13 +126,7 @@ async function isTextFile(filePath: string): Promise<boolean> { ...@@ -125,13 +126,7 @@ async function isTextFile(filePath: string): Promise<boolean> {
return TEXT_FILE_EXTENSIONS.includes(ext); return TEXT_FILE_EXTENSIONS.includes(ext);
} }
function escapeXml(unsafe: string): string { // Use escapeXmlAttr from shared/xmlEscape for XML escaping
return unsafe
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;");
}
// Safely parse an MCP tool key that combines server and tool names. // Safely parse an MCP tool key that combines server and tool names.
// We split on the LAST occurrence of "__" to avoid ambiguity if either // We split on the LAST occurrence of "__" to avoid ambiguity if either
...@@ -1276,7 +1271,7 @@ ${formattedSearchReplaceIssues}`, ...@@ -1276,7 +1271,7 @@ ${formattedSearchReplaceIssues}`,
${problemReport.problems ${problemReport.problems
.map( .map(
(problem) => (problem) =>
`<problem file="${escapeXml(problem.file)}" line="${problem.line}" column="${problem.column}" code="${problem.code}">${escapeXml(problem.message)}</problem>`, `<problem file="${escapeXmlAttr(problem.file)}" line="${problem.line}" column="${problem.column}" code="${problem.code}">${escapeXmlContent(problem.message)}</problem>`,
) )
.join("\n")} .join("\n")}
</dyad-problem-report>`; </dyad-problem-report>`;
......
import { normalizePath } from "../../../shared/normalizePath"; import { normalizePath } from "../../../shared/normalizePath";
import { unescapeXmlAttr, unescapeXmlContent } from "../../../shared/xmlEscape";
import log from "electron-log"; import log from "electron-log";
import { SqlQuery } from "../../lib/schemas"; import { SqlQuery } from "../../lib/schemas";
...@@ -18,14 +19,16 @@ export function getDyadWriteTags(fullResponse: string): { ...@@ -18,14 +19,16 @@ export function getDyadWriteTags(fullResponse: string): {
while ((match = dyadWriteRegex.exec(fullResponse)) !== null) { while ((match = dyadWriteRegex.exec(fullResponse)) !== null) {
const attributesString = match[1]; const attributesString = match[1];
let content = match[2].trim(); let content = unescapeXmlContent(match[2].trim());
const pathMatch = pathRegex.exec(attributesString); const pathMatch = pathRegex.exec(attributesString);
const descriptionMatch = descriptionRegex.exec(attributesString); const descriptionMatch = descriptionRegex.exec(attributesString);
if (pathMatch && pathMatch[1]) { if (pathMatch && pathMatch[1]) {
const path = pathMatch[1]; const path = unescapeXmlAttr(pathMatch[1]);
const description = descriptionMatch?.[1]; const description = descriptionMatch?.[1]
? unescapeXmlAttr(descriptionMatch[1])
: undefined;
const contentLines = content.split("\n"); const contentLines = content.split("\n");
if (contentLines[0]?.startsWith("```")) { if (contentLines[0]?.startsWith("```")) {
...@@ -57,8 +60,8 @@ export function getDyadRenameTags(fullResponse: string): { ...@@ -57,8 +60,8 @@ export function getDyadRenameTags(fullResponse: string): {
const tags: { from: string; to: string }[] = []; const tags: { from: string; to: string }[] = [];
while ((match = dyadRenameRegex.exec(fullResponse)) !== null) { while ((match = dyadRenameRegex.exec(fullResponse)) !== null) {
tags.push({ tags.push({
from: normalizePath(match[1]), from: normalizePath(unescapeXmlAttr(match[1])),
to: normalizePath(match[2]), to: normalizePath(unescapeXmlAttr(match[2])),
}); });
} }
return tags; return tags;
...@@ -70,7 +73,7 @@ export function getDyadDeleteTags(fullResponse: string): string[] { ...@@ -70,7 +73,7 @@ export function getDyadDeleteTags(fullResponse: string): string[] {
let match; let match;
const paths: string[] = []; const paths: string[] = [];
while ((match = dyadDeleteRegex.exec(fullResponse)) !== null) { while ((match = dyadDeleteRegex.exec(fullResponse)) !== null) {
paths.push(normalizePath(match[1])); paths.push(normalizePath(unescapeXmlAttr(match[1])));
} }
return paths; return paths;
} }
...@@ -81,7 +84,7 @@ export function getDyadAddDependencyTags(fullResponse: string): string[] { ...@@ -81,7 +84,7 @@ export function getDyadAddDependencyTags(fullResponse: string): string[] {
let match; let match;
const packages: string[] = []; const packages: string[] = [];
while ((match = dyadAddDependencyRegex.exec(fullResponse)) !== null) { while ((match = dyadAddDependencyRegex.exec(fullResponse)) !== null) {
packages.push(...match[1].split(" ")); packages.push(...unescapeXmlAttr(match[1]).split(" "));
} }
return packages; return packages;
} }
...@@ -91,7 +94,7 @@ export function getDyadChatSummaryTag(fullResponse: string): string | null { ...@@ -91,7 +94,7 @@ export function getDyadChatSummaryTag(fullResponse: string): string | null {
/<dyad-chat-summary>([\s\S]*?)<\/dyad-chat-summary>/g; /<dyad-chat-summary>([\s\S]*?)<\/dyad-chat-summary>/g;
const match = dyadChatSummaryRegex.exec(fullResponse); const match = dyadChatSummaryRegex.exec(fullResponse);
if (match && match[1]) { if (match && match[1]) {
return match[1].trim(); return unescapeXmlContent(match[1].trim());
} }
return null; return null;
} }
...@@ -105,9 +108,11 @@ export function getDyadExecuteSqlTags(fullResponse: string): SqlQuery[] { ...@@ -105,9 +108,11 @@ export function getDyadExecuteSqlTags(fullResponse: string): SqlQuery[] {
while ((match = dyadExecuteSqlRegex.exec(fullResponse)) !== null) { while ((match = dyadExecuteSqlRegex.exec(fullResponse)) !== null) {
const attributesString = match[1] || ""; const attributesString = match[1] || "";
let content = match[2].trim(); let content = unescapeXmlContent(match[2].trim());
const descriptionMatch = descriptionRegex.exec(attributesString); const descriptionMatch = descriptionRegex.exec(attributesString);
const description = descriptionMatch?.[1]; const description = descriptionMatch?.[1]
? unescapeXmlAttr(descriptionMatch[1])
: undefined;
// Handle markdown code blocks if present // Handle markdown code blocks if present
const contentLines = content.split("\n"); const contentLines = content.split("\n");
...@@ -132,7 +137,7 @@ export function getDyadCommandTags(fullResponse: string): string[] { ...@@ -132,7 +137,7 @@ export function getDyadCommandTags(fullResponse: string): string[] {
const commands: string[] = []; const commands: string[] = [];
while ((match = dyadCommandRegex.exec(fullResponse)) !== null) { while ((match = dyadCommandRegex.exec(fullResponse)) !== null) {
commands.push(match[1]); commands.push(unescapeXmlAttr(match[1]));
} }
return commands; return commands;
...@@ -153,14 +158,16 @@ export function getDyadSearchReplaceTags(fullResponse: string): { ...@@ -153,14 +158,16 @@ export function getDyadSearchReplaceTags(fullResponse: string): {
while ((match = dyadSearchReplaceRegex.exec(fullResponse)) !== null) { while ((match = dyadSearchReplaceRegex.exec(fullResponse)) !== null) {
const attributesString = match[1] || ""; const attributesString = match[1] || "";
let content = match[2].trim(); let content = unescapeXmlContent(match[2].trim());
const pathMatch = pathRegex.exec(attributesString); const pathMatch = pathRegex.exec(attributesString);
const descriptionMatch = descriptionRegex.exec(attributesString); const descriptionMatch = descriptionRegex.exec(attributesString);
if (pathMatch && pathMatch[1]) { if (pathMatch && pathMatch[1]) {
const path = pathMatch[1]; const path = unescapeXmlAttr(pathMatch[1]);
const description = descriptionMatch?.[1]; const description = descriptionMatch?.[1]
? unescapeXmlAttr(descriptionMatch[1])
: undefined;
// Handle markdown code fences if present // Handle markdown code fences if present
const contentLines = content.split("\n"); const contentLines = content.split("\n");
......
...@@ -12,17 +12,12 @@ import { AgentTodo } from "@/ipc/ipc_types"; ...@@ -12,17 +12,12 @@ import { AgentTodo } from "@/ipc/ipc_types";
// XML Escape Helpers // XML Escape Helpers
// ============================================================================ // ============================================================================
export function escapeXmlAttr(str: string): string { export {
return str escapeXmlAttr,
.replace(/&/g, "&amp;") unescapeXmlAttr,
.replace(/"/g, "&quot;") escapeXmlContent,
.replace(/</g, "&lt;") unescapeXmlContent,
.replace(/>/g, "&gt;"); } from "../../../../../../../shared/xmlEscape";
}
export function escapeXmlContent(str: string): string {
return str.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
}
// ============================================================================ // ============================================================================
// Todo Types // Todo Types
......
...@@ -8,11 +8,7 @@ ...@@ -8,11 +8,7 @@
*/ */
import type { ToolCallPart } from "ai"; import type { ToolCallPart } from "ai";
import { escapeXmlContent } from "../../../../../../shared/xmlEscape";
// Escape XML content (less strict than attributes)
function escapeXmlContent(str: string): string {
return str.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
}
/** /**
* Wrap thinking text in think tags * Wrap thinking text in think tags
......
Markdown 格式
0%
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论