From ba5af60109027c32fe29936257274d676d42f96a Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Wed, 16 Apr 2025 18:05:04 -0400 Subject: [PATCH] Fix diff escaping issues (#2694) * Fix diff escaping issues * Potential fix for code scanning alert no. 75: Double escaping or unescaping Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- src/core/tools/applyDiffTool.ts | 8 +++- src/core/tools/executeCommandTool.ts | 5 +- src/core/tools/writeToFileTool.ts | 9 +--- .../__tests__/text-normalization.test.ts | 48 ++++++++++++++++++- src/utils/text-normalization.ts | 18 +++++++ 5 files changed, 76 insertions(+), 12 deletions(-) diff --git a/src/core/tools/applyDiffTool.ts b/src/core/tools/applyDiffTool.ts index d18adaa8d0..43de5af988 100644 --- a/src/core/tools/applyDiffTool.ts +++ b/src/core/tools/applyDiffTool.ts @@ -11,7 +11,7 @@ import path from "path" import fs from "fs/promises" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" import { telemetryService } from "../../services/telemetry/TelemetryService" - +import { unescapeHtmlEntities } from "../../utils/text-normalization" export async function applyDiffTool( cline: Cline, block: ToolUse, @@ -21,7 +21,11 @@ export async function applyDiffTool( removeClosingTag: RemoveClosingTag, ) { const relPath: string | undefined = block.params.path - const diffContent: string | undefined = block.params.diff + let diffContent: string | undefined = block.params.diff + + if (diffContent && !cline.api.getModel().id.includes("claude")) { + diffContent = unescapeHtmlEntities(diffContent) + } const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", diff --git a/src/core/tools/executeCommandTool.ts b/src/core/tools/executeCommandTool.ts index 79f89f092d..5a70c657ac 100644 --- a/src/core/tools/executeCommandTool.ts +++ b/src/core/tools/executeCommandTool.ts @@ -2,6 +2,7 @@ import { Cline } from "../Cline" import { ToolUse } from "../assistant-message" import { AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "./types" import { formatResponse } from "../prompts/responses" +import { unescapeHtmlEntities } from "../../utils/text-normalization" export async function executeCommandTool( cline: Cline, @@ -32,8 +33,8 @@ export async function executeCommandTool( return } - // unescape html entities (e.g. < -> <) - command = command.replace(/</g, "<").replace(/>/g, ">").replace(/&/g, "&") + // Unescape HTML entities + command = unescapeHtmlEntities(command) cline.consecutiveMistakeCount = 0 diff --git a/src/core/tools/writeToFileTool.ts b/src/core/tools/writeToFileTool.ts index 25f3a72df2..6a012537f6 100644 --- a/src/core/tools/writeToFileTool.ts +++ b/src/core/tools/writeToFileTool.ts @@ -14,6 +14,7 @@ import { isPathOutsideWorkspace } from "../../utils/pathUtils" import { everyLineHasLineNumbers } from "../../integrations/misc/extract-text" import delay from "delay" import { detectCodeOmission } from "../../integrations/editor/detect-omission" +import { unescapeHtmlEntities } from "../../utils/text-normalization" export async function writeToFileTool( cline: Cline, @@ -60,13 +61,7 @@ export async function writeToFileTool( } if (!cline.api.getModel().id.includes("claude")) { - // it seems not just llama models are doing cline, but also gemini and potentially others - if (newContent.includes(">") || newContent.includes("<") || newContent.includes(""")) { - newContent = newContent - .replace(/>/g, ">") - .replace(/</g, "<") - .replace(/"/g, '"') - } + newContent = unescapeHtmlEntities(newContent) } // Determine if the path is outside the workspace diff --git a/src/utils/__tests__/text-normalization.test.ts b/src/utils/__tests__/text-normalization.test.ts index da7184d889..5908ffc55b 100644 --- a/src/utils/__tests__/text-normalization.test.ts +++ b/src/utils/__tests__/text-normalization.test.ts @@ -1,4 +1,4 @@ -import { normalizeString } from "../text-normalization" +import { normalizeString, unescapeHtmlEntities } from "../text-normalization" describe("Text normalization utilities", () => { describe("normalizeString", () => { @@ -30,4 +30,50 @@ describe("Text normalization utilities", () => { expect(normalizeString(input)).toBe('Let\'s test this-with some "fancy" punctuation... and spaces') }) }) + + describe("unescapeHtmlEntities", () => { + test("unescapes basic HTML entities", () => { + expect(unescapeHtmlEntities("<div>Hello</div>")).toBe("
Hello
") + }) + + test("unescapes ampersand entity", () => { + expect(unescapeHtmlEntities("This & that")).toBe("This & that") + }) + + test("unescapes quote entities", () => { + expect(unescapeHtmlEntities(""quoted" and 'single-quoted'")).toBe( + "\"quoted\" and 'single-quoted'", + ) + }) + + test("unescapes apostrophe entity", () => { + expect(unescapeHtmlEntities("Don't worry")).toBe("Don't worry") + }) + + test("handles mixed content with multiple entity types", () => { + expect( + unescapeHtmlEntities( + "<a href="https://example.com?param1=value&param2=value">Link</a>", + ), + ).toBe('Link') + }) + + test("handles mixed content with apostrophe entities", () => { + expect( + unescapeHtmlEntities( + "<div>Don't forget that Tom&Jerry's show is at 3 o'clock</div>", + ), + ).toBe("
Don't forget that Tom&Jerry's show is at 3 o'clock
") + }) + + test("returns original string when no entities are present", () => { + const original = "Plain text without entities" + expect(unescapeHtmlEntities(original)).toBe(original) + }) + + test("handles empty or undefined input", () => { + expect(unescapeHtmlEntities("")).toBe("") + expect(unescapeHtmlEntities(undefined as unknown as string)).toBe(undefined) + }) + }) }) diff --git a/src/utils/text-normalization.ts b/src/utils/text-normalization.ts index b6e4e8da58..15f35c8437 100644 --- a/src/utils/text-normalization.ts +++ b/src/utils/text-normalization.ts @@ -75,3 +75,21 @@ export function normalizeString(str: string, options: NormalizeOptions = DEFAULT return normalized } + +/** + * Unescapes common HTML entities in a string + * + * @param text The string containing HTML entities to unescape + * @returns The unescaped string with HTML entities converted to their literal characters + */ +export function unescapeHtmlEntities(text: string): string { + if (!text) return text + + return text + .replace(/</g, "<") + .replace(/>/g, ">") + .replace(/"/g, '"') + .replace(/'/g, "'") + .replace(/'/g, "'") + .replace(/&/g, "&") +}