-
-
Notifications
You must be signed in to change notification settings - Fork 439
Fix SVG issue when processed by the API and minor styles fixes #171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* SVG files are treated and displayed as Images in the UI * SVG files are processed as Markup Text to send to the API * Remove support for GIFs
@llanesluis is attempting to deploy a commit to the tweakcn OSS program Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis update introduces stricter image upload validation, particularly for SVG files, across the editor's AI components and hooks. It adds explicit MIME type checks, SVG content validation and optimization, error handling, and size limits. Styling adjustments are made to image preview components. The message conversion logic now decodes SVG images for analysis. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Uploader (UI)
participant useImageUpload (Hook)
participant image-upload utils
participant Toast
User->>Uploader (UI): Selects/Drags image file(s)
Uploader (UI)->>useImageUpload: Passes FileList
useImageUpload->>image-upload utils: Checks MIME type
alt Invalid type
useImageUpload->>Toast: Show error
useImageUpload-->>Uploader (UI): Skip file
else SVG file
useImageUpload->>image-upload utils: validateSvgContent
alt Unsafe SVG
useImageUpload->>Toast: Show warning
end
useImageUpload->>image-upload utils: optimizeSvgContent
useImageUpload->>useImageUpload: URI-encode SVG
alt SVG too large
useImageUpload->>Toast: Show error
useImageUpload-->>Uploader (UI): Remove file
else
useImageUpload-->>Uploader (UI): Add as preview
end
else Non-SVG image
useImageUpload->>useImageUpload: Read as Data URL
useImageUpload-->>Uploader (UI): Add as preview
end
sequenceDiagram
participant MessageConverter
participant Message
participant SVG Decoder
MessageConverter->>Message: Receives image URLs
alt URL is SVG data URL
MessageConverter->>SVG Decoder: Decode SVG (base64 or URI)
SVG Decoder-->>MessageConverter: SVG markup
MessageConverter->>Message: Add as markdown code block
else
MessageConverter->>Message: Add as image part
end
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/editor/ai/chat-image-preview.tsxOops! Something went wrong! :( ESLint: 9.24.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/[email protected][email protected][email protected][email protected]/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. components/editor/ai/drag-and-drop-image-uploader.tsxOops! Something went wrong! :( ESLint: 9.24.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/[email protected][email protected][email protected][email protected]/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. components/editor/ai/image-uploader.tsxOops! Something went wrong! :( ESLint: 9.24.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/[email protected][email protected][email protected][email protected]/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting.
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
utils/ai/image-upload.ts (1)
16-23
: Consider enhancing the dangerous pattern detection.While the current patterns cover common XSS vectors, consider adding validation for:
data:
URLs in attributes that might execute JavaScript<foreignObject>
tags which can contain HTML<use>
tags with external referencesconst dangerousPatterns = [ /<script/i, /javascript:/i, /on\w+\s*=/i, // onclick, onload, etc. /<embed/i, /<object/i, /<iframe/i, + /<foreignObject/i, + /<use[^>]*href\s*=\s*["'][^"']*#/i, // External references + /data:(?!image\/)/i, // Non-image data URLs ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
components/editor/ai/chat-image-preview.tsx
(1 hunks)components/editor/ai/drag-and-drop-image-uploader.tsx
(1 hunks)components/editor/ai/image-uploader.tsx
(2 hunks)components/editor/ai/message.tsx
(1 hunks)components/editor/ai/uploaded-image-preview.tsx
(1 hunks)hooks/use-image-upload-reducer.ts
(2 hunks)hooks/use-image-upload.ts
(3 hunks)lib/constants.ts
(1 hunks)utils/ai/generate-theme.tsx
(1 hunks)utils/ai/image-upload.ts
(1 hunks)utils/ai/message-converter.ts
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
components/editor/ai/message.tsx (1)
components/editor/ai/chat-image-preview.tsx (1)
ChatImagePreview
(17-55)
components/editor/ai/chat-image-preview.tsx (1)
lib/utils.ts (1)
cn
(6-8)
hooks/use-image-upload.ts (3)
types/ai.ts (1)
PromptImage
(12-14)utils/ai/image-upload.ts (3)
ALLOWED_IMAGE_TYPES
(1-7)validateSvgContent
(9-29)optimizeSvgContent
(31-40)lib/constants.ts (1)
MAX_SVG_FILE_SIZE
(9-9)
components/editor/ai/image-uploader.tsx (1)
utils/ai/image-upload.ts (1)
ALLOWED_IMAGE_TYPES
(1-7)
🔇 Additional comments (18)
lib/constants.ts (1)
9-9
: Good addition of SVG-specific size limit.The separate
MAX_SVG_FILE_SIZE
constant with a 1MB limit is appropriate since SVG files are typically much smaller than raster images. This allows for more granular validation control.components/editor/ai/uploaded-image-preview.tsx (1)
25-25
: Good styling consistency improvement.The removal of
inset-0
and addition ofrounded-md
andborder
classes improves visual consistency with other image preview components while maintaining the loading state functionality.components/editor/ai/message.tsx (2)
155-155
: Good separation of styling concerns.Removing
overflow-hidden
androunded-lg
classes from the container allows theChatImagePreview
component to handle its own styling, creating cleaner component boundaries.
163-163
: Consistent styling delegation.The removal of styling classes here maintains consistency with the single image case and properly delegates visual styling to the child component.
components/editor/ai/chat-image-preview.tsx (2)
21-21
: Good addition of consistent rounded corners.Adding
rounded-lg
class to the container improves visual consistency with other image preview components throughout the application.
23-27
: Reasonable image size reduction.Reducing the preview image dimensions from 300px to 250px makes the previews more compact while maintaining good visibility. This change should improve layout efficiency and potentially performance.
components/editor/ai/image-uploader.tsx (2)
5-5
: Good centralization of allowed image types.Importing
ALLOWED_IMAGE_TYPES
from a utility module creates a single source of truth for valid image formats, improving maintainability.
36-36
: Excellent improvement in file type validation.Replacing the generic
"image/*"
with explicit MIME types (ALLOWED_IMAGE_TYPES.join(",")
) provides better security and validation control. This prevents potentially harmful file types from being accepted while still supporting the necessary image formats.components/editor/ai/drag-and-drop-image-uploader.tsx (1)
21-27
: Good security improvement with explicit MIME type restrictions.Replacing the generic "image/*" with specific allowed MIME types is a security best practice that prevents unexpected file types from being uploaded while maintaining support for common image formats including SVG.
hooks/use-image-upload-reducer.ts (2)
26-28
: Good addition of URL-based removal functionality.The new
REMOVE_BY_URL
action type provides a useful way to remove images when you have the URL but not the index, which is particularly helpful for error handling scenarios.
46-51
: Proper synchronization logic update.Correctly includes the new
REMOVE_BY_URL
action in the sync conditions to ensure external state stays consistent.utils/ai/message-converter.ts (1)
16-44
: Well-implemented SVG decoding with appropriate fallback.The logic correctly handles different SVG data URL encoding formats (base64 and URL-encoded) and provides a sensible fallback to image parts when decoding fails. Converting SVG markup to text parts with markdown code blocks is a smart approach for AI analysis.
utils/ai/generate-theme.tsx (1)
8-14
: Clear and comprehensive visual analysis instructions.The updated prompt effectively guides the AI to analyze both traditional images and SVG markup, with specific direction on extracting visual elements. The instructions properly align with the new SVG-as-text handling implemented in the message converter.
utils/ai/image-upload.ts (3)
1-7
: Well-organized centralized image type constants.Good practice to centralize the allowed image types for consistency across components.
9-29
: Solid SVG security validation with comprehensive dangerous pattern detection.The validation function properly checks for SVG structure and screens out common XSS vectors. The regex patterns cover the main security concerns: script tags, JavaScript URLs, event handlers, and embedded content.
31-40
: Efficient SVG optimization with safe error handling.The optimization function provides a good balance between cleanup and safety, with appropriate fallback behavior when processing fails.
hooks/use-image-upload.ts (2)
48-54
: LGTM! Clear validation with helpful error messages.The explicit validation against
ALLOWED_IMAGE_TYPES
improves security and user experience by clearly communicating supported formats.
112-141
: Well-structured error handling and file reading logic.The implementation correctly:
- Handles both error and abort events
- Cleans up object URLs to prevent memory leaks
- Reads SVG files as text and other images as data URLs
- Provides clear error messages to users
const dataUrlPart = image.url.split(",")[1]; | ||
let svgMarkup: string; | ||
|
||
if (image.url.includes("base64")) { | ||
svgMarkup = atob(dataUrlPart); | ||
} else { | ||
svgMarkup = decodeURIComponent(dataUrlPart); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify data URL parsing robustness.
The current implementation assumes a specific data URL format and splits on the first comma. Consider adding validation to ensure the data URL has the expected structure before attempting to decode.
🏁 Script executed:
#!/bin/bash
# Check if there are any existing data URL parsing utilities in the codebase
rg -A 5 "data:.*," --type ts --type tsx
Length of output: 69
🏁 Script executed:
#!/bin/bash
# Retry searching for data URL usage in .ts and .tsx files without relying on rg file types
rg -A 5 "data:.*," -g "*.ts" -g "*.tsx"
Length of output: 5560
Add validation when parsing data URLs in utils/ai/message-converter.ts
Before splitting and decoding the data URL, ensure it has the expected structure to avoid runtime errors on malformed or unexpected values.
• File: utils/ai/message-converter.ts (lines 19–26)
• Replace blind .split(",")[1]
and includes("base64")
checks with explicit comma-indexing and header validation
Suggested diff:
- const dataUrlPart = image.url.split(",")[1];
+ const commaIndex = image.url.indexOf(",");
+ if (commaIndex === -1) {
+ throw new Error(`Invalid data URL: missing comma in ${image.url}`);
+ }
+ const header = image.url.slice(0, commaIndex);
+ const dataUrlPart = image.url.slice(commaIndex + 1);
let svgMarkup: string;
- if (image.url.includes("base64")) {
- svgMarkup = atob(dataUrlPart);
- } else {
- svgMarkup = decodeURIComponent(dataUrlPart);
- }
+ if (header.includes(";base64")) {
+ svgMarkup = atob(dataUrlPart);
+ } else {
+ svgMarkup = decodeURIComponent(dataUrlPart);
+ }
• Optionally tighten validation with a RegExp such as
/^data:[^;]+(;base64)?,/
to verify the data:
prefix and media type before decoding.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const dataUrlPart = image.url.split(",")[1]; | |
let svgMarkup: string; | |
if (image.url.includes("base64")) { | |
svgMarkup = atob(dataUrlPart); | |
} else { | |
svgMarkup = decodeURIComponent(dataUrlPart); | |
} | |
const commaIndex = image.url.indexOf(","); | |
if (commaIndex === -1) { | |
throw new Error(`Invalid data URL: missing comma in ${image.url}`); | |
} | |
const header = image.url.slice(0, commaIndex); | |
const dataUrlPart = image.url.slice(commaIndex + 1); | |
let svgMarkup: string; | |
if (header.includes(";base64")) { | |
svgMarkup = atob(dataUrlPart); | |
} else { | |
svgMarkup = decodeURIComponent(dataUrlPart); | |
} |
🤖 Prompt for AI Agents
In utils/ai/message-converter.ts around lines 19 to 26, the current code blindly
splits the image URL by a comma and checks for "base64" without validating the
data URL format, which can cause runtime errors. Add validation to ensure the
URL starts with "data:" and contains a comma separating the header and data
parts. Use a regular expression like /^data:[^;]+(;base64)?,/ to verify the
structure before splitting and decoding. Only proceed with decoding if the
validation passes; otherwise, handle the error or skip processing to avoid
exceptions.
const isValidSvg = validateSvgContent(result); | ||
if (!isValidSvg) { | ||
toast({ | ||
title: "Potentially unsafe SVG", | ||
description: `"${file.name}" may contain unsafe content but will be processed anyway.`, | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security concern: Processing potentially unsafe SVGs.
The code validates SVG content but still processes files that fail validation (line 87: "will be processed anyway"). This could expose users to XSS attacks if malicious SVG content bypasses the validation.
Consider rejecting unsafe SVGs entirely:
const isValidSvg = validateSvgContent(result);
if (!isValidSvg) {
- toast({
- title: "Potentially unsafe SVG",
- description: `"${file.name}" may contain unsafe content but will be processed anyway.`,
- });
+ toast({
+ title: "Unsafe SVG rejected",
+ description: `"${file.name}" contains potentially unsafe content and cannot be uploaded.`,
+ });
+ handleError();
+ return;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const isValidSvg = validateSvgContent(result); | |
if (!isValidSvg) { | |
toast({ | |
title: "Potentially unsafe SVG", | |
description: `"${file.name}" may contain unsafe content but will be processed anyway.`, | |
}); | |
} | |
const isValidSvg = validateSvgContent(result); | |
if (!isValidSvg) { | |
toast({ | |
title: "Unsafe SVG rejected", | |
description: `"${file.name}" contains potentially unsafe content and cannot be uploaded.`, | |
}); | |
handleError(); | |
return; | |
} |
🤖 Prompt for AI Agents
In hooks/use-image-upload.ts around lines 83 to 89, the code currently shows a
warning toast for unsafe SVGs but proceeds to process them, risking XSS attacks.
Modify the logic to reject and stop processing any SVG files that fail the
validateSvgContent check, preventing unsafe SVGs from being handled further.
if (encodedSvg.length > MAX_SVG_FILE_SIZE) { | ||
handleError(); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect size validation for SVG files.
The code checks the encoded string length against MAX_SVG_FILE_SIZE
, but encodeURIComponent
can significantly increase the size (up to 3x for certain characters). This means valid SVG files under 1MB could be rejected.
Consider checking the original file size instead:
-if (encodedSvg.length > MAX_SVG_FILE_SIZE) {
+// Check original file size before encoding
+if (file.size > MAX_SVG_FILE_SIZE) {
+ toast({
+ title: "SVG file too large",
+ description: `"${file.name}" exceeds the ${MAX_SVG_FILE_SIZE / 1024 / 1024}MB limit.`,
+ });
handleError();
return;
}
Alternatively, if you need to limit the encoded size for API constraints, consider using a different constant name like MAX_ENCODED_SVG_SIZE
with an appropriate value.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (encodedSvg.length > MAX_SVG_FILE_SIZE) { | |
handleError(); | |
return; | |
} | |
// Check original file size before encoding | |
if (file.size > MAX_SVG_FILE_SIZE) { | |
toast({ | |
title: "SVG file too large", | |
description: `"${file.name}" exceeds the ${MAX_SVG_FILE_SIZE / 1024 / 1024}MB limit.`, | |
}); | |
handleError(); | |
return; | |
} |
🤖 Prompt for AI Agents
In hooks/use-image-upload.ts around lines 94 to 97, the size validation
incorrectly checks the length of the encoded SVG string against
MAX_SVG_FILE_SIZE, which can cause valid files to be rejected due to encoding
expansion. Fix this by validating the original SVG file size before encoding
instead of the encoded string length. If limiting encoded size is necessary,
define a separate constant like MAX_ENCODED_SVG_SIZE with an appropriate
threshold and use that for the encoded string length check.
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation