Skip to content

Commit 6d3c54c

Browse files
Copilotmikebarkmin
andcommitted
Address code review feedback: improve security and code quality
- Extract font size mapping logic to avoid duplication - Use matchAll instead of exec for regex parsing - Add URL validation to prevent XSS attacks in image captions Co-authored-by: mikebarkmin <2592379+mikebarkmin@users.noreply.github.com>
1 parent 4efdb72 commit 6d3c54c

2 files changed

Lines changed: 46 additions & 20 deletions

File tree

packages/learningmap/src/fontSizes.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,18 @@ export const FONT_SIZE_VALUES: Record<FontSizeOption, number> = {
1111

1212
export const DEFAULT_FONT_SIZE: FontSizeOption = "M";
1313

14+
// Helper function to map numeric value to closest font size option
15+
function mapNumericToOption(value: number): FontSizeOption {
16+
if (value <= 10) return "S";
17+
if (value <= 14) return "M";
18+
if (value <= 18) return "L";
19+
return "XL";
20+
}
21+
1422
export function getFontSizeValue(size?: number | FontSizeOption): number {
1523
if (typeof size === "number") {
1624
// Convert old numeric values to closest size
17-
if (size <= 10) return FONT_SIZE_VALUES.S;
18-
if (size <= 14) return FONT_SIZE_VALUES.M;
19-
if (size <= 18) return FONT_SIZE_VALUES.L;
20-
return FONT_SIZE_VALUES.XL;
25+
return FONT_SIZE_VALUES[mapNumericToOption(size)];
2126
}
2227
if (size && size in FONT_SIZE_VALUES) {
2328
return FONT_SIZE_VALUES[size as FontSizeOption];
@@ -27,9 +32,5 @@ export function getFontSizeValue(size?: number | FontSizeOption): number {
2732

2833
export function getFontSizeOption(value?: number): FontSizeOption {
2934
if (!value) return DEFAULT_FONT_SIZE;
30-
// Find closest match
31-
if (value <= 10) return "S";
32-
if (value <= 14) return "M";
33-
if (value <= 18) return "L";
34-
return "XL";
35+
return mapNumericToOption(value);
3536
}

packages/learningmap/src/nodes/ImageNode.tsx

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,51 @@
11
import { Node, NodeResizer } from "@xyflow/react";
22
import { ImageNodeData } from "../types";
33

4+
// Validate URL to prevent XSS attacks
5+
function isValidUrl(url: string): boolean {
6+
try {
7+
const parsedUrl = new URL(url);
8+
// Only allow http, https, and mailto protocols
9+
return ['http:', 'https:', 'mailto:'].includes(parsedUrl.protocol);
10+
} catch {
11+
return false;
12+
}
13+
}
14+
415
// Simple markdown link parser for captions
516
function parseMarkdownLinks(text: string): React.ReactNode[] {
617
const parts: React.ReactNode[] = [];
718
const linkRegex = /\[([^\]]+)\]\(([^)]+)\)/g;
19+
20+
// Use matchAll instead of exec to avoid potential infinite loops
21+
const matches = Array.from(text.matchAll(linkRegex));
822
let lastIndex = 0;
9-
let match;
1023

11-
while ((match = linkRegex.exec(text)) !== null) {
24+
matches.forEach((match, index) => {
1225
// Add text before the link
13-
if (match.index > lastIndex) {
26+
if (match.index !== undefined && match.index > lastIndex) {
1427
parts.push(text.substring(lastIndex, match.index));
1528
}
16-
// Add the link
17-
parts.push(
18-
<a key={match.index} href={match[2]} target="_blank" rel="noopener noreferrer" style={{ color: "#3b82f6", textDecoration: "underline" }}>
19-
{match[1]}
20-
</a>
21-
);
22-
lastIndex = match.index + match[0].length;
23-
}
29+
30+
// Add the link only if URL is valid
31+
const linkText = match[1];
32+
const linkUrl = match[2];
33+
34+
if (isValidUrl(linkUrl)) {
35+
parts.push(
36+
<a key={index} href={linkUrl} target="_blank" rel="noopener noreferrer" style={{ color: "#3b82f6", textDecoration: "underline" }}>
37+
{linkText}
38+
</a>
39+
);
40+
} else {
41+
// If URL is invalid, just show the text
42+
parts.push(`[${linkText}](${linkUrl})`);
43+
}
44+
45+
if (match.index !== undefined) {
46+
lastIndex = match.index + match[0].length;
47+
}
48+
});
2449

2550
// Add remaining text
2651
if (lastIndex < text.length) {

0 commit comments

Comments
 (0)