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

[Bolu123-sudo]

Fixes #334

This PR adds three new optional query parameters:
fontSize (number, px)
width (number, px)
height (number, px)

Changes:

• Parsed the new parameters in quotesController.js
• Passed them through quotesService.js
• Added default values in Template.js
• Applied them dynamically in getTemplate.js

Tested locally at:
http://localhost:3004/?fontSize=30&width=600&height=300

Summary by CodeRabbit

• New Features

  • Customize generated SVGs via query parameters: width, height, and fontSize.
  • Consistent SVG sizing with explicit dimensions and font embedding for improved text rendering.

• Bug Fixes

  • Sanitized background image URLs to prevent HTML injection.
  • More reliable layout by applying validated numeric parameters only when valid.

• Chores

  • Added IDE project/module configuration and VCS mapping.
  • Updated ignore rules to exclude common IDE artifacts.

[vercel]

@Bolu123-sudo is attempting to deploy a commit to the shravan20's projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes Check	⚠️ Warning	The pull request includes multiple IDE configuration files and related .idea folder changes that are unrelated to the feature for customizable SVG dimensions and introduce out-of-scope project metadata.	Remove or isolate the .idea directory and its configuration changes from this pull request so that only the code implementing the SVG customization parameters is included.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title clearly and concisely describes the addition of optional fontSize, width, and height parameters for SVG customization and correctly references the issue being fixed without extraneous detail.
Linked Issues Check	✅ Passed	The code introduces parsing of fontSize, width, and height in the controller, propagates these through the service, updates the template model with defaults and setters, and applies the parameters in the SVG output, fully satisfying the customization objectives of issue #334.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

♻️ Duplicate comments (2)

.idea/modules.xml (1)

1-8: IDE configuration files should not be committed.

This file references the CPP_MODULE configuration, perpetuating the IDE mismatch issue. See previous comments about removing IDE configuration from version control.

.idea/vcs.xml (1)

1-6: IDE-specific VCS configuration.

While this Git VCS mapping is less problematic than the C++ configurations, it's still IDE-specific metadata that should be excluded from version control. See comments on .idea/.gitignore.

🧹 Nitpick comments (4)

.idea/.gitignore (1)

1-8: Reconsider committing IDE configuration files.

The .idea directory contains IDE-specific metadata that can vary between developers and cause merge conflicts. This is particularly concerning because:

1. The presence of C++ inspections (in editor.xml) doesn't match this JavaScript/Node.js project
2. IDE configuration is typically user-specific and shouldn't be shared via version control
3. The root .gitignore should exclude .idea/ entirely rather than selectively ignoring files within it

Apply this diff to the root .gitignore to exclude all IDE files:

+# IDE files
+.idea/

Then remove all .idea directory files from version control.

src/api/services/quotesService.js (1)

61-63: Add validation for positive values and reasonable ranges.

While Number.isFinite correctly rejects NaN and Infinity, the code doesn't validate that these dimensions are positive or within reasonable bounds. This could lead to issues:

• Negative values: fontSize=-10, width=-500 would pass validation but break rendering
• Extreme values: fontSize=999999, width=1000000 could cause performance issues or memory exhaustion
• The Number() conversion is redundant since toNum already returns a number

Apply this diff to add bounds checking:

-    if (Number.isFinite(fontSize)) template.fontSize = Number(fontSize);
-    if (Number.isFinite(width))    template.width    = Number(width);
-    if (Number.isFinite(height))   template.height   = Number(height);
+    // Validate and apply custom dimensions with reasonable bounds
+    if (Number.isFinite(fontSize) && fontSize > 0 && fontSize <= 200) {
+      template.fontSize = fontSize;
+    }
+    if (Number.isFinite(width) && width > 0 && width <= 2000) {
+      template.width = width;
+    }
+    if (Number.isFinite(height) && height > 0 && height <= 2000) {
+      template.height = height;
+    }

Adjust the maximum bounds based on your performance and use case requirements.

src/common/getTemplate.js (1)

5-7: Consider extracting default values as named constants.

The default dimensions (700, 200, 16) are magic numbers scattered in the code. For maintainability, consider defining these as constants at the module level.

