mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
refactor: extract URI utilities and add comprehensive tests
- 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 <noreply@anthropic.com>
This commit is contained in:
hannesrudolph
parent
abc237870d
commit
5b8d1721cc
4 changed files with 176 additions and 83 deletions
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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 },
|
||||
|
|
|
|||
125
src/utils/__tests__/uri.test.ts
Normal file
125
src/utils/__tests__/uri.test.ts
Normal file
|
|
@ -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
|
||||
})
|
||||
})
|
||||
})
|
||||
46
src/utils/uri.ts
Normal file
46
src/utils/uri.ts
Normal file
|
|
@ -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"),
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue