diff --git a/.changeset/fix-uri-length-lsp-crash.md b/.changeset/fix-uri-length-lsp-crash.md new file mode 100644 index 0000000000..e177298dc2 --- /dev/null +++ b/.changeset/fix-uri-length-lsp-crash.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +Fix C# LSP crashes caused by excessively long URIs in diff views and checkpoints. Added URI length validation to prevent crashes when working with large files by truncating content that would exceed safe URI length limits. diff --git a/src/core/checkpoints/index.ts b/src/core/checkpoints/index.ts index dcbe796eb7..eb385615c7 100644 --- a/src/core/checkpoints/index.ts +++ b/src/core/checkpoints/index.ts @@ -11,6 +11,7 @@ import { ClineApiReqInfo } from "../../shared/ExtensionMessage" import { getApiMetrics } from "../../shared/getApiMetrics" import { DIFF_VIEW_URI_SCHEME } from "../../integrations/editor/DiffViewProvider" +import { createSafeContentUri } from "../../utils/uri" import { CheckpointServiceOptions, RepoPerTaskCheckpointService } from "../../services/checkpoints" @@ -277,12 +278,8 @@ export async function checkpointDiff(cline: Task, { ts, previousCommitHash, comm mode === "full" ? "Changes since task started" : "Changes since previous checkpoint", changes.map((change) => [ vscode.Uri.file(change.paths.absolute), - vscode.Uri.parse(`${DIFF_VIEW_URI_SCHEME}:${change.paths.relative}`).with({ - query: Buffer.from(change.content.before ?? "").toString("base64"), - }), - vscode.Uri.parse(`${DIFF_VIEW_URI_SCHEME}:${change.paths.relative}`).with({ - query: Buffer.from(change.content.after ?? "").toString("base64"), - }), + createSafeContentUri(DIFF_VIEW_URI_SCHEME, change.paths.relative, change.content.before ?? ""), + createSafeContentUri(DIFF_VIEW_URI_SCHEME, change.paths.relative, change.content.after ?? ""), ]), ) } catch (err) { diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts index 3ab0419618..88170aad43 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -13,6 +13,7 @@ import { ClineSayTool } from "../../shared/ExtensionMessage" import { Task } from "../../core/task/Task" import { DecorationController } from "./DecorationController" +import { createSafeContentUri } from "../../utils/uri" export const DIFF_VIEW_URI_SCHEME = "cline-diff" @@ -310,14 +311,14 @@ export class DiffViewProvider { indentBy: "", suppressEmptyNode: true, processEntities: false, - tagValueProcessor: (name, value) => { + tagValueProcessor: (_name, value) => { if (typeof value === "string") { // Only escape <, >, and & characters return value.replace(/&/g, "&").replace(//g, ">") } return value }, - attributeValueProcessor: (name, value) => { + attributeValueProcessor: (_name, value) => { if (typeof value === "string") { // Only escape <, >, and & characters return value.replace(/&/g, "&").replace(//g, ">") @@ -444,9 +445,7 @@ export class DiffViewProvider { vscode.commands.executeCommand( "vscode.diff", - vscode.Uri.parse(`${DIFF_VIEW_URI_SCHEME}:${fileName}`).with({ - query: Buffer.from(this.originalContent ?? "").toString("base64"), - }), + createSafeContentUri(DIFF_VIEW_URI_SCHEME, fileName, this.originalContent ?? ""), uri, `${fileName}: ${fileExists ? "Original ↔ Roo's Changes" : "New File"} (Editable)`, { preserveFocus: true }, diff --git a/src/utils/__tests__/uri.test.ts b/src/utils/__tests__/uri.test.ts new file mode 100644 index 0000000000..5a8ad7a9d7 --- /dev/null +++ b/src/utils/__tests__/uri.test.ts @@ -0,0 +1,125 @@ +import * as vscode from "vscode" +import { createSafeContentUri, MAX_SAFE_URI_LENGTH } from "../uri" + +// Mock vscode.Uri to avoid VS Code dependency in tests +jest.mock("vscode", () => ({ + Uri: { + parse: jest.fn((uriString: string) => ({ + toString: () => uriString, + with: jest.fn(({ query }: { query: string }) => ({ + toString: () => `${uriString}?${query}`, + scheme: uriString.split(":")[0], + path: uriString.split(":")[1], + query, + })), + })), + }, +})) + +describe("uri utilities", () => { + describe("createSafeContentUri", () => { + const scheme = "test-scheme" + const path = "test-file.txt" + + beforeEach(() => { + jest.clearAllMocks() + }) + + it("should create a normal URI for small content", () => { + const content = "small content" + const result = createSafeContentUri(scheme, path, content) + + expect(vscode.Uri.parse).toHaveBeenCalledWith(`${scheme}:${path}`) + expect(result.query).toBe(Buffer.from(content).toString("base64")) + }) + + it("should truncate content when URI would exceed safe length", () => { + // Create content that would result in a very long URI + const longContent = "x".repeat(10000) // 10KB of content + const result = createSafeContentUri(scheme, path, longContent) + + // Verify the URI was created + expect(vscode.Uri.parse).toHaveBeenCalledWith(`${scheme}:${path}`) + + // Decode the base64 query to check if content was truncated + const decodedContent = Buffer.from(result.query, "base64").toString() + expect(decodedContent).toContain("... [Content truncated to prevent LSP crashes]") + expect(decodedContent.length).toBeLessThan(longContent.length) + + // Verify the total URI length is within safe limits + const totalUriLength = result.toString().length + expect(totalUriLength).toBeLessThanOrEqual(MAX_SAFE_URI_LENGTH) + }) + + it("should handle empty content", () => { + const content = "" + const result = createSafeContentUri(scheme, path, content) + + expect(result.query).toBe(Buffer.from(content).toString("base64")) + }) + + it("should handle content exactly at the safe limit", () => { + // Calculate content size that would result in URI exactly at limit + const baseUri = `${scheme}:${path}` + const overhead = baseUri.length + 50 + const maxBase64Length = MAX_SAFE_URI_LENGTH - overhead + const maxContentLength = Math.floor((maxBase64Length * 3) / 4) + + const content = "x".repeat(maxContentLength) + const result = createSafeContentUri(scheme, path, content) + + const totalUriLength = result.toString().length + expect(totalUriLength).toBeLessThanOrEqual(MAX_SAFE_URI_LENGTH) + }) + + it("should handle invalid characters gracefully", () => { + // Test with potentially problematic content that might cause issues + const problemContent = "\uFFFE\uFFFF\x00\x01\x02" + const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation() + + const result = createSafeContentUri(scheme, path, problemContent) + + // Should still create a valid URI (may log error but shouldn't crash) + expect(result).toBeDefined() + expect(result.query).toBeDefined() + + consoleErrorSpy.mockRestore() + }) + + it("should use provided scheme and path correctly", () => { + const customScheme = "cline-diff" + const customPath = "src/components/Button.tsx" + const content = "test content" + + createSafeContentUri(customScheme, customPath, content) + + expect(vscode.Uri.parse).toHaveBeenCalledWith(`${customScheme}:${customPath}`) + }) + + it("should preserve content when within safe limits", () => { + const content = "This is some test content that should not be truncated" + const result = createSafeContentUri(scheme, path, content) + + const decodedContent = Buffer.from(result.query, "base64").toString() + expect(decodedContent).toBe(content) + expect(decodedContent).not.toContain("... [Content truncated to prevent LSP crashes]") + }) + + it("should calculate base64 expansion correctly", () => { + // Base64 encoding expands content by ~4/3 + const content = "x".repeat(1000) + const base64Content = Buffer.from(content).toString("base64") + + // Verify our calculation assumption + expect(base64Content.length).toBeCloseTo((content.length * 4) / 3, -1) // Within 10% + }) + }) + + describe("MAX_SAFE_URI_LENGTH constant", () => { + it("should be set to a reasonable value", () => { + expect(MAX_SAFE_URI_LENGTH).toBe(8192) + expect(MAX_SAFE_URI_LENGTH).toBeGreaterThan(2000) // Above minimum + expect(MAX_SAFE_URI_LENGTH).toBeLessThan(32768) // Below typical maximum + }) + }) +}) diff --git a/src/utils/uri.ts b/src/utils/uri.ts new file mode 100644 index 0000000000..5ca817bc33 --- /dev/null +++ b/src/utils/uri.ts @@ -0,0 +1,46 @@ +import * as vscode from "vscode" + +// Maximum safe URI length to avoid crashes in language servers +// Most systems have limits between 2KB-32KB, using conservative 8KB limit +export const MAX_SAFE_URI_LENGTH = 8192 + +/** + * Safely creates a URI with content encoded in the query parameter. + * If the resulting URI would be too long, truncates the content to avoid LSP crashes. + * + * @param scheme - The URI scheme (e.g., "cline-diff") + * @param path - The URI path/identifier + * @param content - Content to encode as base64 in the query parameter + * @returns A safe URI that won't exceed system limits + */ +export function createSafeContentUri(scheme: string, path: string, content: string): vscode.Uri { + try { + const base64Content = Buffer.from(content).toString("base64") + const baseUri = `${scheme}:${path}` + const uri = vscode.Uri.parse(baseUri).with({ query: base64Content }) + + if (uri.toString().length <= MAX_SAFE_URI_LENGTH) { + return uri + } + + // Calculate available space for content after accounting for URI overhead + const overhead = baseUri.length + 100 // Extra buffer for URI encoding and VS Code overhead + const maxBase64Length = Math.max(0, MAX_SAFE_URI_LENGTH - overhead) + + // Truncate content to fit within safe URI length + const maxContentLength = Math.floor((maxBase64Length * 3) / 4) // Base64 is ~4/3 the size + const truncatedContent = + content.length > maxContentLength + ? content.substring(0, maxContentLength) + "\n... [Content truncated to prevent LSP crashes]" + : content + + const truncatedBase64 = Buffer.from(truncatedContent).toString("base64") + return vscode.Uri.parse(baseUri).with({ query: truncatedBase64 }) + } catch (error) { + console.error(`Failed to create safe content URI for ${path}:`, error) + // Fallback to empty content if all else fails + return vscode.Uri.parse(`${scheme}:${path}`).with({ + query: Buffer.from("").toString("base64"), + }) + } +}