Add SVG format support to the QR code API#4184
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe QR API now accepts ChangesQR SVG support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant QRRoute
participant getQRAsSVG
participant Response
Client->>QRRoute: GET QR request with format=svg
QRRoute->>getQRAsSVG: Generate SVG from qrProps
getQRAsSVG-->>QRRoute: SVG string
QRRoute->>Response: Set SVG headers and return SVG
Response-->>Client: image/svg+xml response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
ESLint install timed out. The project may have too many dependencies for the sandbox. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/qr/index.tsx (1)
389-396: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
getQRAsSVGDataUriignores explicitbgColorfor downloads. The caller inapps/web/ui/modals/qr-code-design-fields.tsxpasses the fullqrData, so a user-selected background color is dropped here whilegetQRAsSVGstill paints it conditionally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/qr/index.tsx` around lines 389 - 396, The getQRAsSVGDataUri serialization in getQRAsSVGDataUri must honor an explicitly provided bgColor, matching the conditional background behavior in getQRAsSVG. Use the caller-provided qrData background color when constructing the SVG, while preserving transparent output when no background color is selected.
🧹 Nitpick comments (1)
apps/web/app/api/qr/route.tsx (1)
63-66: 🚀 Performance & Scalability | 🔵 TrivialConsider caching headers for the SVG response.
The new SVG response only sets
Content-Type/Content-Disposition; noCache-Controlis set. IfImageResponseapplies caching headers by default for the PNG branch, the SVG branch may end up less cacheable at the CDN/edge, impacting scalability under repeated requests for the same QR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/api/qr/route.tsx` around lines 63 - 66, Update the SVG response in the QR route to include an appropriate Cache-Control header, matching the caching behavior used by the ImageResponse PNG branch where applicable. Preserve the existing Content-Type and Content-Disposition headers while ensuring repeated SVG requests can be cached by the CDN or edge.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/lib/qr/api.tsx`:
- Around line 41-68: Move the excavation call in the image-generation flow so
`excavateModules` runs only after the logo fetch succeeds; failed embedding must
retain the original `cells`. Add an `AbortController`-based timeout to the
`fetch` request for `imageSettings.src`, aborting slow responses and allowing
the existing `catch` fallback to return a non-excavated QR.
---
Outside diff comments:
In `@apps/web/lib/qr/index.tsx`:
- Around line 389-396: The getQRAsSVGDataUri serialization in getQRAsSVGDataUri
must honor an explicitly provided bgColor, matching the conditional background
behavior in getQRAsSVG. Use the caller-provided qrData background color when
constructing the SVG, while preserving transparent output when no background
color is selected.
---
Nitpick comments:
In `@apps/web/app/api/qr/route.tsx`:
- Around line 63-66: Update the SVG response in the QR route to include an
appropriate Cache-Control header, matching the caching behavior used by the
ImageResponse PNG branch where applicable. Preserve the existing Content-Type
and Content-Disposition headers while ensuring repeated SVG requests can be
cached by the CDN or edge.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4379757a-93c1-4f96-ad74-8169a41c0f6f
📒 Files selected for processing (5)
apps/web/app/api/qr/route.tsxapps/web/lib/openapi/qr/index.tsapps/web/lib/qr/api.tsxapps/web/lib/qr/index.tsxapps/web/lib/zod/schemas/qr.ts
|
@coderabbitai Full review. Any breaking changes, and is this backward compatible? |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/lib/qr/api.tsx`:
- Around line 71-85: Escape or otherwise safely encode bgColor and fgColor
before interpolating them into the fill attributes generated by the SVG
construction in the QR rendering function. Keep the existing color behavior
while ensuring crafted query parameters cannot break out of SVG attribute
context; retain schema-level validation as the primary protection.
In `@apps/web/lib/zod/schemas/qr.ts`:
- Around line 35-40: Constrain both optional color fields, bgColor and fgColor,
in the QR schema to documented hexadecimal color values using a regex validator.
Apply the same validation to each z.string() definition before values reach
getQRAsSVG, while preserving their optional behavior and existing
defaults/descriptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1bf828c0-7482-42e6-840c-558414c2e0d6
📒 Files selected for processing (5)
apps/web/app/api/qr/route.tsxapps/web/lib/openapi/qr/index.tsapps/web/lib/qr/api.tsxapps/web/lib/qr/index.tsxapps/web/lib/zod/schemas/qr.ts
| // Drawing strategy: instead of a rect per module, we're going to create a | ||
| // single path for the dark modules and layer that on top of a light rect, | ||
| // for a total of 2 DOM nodes. We pay a bit more in string concat but that's | ||
| // way faster than DOM ops. | ||
| // For level 1, 441 nodes -> 2 | ||
| // For level 40, 31329 -> 2 | ||
| // single path for the dark modules. Background is only painted when bgColor | ||
| // is set — otherwise the SVG stays transparent. | ||
| const fgPath = generatePath(cells, margin); | ||
| const bgPath = bgColor | ||
| ? `<path fill="${bgColor}" d="M0,0 h${numCells}v${numCells}H0z" shape-rendering="crispEdges"></path>` | ||
| : ""; | ||
|
|
||
| return ( | ||
| <svg | ||
| height={size} | ||
| width={size} | ||
| viewBox={`0 0 ${numCells} ${numCells}`} | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| {...otherProps} | ||
| > | ||
| <path | ||
| fill={bgColor} | ||
| d={`M0,0 h${numCells}v${numCells}H0z`} | ||
| shapeRendering="crispEdges" | ||
| /> | ||
| <path fill={fgColor} d={fgPath} shapeRendering="crispEdges" /> | ||
| {image} | ||
| </svg> | ||
| ); | ||
| return [ | ||
| `<svg xmlns="http://www.w3.org/2000/svg" height="${size}" width="${size}" viewBox="0 0 ${numCells} ${numCells}">`, | ||
| bgPath, | ||
| `<path fill="${fgColor}" d="${fgPath}" shape-rendering="crispEdges"></path>`, | ||
| image, | ||
| "</svg>", | ||
| ].join(""); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Unescaped bgColor/fgColor interpolated into SVG attributes — XSS via crafted query params.
bgColor/fgColor are inserted directly into fill="${...}" attributes of a hand-built SVG string, then returned with Content-Type: image/svg+xml (see apps/web/app/api/qr/route.tsx lines 58-67) from a public, unauthenticated GET endpoint. A value containing "> breaks out of the attribute context and injects arbitrary SVG markup (e.g. <script>), which executes when the SVG is opened directly or embedded via <object>/<iframe>. This is a new attack surface: the PNG path is safe because it's JSX-rendered and rasterized, but this raw string-based SVG output returns attacker-controlled markup verbatim.
Fixing the input at the schema layer (see apps/web/lib/zod/schemas/qr.ts comment) is the primary fix; escaping here as well provides defense-in-depth.
🛡️ Proposed defense-in-depth fix
+const escapeSvgAttr = (value: string) =>
+ value
+ .replace(/&/g, "&")
+ .replace(/"/g, """)
+ .replace(/</g, "<")
+ .replace(/>/g, ">");
+
const fgPath = generatePath(cells, margin);
const bgPath = bgColor
- ? `<path fill="${bgColor}" d="M0,0 h${numCells}v${numCells}H0z" shape-rendering="crispEdges"></path>`
+ ? `<path fill="${escapeSvgAttr(bgColor)}" d="M0,0 h${numCells}v${numCells}H0z" shape-rendering="crispEdges"></path>`
: "";
return [
`<svg xmlns="http://www.w3.org/2000/svg" height="${size}" width="${size}" viewBox="0 0 ${numCells} ${numCells}">`,
bgPath,
- `<path fill="${fgColor}" d="${fgPath}" shape-rendering="crispEdges"></path>`,
+ `<path fill="${escapeSvgAttr(fgColor)}" d="${fgPath}" shape-rendering="crispEdges"></path>`,
image,
"</svg>",
].join("");📝 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.
| // Drawing strategy: instead of a rect per module, we're going to create a | |
| // single path for the dark modules and layer that on top of a light rect, | |
| // for a total of 2 DOM nodes. We pay a bit more in string concat but that's | |
| // way faster than DOM ops. | |
| // For level 1, 441 nodes -> 2 | |
| // For level 40, 31329 -> 2 | |
| // single path for the dark modules. Background is only painted when bgColor | |
| // is set — otherwise the SVG stays transparent. | |
| const fgPath = generatePath(cells, margin); | |
| const bgPath = bgColor | |
| ? `<path fill="${bgColor}" d="M0,0 h${numCells}v${numCells}H0z" shape-rendering="crispEdges"></path>` | |
| : ""; | |
| return ( | |
| <svg | |
| height={size} | |
| width={size} | |
| viewBox={`0 0 ${numCells} ${numCells}`} | |
| xmlns="http://www.w3.org/2000/svg" | |
| {...otherProps} | |
| > | |
| <path | |
| fill={bgColor} | |
| d={`M0,0 h${numCells}v${numCells}H0z`} | |
| shapeRendering="crispEdges" | |
| /> | |
| <path fill={fgColor} d={fgPath} shapeRendering="crispEdges" /> | |
| {image} | |
| </svg> | |
| ); | |
| return [ | |
| `<svg xmlns="http://www.w3.org/2000/svg" height="${size}" width="${size}" viewBox="0 0 ${numCells} ${numCells}">`, | |
| bgPath, | |
| `<path fill="${fgColor}" d="${fgPath}" shape-rendering="crispEdges"></path>`, | |
| image, | |
| "</svg>", | |
| ].join(""); | |
| // Drawing strategy: instead of a rect per module, we're going to create a | |
| // single path for the dark modules. Background is only painted when bgColor | |
| // is set — otherwise the SVG stays transparent. | |
| const escapeSvgAttr = (value: string) => | |
| value | |
| .replace(/&/g, "&") | |
| .replace(/"/g, """) | |
| .replace(/</g, "<") | |
| .replace(/>/g, ">"); | |
| const fgPath = generatePath(cells, margin); | |
| const bgPath = bgColor | |
| ? `<path fill="${escapeSvgAttr(bgColor)}" d="M0,0 h${numCells}v${numCells}H0z" shape-rendering="crispEdges"></path>` | |
| : ""; | |
| return [ | |
| `<svg xmlns="http://www.w3.org/2000/svg" height="${size}" width="${size}" viewBox="0 0 ${numCells} ${numCells}">`, | |
| bgPath, | |
| `<path fill="${escapeSvgAttr(fgColor)}" d="${fgPath}" shape-rendering="crispEdges"></path>`, | |
| image, | |
| "</svg>", | |
| ].join(""); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/qr/api.tsx` around lines 71 - 85, Escape or otherwise safely
encode bgColor and fgColor before interpolating them into the fill attributes
generated by the SVG construction in the QR rendering function. Keep the
existing color behavior while ensuring crafted query parameters cannot break out
of SVG attribute context; retain schema-level validation as the primary
protection.
| bgColor: z | ||
| .string() | ||
| .optional() | ||
| .default(DEFAULT_BGCOLOR) | ||
| .describe( | ||
| "The background color of the QR code in hex format. Defaults to `#ffffff` if not provided.", | ||
| "The background color of the QR code in hex format. Defaults to `#ffffff` for `png`. When omitted for `svg`, the background is transparent.", | ||
| ), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
No format validation on bgColor/fgColor enables SVG attribute-injection.
Both bgColor (here) and fgColor (lines 28-34) accept arbitrary strings. apps/web/lib/qr/api.tsx's new getQRAsSVG interpolates these values unescaped directly into SVG attribute values (fill="${bgColor}") and returns the result with Content-Type: image/svg+xml from a public, unauthenticated endpoint. A value like "><script>...</script><path fill=" breaks out of the attribute and injects arbitrary SVG/script markup. Unlike the PNG path (JSX-rendered → rasterized, immune to this), the SVG string path returns attacker-controlled markup verbatim — this is a well-known SVG XSS vector when served as image/svg+xml (script execution on direct navigation or <object>/<iframe> embedding).
Constrain both fields to the documented "hex format" via a regex so malformed/malicious values are rejected before reaching the renderer.
🛡️ Proposed fix
+const HEX_COLOR_REGEX = /^#?(?:[0-9A-Fa-f]{3,4}|[0-9A-Fa-f]{6}|[0-9A-Fa-f]{8})$/;
+
fgColor: z
.string()
+ .regex(HEX_COLOR_REGEX, "fgColor must be a valid hex color (e.g. `#000000`).")
.optional()
.default(DEFAULT_FGCOLOR)
.describe(
"The foreground color of the QR code in hex format. Defaults to `#000000` if not provided.",
),
bgColor: z
.string()
+ .regex(HEX_COLOR_REGEX, "bgColor must be a valid hex color (e.g. `#ffffff`).")
.optional()
.describe(
"The background color of the QR code in hex format. Defaults to `#ffffff` for `png`. When omitted for `svg`, the background is transparent.",
),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/zod/schemas/qr.ts` around lines 35 - 40, Constrain both optional
color fields, bgColor and fgColor, in the QR schema to documented hexadecimal
color values using a regex validator. Apply the same validation to each
z.string() definition before values reach getQRAsSVG, while preserving their
optional behavior and existing defaults/descriptions.
1 participant
Summary by CodeRabbit
New Features
format=svg, includingimage/svg+xmlheaders and an inlineqr.svgfilename.image/svg+xml) alongside PNG.Enhancements
bgColoromission now behaves format-dependently: PNG defaults to#ffffff, while SVG is transparent.