From 5b8d1721cc1dd66dfbcf88262a0d1c6c0cb35df8 Mon Sep 17 00:00:00 2001 From: hannesrudolph Date: Wed, 11 Jun 2025 21:35:22 -0600 Subject: [PATCH] refactor: extract URI utilities and add comprehensive tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract createSafeContentUri to shared utils/uri.ts module - Add extensive unit tests for URI truncation logic - Remove code duplication between DiffViewProvider and checkpoints - Move MAX_SAFE_URI_LENGTH constant to shared utility - Add edge case testing for invalid characters and length limits 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/core/checkpoints/index.ts | 45 +------ src/integrations/editor/DiffViewProvider.ts | 43 +------ src/utils/__tests__/uri.test.ts | 125 ++++++++++++++++++++ src/utils/uri.ts | 46 +++++++ 4 files changed, 176 insertions(+), 83 deletions(-) create mode 100644 src/utils/__tests__/uri.test.ts create mode 100644 src/utils/uri.ts diff --git a/src/core/checkpoints/index.ts b/src/core/checkpoints/index.ts index 62c31a212e..eb385615c7 100644 --- a/src/core/checkpoints/index.ts +++ b/src/core/checkpoints/index.ts @@ -11,46 +11,7 @@ import { ClineApiReqInfo } from "../../shared/ExtensionMessage" import { getApiMetrics } from "../../shared/getApiMetrics" import { DIFF_VIEW_URI_SCHEME } from "../../integrations/editor/DiffViewProvider" - -// Maximum safe URI length to avoid crashes in language servers -// Most systems have limits between 2KB-32KB, using conservative 8KB limit -const MAX_SAFE_URI_LENGTH = 8192 - -/** - * Safely creates a diff URI by validating the total URI length. - * If the URI would be too long, truncates the content to avoid LSP crashes. - */ -function createSafeDiffUri(fileName: string, content: string): vscode.Uri { - try { - const base64Content = Buffer.from(content).toString("base64") - const baseUri = `${DIFF_VIEW_URI_SCHEME}:${fileName}` - const testUri = vscode.Uri.parse(baseUri).with({ query: base64Content }).toString() - - if (testUri.length <= MAX_SAFE_URI_LENGTH) { - return vscode.Uri.parse(baseUri).with({ query: base64Content }) - } - - // Calculate available space for content after accounting for URI overhead - const overhead = baseUri.length + 50 // Extra buffer for URI encoding - 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 diff URI for ${fileName}:`, error) - // Fallback to empty content if all else fails - return vscode.Uri.parse(`${DIFF_VIEW_URI_SCHEME}:${fileName}`).with({ - query: Buffer.from("").toString("base64"), - }) - } -} +import { createSafeContentUri } from "../../utils/uri" import { CheckpointServiceOptions, RepoPerTaskCheckpointService } from "../../services/checkpoints" @@ -317,8 +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), - createSafeDiffUri(change.paths.relative, change.content.before ?? ""), - createSafeDiffUri(change.paths.relative, change.content.after ?? ""), + 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 8ffba4fd9a..88170aad43 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -13,49 +13,10 @@ 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" -// Maximum safe URI length to avoid crashes in language servers -// Most systems have limits between 2KB-32KB, using conservative 8KB limit -const MAX_SAFE_URI_LENGTH = 8192 - -/** - * Safely creates a diff URI by validating the total URI length. - * If the URI would be too long, truncates the content to avoid LSP crashes. - */ -function createSafeDiffUri(fileName: string, content: string): vscode.Uri { - try { - const base64Content = Buffer.from(content).toString("base64") - const baseUri = `${DIFF_VIEW_URI_SCHEME}:${fileName}` - const testUri = vscode.Uri.parse(baseUri).with({ query: base64Content }).toString() - - if (testUri.length <= MAX_SAFE_URI_LENGTH) { - return vscode.Uri.parse(baseUri).with({ query: base64Content }) - } - - // Calculate available space for content after accounting for URI overhead - const overhead = baseUri.length + 50 // Extra buffer for URI encoding - 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 diff URI for ${fileName}:`, error) - // Fallback to empty content if all else fails - return vscode.Uri.parse(`${DIFF_VIEW_URI_SCHEME}:${fileName}`).with({ - query: Buffer.from("").toString("base64"), - }) - } -} - // TODO: https://github.com/cline/cline/pull/3354 export class DiffViewProvider { // Properties to store the results of saveChanges @@ -484,7 +445,7 @@ export class DiffViewProvider { vscode.commands.executeCommand( "vscode.diff", - createSafeDiffUri(fileName, this.originalContent ?? ""), + 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..149e3ec5f0 --- /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 testUri = vscode.Uri.parse(baseUri).with({ query: base64Content }).toString() + + if (testUri.length <= MAX_SAFE_URI_LENGTH) { + return vscode.Uri.parse(baseUri).with({ query: base64Content }) + } + + // Calculate available space for content after accounting for URI overhead + const overhead = baseUri.length + 50 // Extra buffer for URI encoding + 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"), + }) + } +}