feat: implement dynamic google font support via URL parameter#96
Conversation
|
@Animesh-86 is attempting to deploy a commit to the jhasourav07's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR adds support for selecting a Google Font dynamically via the font URL parameter, injecting a corresponding @import into the generated SVG’s <style> and applying the requested font-family.
Changes:
- Extend SVG generation to support non-predefined
fontvalues by generating a Google Fonts@importand using the font name infont-family. - Update unit tests to cover dynamic font import generation and space-to-
+conversion in font family URLs. - Document the new
fontURL parameter and add a README example.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| README.md | Documents the new font parameter and adds an example usage link. |
| lib/svg/generator.ts | Implements dynamic Google Fonts @import and updates font selection logic. |
| lib/svg/generator.test.ts | Adds test cases for dynamic Google Fonts import behavior. |
Comments suppressed due to low confidence (3)
lib/svg/generator.ts:67
params.fontis interpolated directly into CSS (font-family: "${params.font}"...) without escaping/validation. A crafted font value containing quotes, semicolons, or</style>can break the style block and potentially inject markup/script into the generated SVG. Please sanitize the font name to an allowed character set (e.g., letters/numbers/spaces/hyphens) and/or escape it before embedding (e.g., build the CSS string literal safely).
const selectedFont = isPredefinedFont
? FONT_MAP[params.font!.toLowerCase()]
: params.font
? `"${params.font}", sans-serif`
: null;
lib/svg/generator.ts:138
- The dynamic Google Fonts
@importbuilds the URL fromparams.fontby only replacing whitespace with+. This doesn’t URL-encode other characters and also doesn’t guard against quote/newline injection into theurl('...')token. Pleasetrim()and URL-encode the family value (e.g., viaencodeURIComponentand then converting%20to+if desired), and reject/strip disallowed characters so invalid/malicious font names fall back to the default theme fonts instead of being embedded verbatim.
const googleFontsImport =
params.font && !isPredefinedFont
? `@import url('https://fonts.googleapis.com/css2?family=${params.font.replace(
/\s+/g,
'+'
)}&display=swap');`
: '';
lib/svg/generator.ts:67
- This change removes the previous “unknown font => default font stack” behavior: any unrecognized
fontvalue now becomes afont-familyand triggers a Google Fonts import. That doesn’t implement the stated “fallback mechanism” for invalid/unavailable fonts and can change rendering unexpectedly. Consider validating the font name format (and/or keeping a default fallback when the name is invalid) so genuinely invalid inputs don’t alter the SVG’s typography/imports.
const isPredefinedFont = params.font && FONT_MAP[params.font.toLowerCase()];
const selectedFont = isPredefinedFont
? FONT_MAP[params.font!.toLowerCase()]
: params.font
? `"${params.font}", sans-serif`
: null;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isPredefinedFont = params.font && FONT_MAP[params.font.toLowerCase()]; | ||
| const selectedFont = isPredefinedFont | ||
| ? FONT_MAP[params.font!.toLowerCase()] |
| it('supports dynamic Google Fonts for non-predefined fonts', () => { | ||
| const svg = generateSVG( | ||
| mockStats, | ||
| { user: 'avi', font: 'invalidfont' } as unknown as BadgeParams, | ||
| { user: 'avi', font: 'Inter' } as unknown as BadgeParams, | ||
| mockCalendar | ||
| ); | ||
| expect(svg).toContain('JetBrains Mono'); | ||
|
|
||
| expect(svg).toContain('@import url(\'https://fonts.googleapis.com/css2?family=Inter&display=swap\');'); | ||
| expect(svg).toContain('font-family: "Inter", sans-serif;'); | ||
| }); |
| | `speed` | `string` | No | `8s` | Radar scan animation duration (e.g. `4s`, `12s`) | | ||
| | `scale` | `string` | No | `linear` | Tower height scaling: `linear` or `log` (logarithmic) | | ||
| | `font` | `string` | No | Theme default | Any **Google Font** name (e.g., `Orbitron`, `Inter`) | | ||
| | `refresh` | `boolean` | No | `false` | Bypass cache for real-time data | |
f429275 to
b2e00e2
Compare
|
Can you review this PR? |
| const sanitizeFont = (name: string) => name.replace(/[^a-zA-Z0-9\s-]/g, '').trim(); | ||
| const sanitizedFont = params.font ? sanitizeFont(params.font) : null; | ||
|
|
||
| const isPredefinedFont = sanitizedFont && FONT_MAP[sanitizedFont.toLowerCase()]; |
There was a problem hiding this comment.
Consider splitting isPredefinedFont into
const predefinedFont = ...;
const isPredefinedFont = Boolean(predefinedFont);
for clarity.
Use predefinedFont when building selectedFont.
| | `radius` | `number` | No | `8` | Border corner radius in pixels | | ||
| | `speed` | `string` | No | `8s` | Radar scan animation duration (e.g. `4s`, `12s`) | | ||
| | `scale` | `string` | No | `linear` | Tower height scaling: `linear` or `log` (logarithmic) | | ||
| | `font` | `string` | No | Theme default | Any **Google Font** name (e.g., `Orbitron`, `Inter`) | |
There was a problem hiding this comment.
The default should reflect “CommitPulse default typography” rather than “Theme default”, as themes do not set fonts and the actual fallback is hardcoded.
|
Good job adding sanitization! For better safety and clarity, kindly
Also, consider adding a test that confirms malicious font names don’t inject CSS/imports and always use the fallback stack. I have added some comments in the files, rest all looks good |
|
Hi @harxhe, thank you for the detailed review and the positive feedback on the sanitization! Those are great suggestions. I will refactor the font detection logic for better clarity, update the README wording to 'CommitPulse default typography', and add a specific test case to verify that invalid/malicious inputs correctly fall back to the default stack without extra imports. I'll push the updates shortly. Thanks again! |
b2e00e2 to
91b8173
Compare
…ated documentation
91b8173 to
3351ce6
Compare
Hi @harxhe, I've updated the PR based on your feedback:
All tests are passing. Thanks for the guidance! |
|
Hey can you fix the merge conflict nicely so I can merge it? |
c36136c to
87646c0
Compare
|
@JhaSourav07 Fixed the merge conflicts and synced everything cleanly with upstream/main. It should be ready to merge now. |
|
Thanks for the contribution @harxhe |
|
Thanks for contributing to CommitPulse 🚀 To make collaboration, PR reviews, issue coordination, and future contributions smoother, we highly recommend joining the CommitPulse Discord community: Inside the server you can: Most contributor coordination happens there, so joining will make the contribution experience much smoother 💜 |
…injection The escapeXML function escaped ampersand, angle brackets, double quotes, and single quotes, but not backticks. Backticks can break out of SVG attributes when interpolated into template literals, enabling layout injection attacks. Add backtick -> &JhaSourav07#96; escaping to close this gap. Fixes JhaSourav07#5264
…injection The escapeXML function escaped ampersand, angle brackets, double quotes, and single quotes, but not backticks. Backticks can break out of SVG attributes when interpolated into template literals, enabling layout injection attacks. Add backtick -> &JhaSourav07#96; escaping to close this gap. Fixes JhaSourav07#5264
## What does this PR do?
Adds a `CHANGELOG.md` file at the project root to track CommitPulse's
feature history and ongoing development milestones.
## Checklist
- [x] I have read CONTRIBUTING.md
- [x] npm run lint passes
- [x] npm run format passes
- [x] npm run test passes (22 pre-existing test file failures confirmed
present on clean main branch before any changes — all failures are
in PNG route worker timeout tests unrelated to this docs-only PR.
Verified by running `git stash` + `npm run test` on upstream main.)
- [x] No source files modified — docs-only PR (CHANGELOG.md + README.md)
## Files changed
- `CHANGELOG.md` — new file following keepachangelog.com/en/1.1.0 format
- `README.md` — added changelog badge to the existing badge row
## Evidence for changelog entries
All entries are sourced from verifiable repository artifacts:
- Ghost City mode, ?grace=, UTC sync — README.md
- 20 theme presets — THEMES.md
- ?year= — PR #94
- ?font= — PR #96, Issue #81
- ?theme=auto — PR #102, Issue #75
- MongoDB /api/track-user — README architecture table
- .env.local.example — repo file tree
- No dates or version numbers were invented
## Related issue
Closes #5702
…injection The escapeXML function escaped ampersand, angle brackets, double quotes, and single quotes, but not backticks. Backticks can break out of SVG attributes when interpolated into template literals, enabling layout injection attacks. Add backtick -> ` escaping to close this gap. Fixes #5264
…nt SVG injection (#5795) ## Summary The \escapeXML\ function escaped ampersand, angle brackets, double quotes, and single quotes, but not backticks. Backticks can break out of SVG attributes when interpolated into template literals, enabling layout injection attacks. ## Changes - **\lib/svg/generator.ts\**: Added backtick -> \`\ escaping to \escapeXML\. ## Testing - Existing escapeXML tests pass ## Issue Fixes #5264
…rav07#96) feat: support dynamic google fonts with security sanitization and updated documentation
…ourav07#5865) ## What does this PR do? Adds a `CHANGELOG.md` file at the project root to track CommitPulse's feature history and ongoing development milestones. ## Checklist - [x] I have read CONTRIBUTING.md - [x] npm run lint passes - [x] npm run format passes - [x] npm run test passes (22 pre-existing test file failures confirmed present on clean main branch before any changes — all failures are in PNG route worker timeout tests unrelated to this docs-only PR. Verified by running `git stash` + `npm run test` on upstream main.) - [x] No source files modified — docs-only PR (CHANGELOG.md + README.md) ## Files changed - `CHANGELOG.md` — new file following keepachangelog.com/en/1.1.0 format - `README.md` — added changelog badge to the existing badge row ## Evidence for changelog entries All entries are sourced from verifiable repository artifacts: - Ghost City mode, ?grace=, UTC sync — README.md - 20 theme presets — THEMES.md - ?year= — PR JhaSourav07#94 - ?font= — PR JhaSourav07#96, Issue JhaSourav07#81 - ?theme=auto — PR JhaSourav07#102, Issue JhaSourav07#75 - MongoDB /api/track-user — README architecture table - .env.local.example — repo file tree - No dates or version numbers were invented ## Related issue Closes JhaSourav07#5702
…injection The escapeXML function escaped ampersand, angle brackets, double quotes, and single quotes, but not backticks. Backticks can break out of SVG attributes when interpolated into template literals, enabling layout injection attacks. Add backtick -> &JhaSourav07#96; escaping to close this gap. Fixes JhaSourav07#5264
…injection The escapeXML function escaped ampersand, angle brackets, double quotes, and single quotes, but not backticks. Backticks can break out of SVG attributes when interpolated into template literals, enabling layout injection attacks. Add backtick -> &JhaSourav07#96; escaping to close this gap. Fixes JhaSourav07#5264
…nt SVG injection (JhaSourav07#5795) ## Summary The \escapeXML\ function escaped ampersand, angle brackets, double quotes, and single quotes, but not backticks. Backticks can break out of SVG attributes when interpolated into template literals, enabling layout injection attacks. ## Changes - **\lib/svg/generator.ts\**: Added backtick -> \&JhaSourav07#96;\ escaping to \escapeXML\. ## Testing - Existing escapeXML tests pass ## Issue Fixes JhaSourav07#5264
Description
Fixes #81
I have implemented support for dynamic Google Fonts. This allows users to pass any Google Font name via the font URL parameter, which is then dynamically imported into the SVG's style block. It includes proper URL encoding for font names with spaces and a fallback mechanism to maintain existing font support.
Pillar
Visual Preview
Checklist before requesting a review:
CONTRIBUTING.mdfile.localhost:3000/api/streak?user=YOUR_USERNAME).npm run formatandnpm run lintlocally and resolved all errors (CI will fail otherwise).feat(themes): ...,fix(calculate): ...).README.mdif I added a new theme or URL parameter.