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>
This commit is contained in:
Matt Rubens 2025-04-16 18:05:04 -04:00 committed by GitHub
parent 4bf746d635
commit ba5af60109
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 76 additions and 12 deletions

View file

@ -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",

View file

@ -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. &lt; -> <)
command = command.replace(/&lt;/g, "<").replace(/&gt;/g, ">").replace(/&amp;/g, "&")
// Unescape HTML entities
command = unescapeHtmlEntities(command)
cline.consecutiveMistakeCount = 0

View file

@ -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("&gt;") || newContent.includes("&lt;") || newContent.includes("&quot;")) {
newContent = newContent
.replace(/&gt;/g, ">")
.replace(/&lt;/g, "<")
.replace(/&quot;/g, '"')
}
newContent = unescapeHtmlEntities(newContent)
}
// Determine if the path is outside the workspace

View file

@ -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("&lt;div&gt;Hello&lt;/div&gt;")).toBe("<div>Hello</div>")
})
test("unescapes ampersand entity", () => {
expect(unescapeHtmlEntities("This &amp; that")).toBe("This & that")
})
test("unescapes quote entities", () => {
expect(unescapeHtmlEntities("&quot;quoted&quot; and &#39;single-quoted&#39;")).toBe(
"\"quoted\" and 'single-quoted'",
)
})
test("unescapes apostrophe entity", () => {
expect(unescapeHtmlEntities("Don&apos;t worry")).toBe("Don't worry")
})
test("handles mixed content with multiple entity types", () => {
expect(
unescapeHtmlEntities(
"&lt;a href=&quot;https://example.com?param1=value&amp;param2=value&quot;&gt;Link&lt;/a&gt;",
),
).toBe('<a href="https://example.com?param1=value&param2=value">Link</a>')
})
test("handles mixed content with apostrophe entities", () => {
expect(
unescapeHtmlEntities(
"&lt;div&gt;Don&apos;t forget that Tom&amp;Jerry&apos;s show is at 3 o&apos;clock&lt;/div&gt;",
),
).toBe("<div>Don't forget that Tom&Jerry's show is at 3 o'clock</div>")
})
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)
})
})
})

View file

@ -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(/&lt;/g, "<")
.replace(/&gt;/g, ">")
.replace(/&quot;/g, '"')
.replace(/&#39;/g, "'")
.replace(/&apos;/g, "'")
.replace(/&amp;/g, "&")
}