Skip to content

Add optional fontSize, width, and height parameters for SVG customization (Fixes #334) #357

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.

Sign up for GitHub

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

Open
Bolu123-sudo wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add optional fontSize, width, and height parameters for SVG customization (Fixes #334) #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
802849e
FontSize, width, height query parameters for SVG generation (Fixes #334)
Bolu123-sudo Oct 3, 2025
e245415
FontSize, width, height query parameters for SVG generation (Fixes #334)
Bolu123-sudo Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .idea/.gitignore
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

483 changes: 483 additions & 0 deletions .idea/editor.xml
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions .idea/github-readme-quotes.iml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions .idea/modules.xml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions .idea/vcs.xml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 10 additions & 1 deletion src/api/controllers/quotesController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ const quoteController = async (req, res, next) => {
let quoteType = req.query.quoteType || '';
let unsplashQuery = req.query.unsplashQuery || '';

// (Issue #334)
const toNum = (v) => (v !== undefined && v !== '' && !Number.isNaN(Number(v))) ? Number(v) : undefined;
const fontSize = toNum(req.query.fontSize);
const width = toNum(req.query.width);
const height = toNum(req.query.height);
Comment on lines +41 to +45
Copy link

@coderabbitai coderabbitai bot Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Enhance input validation in the toNum helper.

The current implementation has several gaps:

  1. Allows Infinity and -Infinity: These pass the !Number.isNaN(Number(v)) check but are rejected later by Number.isFinite in the service layer. Validation should be consolidated.

  2. Allows negative and zero values: Negative dimensions (width=-100) and zero dimensions (fontSize=0) are nonsensical but aren't rejected.

  3. Redundant conversion: Number(v) is called twice—once for the NaN check and once for the return value.

Apply this diff to add comprehensive validation:

-    // (Issue #334)
-    const toNum = (v) => (v !== undefined && v !== '' && !Number.isNaN(Number(v))) ? Number(v) : undefined;
-    const fontSize = toNum(req.query.fontSize);
-    const width = toNum(req.query.width);
-    const height = toNum(req.query.height);
+    // (Issue #334) Parse and validate numeric query parameters
+    const toNum = (v) => {
+      if (v === undefined || v === '') return undefined;
+      const num = Number(v);
+      // Reject NaN, Infinity, and non-positive values
+      return Number.isFinite(num) && num > 0 ? num : undefined;
+    };
+    const fontSize = toNum(req.query.fontSize);
+    const width = toNum(req.query.width);
+    const height = toNum(req.query.height);

This consolidates validation, removes redundant conversions, and ensures only positive finite numbers are accepted. You may also want to add upper bounds (e.g., num > 0 && num <= 2000).

📝 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.

Suggested change
// (Issue #334)
const toNum = (v) => (v !== undefined && v !== '' && !Number.isNaN(Number(v))) ? Number(v) : undefined;
const fontSize = toNum(req.query.fontSize);
const width = toNum(req.query.width);
const height = toNum(req.query.height);
// (Issue #334) Parse and validate numeric query parameters
const toNum = (v) => {
if (v === undefined || v === '') return undefined;
const num = Number(v);
// Reject NaN, Infinity, and non-positive values
return Number.isFinite(num) && num > 0 ? num : undefined;
};
const fontSize = toNum(req.query.fontSize);
const width = toNum(req.query.width);
const height = toNum(req.query.height);
🤖 Prompt for AI Agents 
In src/api/controllers/quotesController.js around lines 41 to 45, the toNum
helper currently calls Number twice, allows Infinity/-Infinity, and accepts
zero/negative values; replace it with a single Number conversion stored in a
variable, validate with Number.isFinite(num) and require num > 0 (and optionally
an upper bound like num <= 2000), and return the finite positive number or
undefined; update usages to rely on this consolidated validation so downstream
service-layer checks are unnecessary.


let quoteObject = {
theme,
animation,
Expand All @@ -48,7 +54,10 @@ const quoteController = async (req, res, next) => {
quoteType,
borderColor,
bgSource,
unsplashQuery
unsplashQuery,
fontSize,
width,
height
}

let svgResponse = await quoteService.getQuote(quoteObject);
Expand Down
6 changes: 5 additions & 1 deletion src/api/services/quotesService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ getQuoteIndex = (apiResponseLength, quoteType) => {
const getQuote = async (quoteObj) => {

try {
let { theme, animation, layout, quotesUrl, quoteCategory, font, quoteType, borderColor, bgSource, unsplashQuery } = quoteObj;
let { theme, animation, layout, quotesUrl, quoteCategory, font, quoteType, borderColor, bgSource, unsplashQuery,fontSize, width,height } = quoteObj;
let apiResponse;
let { customQuotesUrl, isValidUrl } = await getValidUrl(quotesUrl);
let isCustomQuote = false;
Expand Down Expand Up @@ -58,6 +58,10 @@ const getQuote = async (quoteObj) => {
template.setLayout(layout);
template.bgImage = bgImageUrl;

if (Number.isFinite(fontSize)) template.fontSize = Number(fontSize);
if (Number.isFinite(width)) template.width = Number(width);
if (Number.isFinite(height)) template.height = Number(height);

let svg = cardTemplate.generateTemplate(template);
return svg;
} catch (error) {
Expand Down
64 changes: 36 additions & 28 deletions src/common/getTemplate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,35 +1,43 @@
const getTemplate = (template) => {
const escapeHtml = require('escape-html');
const safeUrl = escapeHtml(template.bgImage);
const backgroundImageLayer = safeUrl
? `<image href="${safeUrl}"
const escapeHtml = require('escape-html');


const svgWidth = Number.isFinite(template.width) ? template.width : 700;
const svgHeight = Number.isFinite(template.height) ? template.height : 200;
const textFontSize = Number.isFinite(template.fontSize) ? template.fontSize : 16;

const safeUrl = escapeHtml(template.bgImage);
const backgroundImageLayer = safeUrl
? `<image href="${safeUrl}"
x="0" y="0"
width="700"
height="${parseInt(template.height)}"
width="${svgWidth}"
height="${svgHeight}"
preserveAspectRatio="xMidYMid slice" />`
: '';
: '';

return `
<svg width="700px" height="${parseInt(
template.height
)}px" fill="none" xmlns="http://www.w3.org/2000/svg">
<defs>
<style>
@font-face{
font-family: "customFont";
src: ${template.font.src}
}
</style>
</defs>
${backgroundImageLayer}
<foreignObject width="100%" height="100%">
<div xmlns="http://www.w3.org/1999/xhtml">
<style>
${template.css}
</style>
${template.structure}
</div>
</foreignObject>
return `
<svg width="${svgWidth}px" height="${svgHeight}px" fill="none" xmlns="http://www.w3.org/2000/svg">
<defs>
<style>
@font-face{
font-family: "customFont";
src: ${template.font.src}
}

.quote-text {
font-size: ${textFontSize}px !important;
}
Comment on lines +27 to +29
Copy link

@coderabbitai coderabbitai bot Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that .quote-text selector targets the actual quote text.

The new CSS rule applies font-size via the .quote-text class, but the effectiveness depends on whether template.structure actually uses this class. If the quote text elements don't have class="quote-text", this rule won't apply.

Run the following script to verify that the .quote-text class is used in the layout structures:

🏁 Script executed:

#!/bin/bash
# Search for 'quote-text' class usage in layout files
rg -n --type=js 'quote-text' src/layouts/ src/models/

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Broad search for 'quote-text' usage across all source files
rg -n 'quote-text' src/

Length of output: 50

Ensure .quote-text selector targets quote elements. No instances of class="quote-text" were found in src/—either add this class to the quote text nodes in your template or adjust the selector to match the existing markup.

🤖 Prompt for AI Agents 
In src/common/getTemplate.js around lines 27 to 29, the CSS rule targets
.quote-text but no elements in src/ use that class; either add
class="quote-text" to the quote text nodes in the template where the actual
quote content is rendered, or change the selector here to match the existing
markup (for example the current element/class/id used for quote text). Update
the template or the selector so the style applies to the correct elements and
keep naming consistent across files.

</style>
</defs>
${backgroundImageLayer}
<foreignObject width="100%" height="100%">
<div xmlns="http://www.w3.org/1999/xhtml">
<style>
${template.css}
</style>
${template.structure}
</div>
</foreignObject>
</svg>`;
Comment on lines +18 to 41
Copy link

@coderabbitai coderabbitai bot Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify sanitization of template.css and template.structure.

While template.bgImage is now properly sanitized with escape-html, the template also injects template.css and template.structure directly into the SVG. If these properties contain user-controlled content, they could introduce XSS vulnerabilities.

Run the following script to trace the sources of template.css and template.structure:

If these properties incorporate user input without sanitization, apply escape-html to them as well.

🏁 Script executed:

#!/bin/bash
# Find where template.css and template.structure are set
rg -n --type=js 'template\.css\s*=' src/
rg -n --type=js 'template\.structure\s*=' src/
rg -n --type=js '\.setTheme\(' src/
rg -n --type=js '\.setLayout\(' src/

Length of output: 135

🏁 Script executed:

#!/bin/bash
# Show definitions of setTheme and setLayout with context
rg -nC5 "setTheme" src/
rg -nC5 "setLayout" src/

Length of output: 2747

🏁 Script executed:

#!/bin/bash
# Show definitions of setStyle and setStructure with context
rg -nC5 "setStyle" src/models/Template.js
rg -nC5 "setStructure" src/models/Template.js

Length of output: 940

🏁 Script executed:

#!/bin/bash
# Show quotesService.js with context
sed -n '1,200p' src/api/services/quotesService.js

Length of output: 3622

🏁 Script executed:

#!/bin/bash
# Inspect generateTemplate implementation and layout definitions
sed -n '1,200p' src/utils/generateTemplate.js

Length of output: 293

Sanitize layout CSS and HTML structure before injection

In src/common/getTemplate.js, you’re inserting template.css and template.structure directly into the SVG. Wrap their outputs in an HTML‐safe escape (e.g. escapeHtml) to prevent XSS, for example:

     <style>
-      ${template.css}
+      ${escapeHtml(template.css)}
     </style>
…
-    ${template.structure}
+    ${escapeHtml(template.structure)}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/common/getTemplate.js around lines 18 to 41, template.css and
template.structure are injected directly into the SVG which allows XSS; sanitize
or escape both values before embedding them. Replace direct insertion with
sanitizedCss and sanitizedStructure produced by a trusted sanitizer (or an
escapeHtml for simple escaping) that removes/neutralizes script tags, event
handlers, javascript: URIs and unsafe CSS (or use a well-maintained library like
DOMPurify configured for HTML+SVG/CSS), then use those sanitized variables in
the template; ensure template.font.src is validated/whitelisted or escaped as
well.

};

Expand Down
18 changes: 17 additions & 1 deletion src/models/Template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
const layouts = require("../layouts/layout");

class Template {
constructor() {}
constructor() {
this.width = 500;
this.height = 200;
this.fontSize = 16;}

setTheme(theme) {
this.theme = theme;
}



setData(data) {
this.quote = data.quote;
this.author = data.author;
Expand Down Expand Up @@ -38,6 +43,17 @@ class Template {
setFont(font){
this.font = font;
}
setFontSize(size) {
this.fontSize = size;
}

setWidth(width) {
this.width = width;
}

setHeight(height) {
this.height = height;
}

calculateHeight(length) {
let lines;
Expand Down