-
-
Notifications
You must be signed in to change notification settings - Fork 457
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
Changes from all commits
4bc632e
b2d712f
a2bc541
1db4146
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,12 +1,19 @@ | ||||||||||||||||||||||||||||
| import { useToast } from "@/components/ui/use-toast"; | ||||||||||||||||||||||||||||
| import { MAX_SVG_FILE_SIZE } from "@/lib/constants"; | ||||||||||||||||||||||||||||
| import { PromptImage } from "@/types/ai"; | ||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||
| ALLOWED_IMAGE_TYPES, | ||||||||||||||||||||||||||||
| optimizeSvgContent, | ||||||||||||||||||||||||||||
| validateSvgContent, | ||||||||||||||||||||||||||||
| } from "@/utils/ai/image-upload"; | ||||||||||||||||||||||||||||
| import { useRef } from "react"; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| export type PromptImageWithLoading = PromptImage & { loading: boolean }; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| export type ImageUploadAction = | ||||||||||||||||||||||||||||
| | { type: "ADD"; payload: { url: string; file: File }[] } | ||||||||||||||||||||||||||||
| | { type: "REMOVE"; payload: { index: number } } | ||||||||||||||||||||||||||||
| | { type: "REMOVE_BY_URL"; payload: { url: string } } | ||||||||||||||||||||||||||||
| | { type: "CLEAR" } | ||||||||||||||||||||||||||||
| | { type: "UPDATE_URL"; payload: { tempUrl: string; finalUrl: string } } | ||||||||||||||||||||||||||||
| | { type: "INITIALIZE"; payload: { url: string }[] }; | ||||||||||||||||||||||||||||
|
|
@@ -38,7 +45,14 @@ export function useImageUpload({ maxFiles, maxFileSize, images, dispatch }: UseI | |||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| const validFiles = fileArray.filter((file) => { | ||||||||||||||||||||||||||||
| if (!file.type.startsWith("image/")) return false; | ||||||||||||||||||||||||||||
| if (!ALLOWED_IMAGE_TYPES.includes(file.type)) { | ||||||||||||||||||||||||||||
| toast({ | ||||||||||||||||||||||||||||
| title: "Unsupported file type", | ||||||||||||||||||||||||||||
| description: `"${file.name}" is not supported. Please use JPG, PNG, WebP, or SVG files.`, | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (file.size > maxFileSize) { | ||||||||||||||||||||||||||||
| toast({ | ||||||||||||||||||||||||||||
| title: "File too large", | ||||||||||||||||||||||||||||
|
|
@@ -60,12 +74,71 @@ export function useImageUpload({ maxFiles, maxFileSize, images, dispatch }: UseI | |||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| filesWithTempUrls.forEach(({ url: tempUrl, file }) => { | ||||||||||||||||||||||||||||
| const reader = new FileReader(); | ||||||||||||||||||||||||||||
| reader.onload = (e) => { | ||||||||||||||||||||||||||||
| const finalUrl = e.target?.result as string; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| const handleSuccess = (result: string) => { | ||||||||||||||||||||||||||||
| let finalUrl: string; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (file.type === "image/svg+xml") { | ||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||
| const isValidSvg = validateSvgContent(result); | ||||||||||||||||||||||||||||
| if (!isValidSvg) { | ||||||||||||||||||||||||||||
| toast({ | ||||||||||||||||||||||||||||
| title: "Potentially unsafe SVG", | ||||||||||||||||||||||||||||
| description: `"${file.name}" may contain unsafe content but will be processed anyway.`, | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| const optimizedSvg = optimizeSvgContent(result); | ||||||||||||||||||||||||||||
| const encodedSvg = encodeURIComponent(optimizedSvg); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (encodedSvg.length > MAX_SVG_FILE_SIZE) { | ||||||||||||||||||||||||||||
| handleError(); | ||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+94
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| finalUrl = `data:image/svg+xml,${encodedSvg}`; | ||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||
| handleError(); | ||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||
| finalUrl = result; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| dispatch({ type: "UPDATE_URL", payload: { tempUrl, finalUrl } }); | ||||||||||||||||||||||||||||
| URL.revokeObjectURL(tempUrl); | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
| reader.readAsDataURL(file); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| const handleError = () => { | ||||||||||||||||||||||||||||
| toast({ | ||||||||||||||||||||||||||||
| title: "File read error", | ||||||||||||||||||||||||||||
| description: `Failed to read "${file.name}". Please try again.`, | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| dispatch({ | ||||||||||||||||||||||||||||
| type: "REMOVE_BY_URL", | ||||||||||||||||||||||||||||
| payload: { url: tempUrl }, | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
| URL.revokeObjectURL(tempUrl); | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| reader.onload = (e) => { | ||||||||||||||||||||||||||||
| const result = e.target?.result; | ||||||||||||||||||||||||||||
| if (!result || typeof result !== "string") { | ||||||||||||||||||||||||||||
| handleError(); | ||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| handleSuccess(result); | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| reader.onerror = handleError; | ||||||||||||||||||||||||||||
| reader.onabort = handleError; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (file.type === "image/svg+xml") { | ||||||||||||||||||||||||||||
| reader.readAsText(file); | ||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||
| reader.readAsDataURL(file); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| export const ALLOWED_IMAGE_TYPES = [ | ||
| "image/jpeg", | ||
| "image/jpg", | ||
| "image/png", | ||
| "image/webp", | ||
| "image/svg+xml", | ||
| ]; | ||
|
|
||
| export function validateSvgContent(svgText: string): boolean { | ||
| try { | ||
| const trimmed = svgText.trim(); | ||
| if (!trimmed.toLowerCase().includes("<svg")) { | ||
| return false; | ||
| } | ||
|
|
||
| const dangerousPatterns = [ | ||
| /<script/i, | ||
| /javascript:/i, | ||
| /on\w+\s*=/i, // onclick, onload, etc. | ||
| /<embed/i, | ||
| /<object/i, | ||
| /<iframe/i, | ||
| ]; | ||
|
|
||
| return !dangerousPatterns.some((pattern) => pattern.test(svgText)); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export function optimizeSvgContent(svgText: string): string { | ||
| try { | ||
| return svgText | ||
| .replace(/<!--[\s\S]*?-->/g, "") // Remove comments | ||
| .replace(/>\s+</g, "><") // Remove unnecessary whitespace | ||
| .trim(); | ||
| } catch { | ||
| return svgText.trim(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import { ChatMessage } from "@/types/ai"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { buildPromptForAPI } from "@/utils/ai/ai-prompt"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { CoreMessage, ImagePart, TextPart, UserContent } from "ai"; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { CoreMessage, TextPart, UserContent } from "ai"; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| export async function convertChatMessagesToCoreMessages( | ||||||||||||||||||||||||||||||||||||||||||||||
| messages: ChatMessage[] | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -12,15 +12,36 @@ export async function convertChatMessagesToCoreMessages( | |||||||||||||||||||||||||||||||||||||||||||||
| const content: UserContent = []; | ||||||||||||||||||||||||||||||||||||||||||||||
| const { promptData } = message; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // Add image parts | ||||||||||||||||||||||||||||||||||||||||||||||
| if (promptData.images && promptData.images.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const imageParts = promptData.images.map( | ||||||||||||||||||||||||||||||||||||||||||||||
| (image): ImagePart => ({ | ||||||||||||||||||||||||||||||||||||||||||||||
| type: "image", | ||||||||||||||||||||||||||||||||||||||||||||||
| image: image.url, | ||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||
| content.push(...imageParts); | ||||||||||||||||||||||||||||||||||||||||||||||
| promptData.images.forEach((image) => { | ||||||||||||||||||||||||||||||||||||||||||||||
| if (image.url.startsWith("data:image/svg+xml")) { | ||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||
| const dataUrlPart = image.url.split(",")[1]; | ||||||||||||||||||||||||||||||||||||||||||||||
| let svgMarkup: string; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if (image.url.includes("base64")) { | ||||||||||||||||||||||||||||||||||||||||||||||
| svgMarkup = atob(dataUrlPart); | ||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||
| svgMarkup = decodeURIComponent(dataUrlPart); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+19
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify 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 tsxLength 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) 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 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| content.push({ | ||||||||||||||||||||||||||||||||||||||||||||||
| type: "text", | ||||||||||||||||||||||||||||||||||||||||||||||
| text: `Here is an SVG image for analysis:\n\`\`\`svg\n${svgMarkup}\n\`\`\``, | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||
| content.push({ | ||||||||||||||||||||||||||||||||||||||||||||||
| type: "image", | ||||||||||||||||||||||||||||||||||||||||||||||
| image: image.url, | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||
| content.push({ | ||||||||||||||||||||||||||||||||||||||||||||||
| type: "image", | ||||||||||||||||||||||||||||||||||||||||||||||
| image: image.url, | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // Add text part | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
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
🤖 Prompt for AI Agents