Skip to content

Commit adc3f3d

Browse files
committed
feat: enhance image handling in MCP tool processing
**Changes:** - Improved validation for base64 image data in `processToolContent` to handle invalid and non-string data gracefully. - Added error handling to log warnings for corrupted images without interrupting processing. - Updated tests to verify correct behavior when encountering invalid base64 data and non-string inputs. **Files Modified:** - `src/core/tools/useMcpToolTool.ts` - `src/core/tools/__tests__/useMcpToolTool.spec.ts` - `webview-ui/src/components/common/Thumbnails.tsx`
1 parent a05620b commit adc3f3d

File tree

3 files changed

+195
-15
lines changed

3 files changed

+195
-15
lines changed

src/core/tools/__tests__/useMcpToolTool.spec.ts

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,135 @@ describe("useMcpToolTool", () => {
333333
expect(mockPushToolResult).toHaveBeenCalledWith("Tool result: (with 1 images)")
334334
})
335335

336+
it("should handle corrupted base64 image data gracefully", async () => {
337+
const block: ToolUse = {
338+
type: "tool_use",
339+
name: "use_mcp_tool",
340+
params: {
341+
server_name: "test_server",
342+
tool_name: "test_tool",
343+
arguments: '{"param": "value"}',
344+
},
345+
partial: false,
346+
}
347+
348+
mockAskApproval.mockResolvedValue(true)
349+
350+
const mockToolResult = {
351+
content: [
352+
{ type: "text", text: "Generated content with images:" },
353+
{
354+
type: "image",
355+
data: "invalid@base64@data", // Invalid base64 characters
356+
mimeType: "image/png",
357+
},
358+
{
359+
type: "image",
360+
data: "", // Empty base64 data
361+
mimeType: "image/png",
362+
},
363+
{
364+
type: "image",
365+
data: "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChAI9jU", // Valid base64
366+
mimeType: "image/png",
367+
},
368+
],
369+
isError: false,
370+
}
371+
372+
mockProviderRef.deref.mockReturnValue({
373+
getMcpHub: () => ({
374+
callTool: vi.fn().mockResolvedValue(mockToolResult),
375+
}),
376+
postMessageToWebview: vi.fn(),
377+
})
378+
379+
// Spy on console.warn to verify error logging
380+
const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
381+
382+
await useMcpToolTool(
383+
mockTask as Task,
384+
block,
385+
mockAskApproval,
386+
mockHandleError,
387+
mockPushToolResult,
388+
mockRemoveClosingTag,
389+
)
390+
391+
// Should continue processing despite corrupted images
392+
expect(mockTask.consecutiveMistakeCount).toBe(0)
393+
expect(mockAskApproval).toHaveBeenCalled()
394+
expect(mockTask.say).toHaveBeenCalledWith("mcp_server_request_started")
395+
396+
// Should only include the valid image, not the corrupted ones
397+
expect(mockTask.say).toHaveBeenCalledWith("mcp_server_response", "Generated content with images:", [
398+
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChAI9jU",
399+
])
400+
expect(mockPushToolResult).toHaveBeenCalledWith(
401+
"Tool result: Generated content with images: (with 1 images)",
402+
)
403+
404+
// Should log warnings for corrupted images
405+
expect(consoleSpy).toHaveBeenCalledWith("Invalid MCP ImageContent: base64 data contains invalid characters")
406+
expect(consoleSpy).toHaveBeenCalledWith("Invalid MCP ImageContent: base64 data is not a valid string")
407+
408+
consoleSpy.mockRestore()
409+
})
410+
411+
it("should handle non-string base64 data", async () => {
412+
const block: ToolUse = {
413+
type: "tool_use",
414+
name: "use_mcp_tool",
415+
params: {
416+
server_name: "test_server",
417+
tool_name: "test_tool",
418+
arguments: '{"param": "value"}',
419+
},
420+
partial: false,
421+
}
422+
423+
mockAskApproval.mockResolvedValue(true)
424+
425+
const mockToolResult = {
426+
content: [
427+
{ type: "text", text: "Some text" },
428+
{
429+
type: "image",
430+
data: 12345, // Non-string data
431+
mimeType: "image/png",
432+
},
433+
],
434+
isError: false,
435+
}
436+
437+
mockProviderRef.deref.mockReturnValue({
438+
getMcpHub: () => ({
439+
callTool: vi.fn().mockResolvedValue(mockToolResult),
440+
}),
441+
postMessageToWebview: vi.fn(),
442+
})
443+
444+
const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
445+
446+
await useMcpToolTool(
447+
mockTask as Task,
448+
block,
449+
mockAskApproval,
450+
mockHandleError,
451+
mockPushToolResult,
452+
mockRemoveClosingTag,
453+
)
454+
455+
// Should process text content normally
456+
expect(mockTask.say).toHaveBeenCalledWith("mcp_server_response", "Some text", [])
457+
expect(mockPushToolResult).toHaveBeenCalledWith("Tool result: Some text")
458+
459+
// Should log warning for invalid data type
460+
expect(consoleSpy).toHaveBeenCalledWith("Invalid MCP ImageContent: base64 data is not a valid string")
461+
462+
consoleSpy.mockRestore()
463+
})
464+
336465
it("should handle user rejection", async () => {
337466
const block: ToolUse = {
338467
type: "tool_use",

src/core/tools/useMcpToolTool.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -207,11 +207,29 @@ function processToolContent(toolResult: any): { text: string; images: string[] }
207207
if (item.type === "text") {
208208
textParts.push(item.text)
209209
} else if (item.type === "image") {
210-
if (item.data && item.mimeType) {
210+
if (item.mimeType && item.data !== undefined && item.data !== null) {
211211
const validImageTypes = ["image/png", "image/jpeg", "image/gif", "image/webp"]
212212
if (validImageTypes.includes(item.mimeType)) {
213-
const dataUrl = `data:${item.mimeType};base64,${item.data}`
214-
images.push(dataUrl)
213+
try {
214+
// Validate base64 data before constructing data URL
215+
if (typeof item.data !== "string" || item.data.trim() === "") {
216+
console.warn("Invalid MCP ImageContent: base64 data is not a valid string")
217+
return
218+
}
219+
220+
// Basic validation for base64 format
221+
const base64Regex = /^[A-Za-z0-9+/]*={0,2}$/
222+
if (!base64Regex.test(item.data.replace(/\s/g, ""))) {
223+
console.warn("Invalid MCP ImageContent: base64 data contains invalid characters")
224+
return
225+
}
226+
227+
const dataUrl = `data:${item.mimeType};base64,${item.data}`
228+
images.push(dataUrl)
229+
} catch (error) {
230+
console.warn("Failed to process MCP image content:", error)
231+
// Continue processing other content instead of failing entirely
232+
}
215233
} else {
216234
console.warn(`Unsupported image MIME type: ${item.mimeType}`)
217235
}

webview-ui/src/components/common/Thumbnails.tsx

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ interface ThumbnailsProps {
1111

1212
const Thumbnails = ({ images, style, setImages, onHeightChange }: ThumbnailsProps) => {
1313
const [hoveredIndex, setHoveredIndex] = useState<number | null>(null)
14+
const [failedImages, setFailedImages] = useState<Set<number>>(new Set())
1415
const containerRef = useRef<HTMLDivElement>(null)
1516
const { width } = useWindowSize()
1617

@@ -24,12 +25,19 @@ const Thumbnails = ({ images, style, setImages, onHeightChange }: ThumbnailsProp
2425
onHeightChange?.(height)
2526
}
2627
setHoveredIndex(null)
28+
// Reset failed images when images change
29+
setFailedImages(new Set())
2730
}, [images, width, onHeightChange])
2831

2932
const handleDelete = (index: number) => {
3033
setImages?.((prevImages) => prevImages.filter((_, i) => i !== index))
3134
}
3235

36+
const handleImageError = (index: number) => {
37+
setFailedImages((prev) => new Set(prev).add(index))
38+
console.warn(`Failed to load image at index ${index}`)
39+
}
40+
3341
const isDeletable = setImages !== undefined
3442

3543
const handleImageClick = (image: string) => {
@@ -53,18 +61,43 @@ const Thumbnails = ({ images, style, setImages, onHeightChange }: ThumbnailsProp
5361
style={{ position: "relative" }}
5462
onMouseEnter={() => setHoveredIndex(index)}
5563
onMouseLeave={() => setHoveredIndex(null)}>
56-
<img
57-
src={image}
58-
alt={`Thumbnail ${index + 1}`}
59-
style={{
60-
width: 34,
61-
height: 34,
62-
objectFit: "cover",
63-
borderRadius: 4,
64-
cursor: "pointer",
65-
}}
66-
onClick={() => handleImageClick(image)}
67-
/>
64+
{failedImages.has(index) ? (
65+
<div
66+
style={{
67+
width: 34,
68+
height: 34,
69+
borderRadius: 4,
70+
backgroundColor: "var(--vscode-input-background)",
71+
border: "1px solid var(--vscode-input-border)",
72+
display: "flex",
73+
justifyContent: "center",
74+
alignItems: "center",
75+
cursor: "pointer",
76+
}}
77+
onClick={() => handleImageClick(image)}
78+
title="Failed to load image">
79+
<span
80+
className="codicon codicon-file-media"
81+
style={{
82+
color: "var(--vscode-descriptionForeground)",
83+
fontSize: 16,
84+
}}></span>
85+
</div>
86+
) : (
87+
<img
88+
src={image}
89+
alt={`Thumbnail ${index + 1}`}
90+
style={{
91+
width: 34,
92+
height: 34,
93+
objectFit: "cover",
94+
borderRadius: 4,
95+
cursor: "pointer",
96+
}}
97+
onClick={() => handleImageClick(image)}
98+
onError={() => handleImageError(index)}
99+
/>
100+
)}
68101
{isDeletable && hoveredIndex === index && (
69102
<div
70103
onClick={() => handleDelete(index)}

0 commit comments

Comments
 (0)