+// Default SVG dimensions
+const DEFAULT_WIDTH = 700;
+const DEFAULT_HEIGHT = 200;
+const DEFAULT_FONT_SIZE = 16;
+
 const getTemplate = (template) => {
     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 svgWidth = Number.isFinite(template.width) ? template.width : DEFAULT_WIDTH;
+    const svgHeight = Number.isFinite(template.height) ? template.height : DEFAULT_HEIGHT;
+    const textFontSize = Number.isFinite(template.fontSize) ? template.fontSize : DEFAULT_FONT_SIZE;

src/models/Template.js (1)

46-56: Consider adding input validation.

The new setters don't validate inputs. Consider adding checks to ensure:

• Values are positive numbers
• Values are within reasonable bounds (e.g., fontSize > 0, width/height not excessively large)

This would prevent potential rendering issues or unexpected behavior from invalid query parameters.

Example validation for setFontSize:

setFontSize(size) {
  if (typeof size !== 'number' || size <= 0) {
    throw new Error('fontSize must be a positive number');
  }
  this.fontSize = size;
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc57607 and e245415.

📒 Files selected for processing (9)

• .idea/.gitignore (1 hunks)
• .idea/editor.xml (1 hunks)
• .idea/github-readme-quotes.iml (1 hunks)
• .idea/modules.xml (1 hunks)
• .idea/vcs.xml (1 hunks)
• src/api/controllers/quotesController.js (2 hunks)
• src/api/services/quotesService.js (2 hunks)
• src/common/getTemplate.js (1 hunks)
• src/models/Template.js (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

src/api/services/quotesService.js (1)

src/api/controllers/quotesController.js (3)

• fontSize (43-43)
• width (44-44)
• height (45-45)

src/common/getTemplate.js (2)

src/api/controllers/quotesController.js (2)

• escapeHtml (88-88)
• safeUrl (89-89)

frontend/src/util/template/getTemplate.js (1)

• backgroundImageLayer (3-13)

src/models/Template.js (1)

src/api/controllers/quotesController.js (3)

• theme (10-10)
• width (44-44)
• height (45-45)

🔇 Additional comments (6)

src/api/services/quotesService.js (1)

18-18: LGTM!

The destructuring correctly extracts the new fontSize, width, and height parameters from the quote object.

src/api/controllers/quotesController.js (1)

57-60: LGTM!

The new parameters are correctly added to the quoteObject for downstream processing.

src/common/getTemplate.js (2)

2-2: Good security practice: input sanitization.

The addition of escape-html to sanitize template.bgImage is a solid security improvement that prevents potential XSS attacks via malicious URLs.

---

9-16: LGTM: Sanitized URL and dynamic dimensions.

The background image layer correctly uses the sanitized URL and the new dynamic svgWidth and svgHeight values, allowing for flexible SVG sizing.

src/models/Template.js (2)

4-7: Remove outdated height override warning
No .setHeight calls exist—height is only computed in setLayout via calculateHeight, so the described conflict cannot occur.

Likely an incorrect or invalid review comment.

---

7-7: Fix syntax error: closing brace on same line.

The closing brace is incorrectly placed on the same line as the statement, causing a syntax error.

Apply this diff to fix the formatting:

-    this.fontSize = 16;}
+    this.fontSize = 16;
+  }

Likely an incorrect or invalid review comment.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

482 lines of C++ inspections for a JavaScript project.

This file contains exclusively C++ code inspections (CppSmartPointerVsMakeFunction, CppCStyleCast, CppDFA*, etc.) which are completely irrelevant for this Node.js/JavaScript codebase. This will:

• Degrade IDE performance with unnecessary analysis
• Generate false warnings or miss JavaScript-specific issues
• Confuse developers expecting JavaScript tooling

Remove this file along with all other .idea directory contents. If shared IDE configuration is needed, consider using EditorConfig (.editorconfig) which is IDE-agnostic and language-appropriate.

🤖 Prompt for AI Agents

In .idea/editor.xml lines 1-482 the file contains 482 lines of C++ inspection
settings irrelevant to this Node.js/JavaScript repo; remove this file and all
.idea directory contents from the repository, add a .gitignore rule to exclude
/.idea/* (or remove from VCS if already committed), and if shared editor
settings are required replace them with an EditorConfig (.editorconfig) or
repo-appropriate IDE-agnostic configuration instead of committing IDE-specific
.idea XML files.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Module type mismatch: CPP_MODULE for a JavaScript project.

This configuration defines a C++ module (type="CPP_MODULE") for a Node.js/JavaScript project. This mismatch will:

• Cause the IDE to apply C++ analysis and inspections to JavaScript code
• Result in irrelevant warnings and missing JavaScript-specific tooling
• Create confusion for developers

Either use a JavaScript-appropriate IDE (e.g., WebStorm, VS Code) or remove these configuration files entirely.

🤖 Prompt for AI Agents

.idea/github-readme-quotes.iml lines 1-8: The module file declares
type="CPP_MODULE" which is incorrect for this Node.js/JavaScript project; remove
or replace it with a JavaScript-appropriate module config. Delete the .iml file
from the repo (or update its module type to a JS/Node-compatible configuration
if your IDE requires one), and add .idea/*.iml to .gitignore to prevent
committing IDE-specific C++ configs accidentally; if using a JetBrains JS IDE,
re-create the project config through the IDE so it generates the correct
settings.

[coderabbitai]

🛠️ 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.

[coderabbitai]

⚠️ 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.

[coderabbitai]

⚠️ 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.

[zhravan]

Please remove:

• .idea files

Add this change to UI as well