Skip to content

Commit 77b4d70

Browse files
committed
Fix Copilot review issues and add tests
- Add quote escaping to escapeHtml in CodeViewer.tsx for XSS protection - Fix string regex patterns to properly handle escaped quotes - Use template literal for paragraph wrapping in formatLargeText - Add vitest and tests for escapeHtml, formatLargeText, and string patterns
1 parent a0f3637 commit 77b4d70

7 files changed

Lines changed: 1942 additions & 2405 deletions

File tree

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { describe, it, expect } from 'vitest';
2+
3+
// Test the escapeHtml and string regex patterns used in CodeViewer
4+
// We test the logic directly since the component uses internal functions
5+
6+
describe('CodeViewer escapeHtml', () => {
7+
// Replicate the escapeHtml function from CodeViewer
8+
const escapeHtml = (str: string) => str
9+
.replace(/&/g, '&')
10+
.replace(/</g, '&lt;')
11+
.replace(/>/g, '&gt;')
12+
.replace(/"/g, '&quot;')
13+
.replace(/'/g, '&#039;');
14+
15+
it('escapes double quotes for attribute safety', () => {
16+
expect(escapeHtml('class="foo"')).toBe('class=&quot;foo&quot;');
17+
});
18+
19+
it('escapes single quotes for attribute safety', () => {
20+
expect(escapeHtml("class='foo'")).toBe("class=&#039;foo&#039;");
21+
});
22+
23+
it('escapes HTML tags', () => {
24+
expect(escapeHtml('<div>')).toBe('&lt;div&gt;');
25+
});
26+
27+
it('escapes ampersands', () => {
28+
expect(escapeHtml('a && b')).toBe('a &amp;&amp; b');
29+
});
30+
});
31+
32+
describe('CodeViewer string regex patterns', () => {
33+
// Test the improved string patterns
34+
const doubleQuotePattern = /"(?:[^"\\]|\\.)*"/;
35+
const singleQuotePattern = /'(?:[^'\\]|\\.)*'/;
36+
const backtickPattern = /`(?:[^`\\]|\\.)*`/;
37+
38+
describe('double-quoted strings', () => {
39+
it('matches simple double-quoted strings', () => {
40+
expect('"hello"'.match(doubleQuotePattern)?.[0]).toBe('"hello"');
41+
});
42+
43+
it('matches strings with escaped quotes', () => {
44+
expect('"He said \\"hello\\""'.match(doubleQuotePattern)?.[0]).toBe('"He said \\"hello\\""');
45+
});
46+
47+
it('matches strings with escaped backslashes', () => {
48+
expect('"path\\\\to\\\\file"'.match(doubleQuotePattern)?.[0]).toBe('"path\\\\to\\\\file"');
49+
});
50+
51+
it('matches empty strings', () => {
52+
expect('""'.match(doubleQuotePattern)?.[0]).toBe('""');
53+
});
54+
});
55+
56+
describe('single-quoted strings', () => {
57+
it('matches simple single-quoted strings', () => {
58+
expect("'hello'".match(singleQuotePattern)?.[0]).toBe("'hello'");
59+
});
60+
61+
it('matches strings with escaped quotes', () => {
62+
expect("'It\\'s fine'".match(singleQuotePattern)?.[0]).toBe("'It\\'s fine'");
63+
});
64+
65+
it('matches empty strings', () => {
66+
expect("''".match(singleQuotePattern)?.[0]).toBe("''");
67+
});
68+
});
69+
70+
describe('backtick strings', () => {
71+
it('matches simple backtick strings', () => {
72+
expect('`hello`'.match(backtickPattern)?.[0]).toBe('`hello`');
73+
});
74+
75+
it('matches strings with escaped backticks', () => {
76+
expect('`use \\`code\\``'.match(backtickPattern)?.[0]).toBe('`use \\`code\\``');
77+
});
78+
79+
it('matches empty strings', () => {
80+
expect('``'.match(backtickPattern)?.[0]).toBe('``');
81+
});
82+
});
83+
});

web/app/components/CodeViewer.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,16 @@ export function CodeViewer({ code, fileName, language }: CodeViewerProps) {
8888
const escapeHtml = (str: string) => str
8989
.replace(/&/g, '&amp;')
9090
.replace(/</g, '&lt;')
91-
.replace(/>/g, '&gt;');
91+
.replace(/>/g, '&gt;')
92+
.replace(/"/g, '&quot;')
93+
.replace(/'/g, '&#039;');
9294

9395
// Define token patterns with priorities (first match wins)
9496
// Order matters: strings and comments first to avoid highlighting inside them
9597
const tokenPatterns = [
96-
{ regex: /(["'`])(?:(?=(\\?))\2.)*?\1/, className: 'text-green-400' }, // strings
98+
{ regex: /"(?:[^"\\]|\\.)*"/, className: 'text-green-400' }, // double-quoted strings
99+
{ regex: /'(?:[^'\\]|\\.)*'/, className: 'text-green-400' }, // single-quoted strings
100+
{ regex: /`(?:[^`\\]|\\.)*`/, className: 'text-green-400' }, // backtick strings
97101
{ regex: /\/\/.*$/, className: 'text-gray-500 italic' }, // single-line comments
98102
{ regex: /\/\*[\s\S]*?\*\//, className: 'text-gray-500 italic' }, // multi-line comments
99103
{ regex: /#.*$/, className: 'text-gray-500 italic' }, // hash comments

web/app/utils/formatters.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { escapeHtml, formatLargeText } from './formatters';
3+
4+
describe('escapeHtml', () => {
5+
it('escapes ampersands', () => {
6+
expect(escapeHtml('a & b')).toBe('a &amp; b');
7+
});
8+
9+
it('escapes less than', () => {
10+
expect(escapeHtml('a < b')).toBe('a &lt; b');
11+
});
12+
13+
it('escapes greater than', () => {
14+
expect(escapeHtml('a > b')).toBe('a &gt; b');
15+
});
16+
17+
it('escapes double quotes', () => {
18+
expect(escapeHtml('He said "hello"')).toBe('He said &quot;hello&quot;');
19+
});
20+
21+
it('escapes single quotes', () => {
22+
expect(escapeHtml("It's fine")).toBe("It&#039;s fine");
23+
});
24+
25+
it('escapes all special characters together', () => {
26+
expect(escapeHtml('<script>"alert(\'xss\')&"</script>')).toBe(
27+
'&lt;script&gt;&quot;alert(&#039;xss&#039;)&amp;&quot;&lt;/script&gt;'
28+
);
29+
});
30+
});
31+
32+
describe('formatLargeText', () => {
33+
it('returns empty string for empty input', () => {
34+
expect(formatLargeText('')).toBe('');
35+
});
36+
37+
it('wraps simple text in paragraph tags', () => {
38+
expect(formatLargeText('Hello world')).toBe('<p>Hello world</p>');
39+
});
40+
41+
it('converts single newlines to br tags', () => {
42+
expect(formatLargeText('Line1\nLine2')).toBe('<p>Line1<br>Line2</p>');
43+
});
44+
45+
it('converts double newlines to paragraph breaks with proper nesting', () => {
46+
const result = formatLargeText('Line1\n\nLine2');
47+
expect(result).toBe('<p>Line1</p><p class="mt-3">Line2</p>');
48+
});
49+
50+
it('handles multiple paragraph breaks correctly', () => {
51+
const result = formatLargeText('Para1\n\nPara2\n\nPara3');
52+
expect(result).toBe('<p>Para1</p><p class="mt-3">Para2</p><p class="mt-3">Para3</p>');
53+
});
54+
55+
it('escapes HTML in the input', () => {
56+
const result = formatLargeText('<script>alert("xss")</script>');
57+
expect(result).toContain('&lt;script&gt;');
58+
expect(result).not.toContain('<script>');
59+
});
60+
61+
it('formats inline code with backticks', () => {
62+
const result = formatLargeText('Use `code` here');
63+
expect(result).toContain('<code');
64+
expect(result).toContain('>code</code>');
65+
});
66+
67+
it('formats bold text', () => {
68+
const result = formatLargeText('This is **bold** text');
69+
expect(result).toContain('<strong>bold</strong>');
70+
});
71+
72+
it('formats italic text', () => {
73+
const result = formatLargeText('This is *italic* text');
74+
expect(result).toContain('<em>italic</em>');
75+
});
76+
});

web/app/utils/formatters.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export function formatLargeText(text: string): string {
5555
const escaped = escapeHtml(text);
5656

5757
// Simple, safe formatting - just handle line breaks and basic markdown
58-
return escaped
58+
const formatted = escaped
5959
// Preserve existing double line breaks as paragraph breaks
6060
.replace(/\n\n/g, '</p><p class="mt-3">')
6161
// Convert single line breaks to <br> tags
@@ -65,9 +65,10 @@ export function formatLargeText(text: string): string {
6565
// Format bold text
6666
.replace(/\*\*([^*]+)\*\*/g, '<strong>$1</strong>')
6767
// Format italic text
68-
.replace(/\*([^*]+)\*/g, '<em>$1</em>')
69-
// Wrap in paragraph
70-
.replace(/^(.*)$/, '<p>$1</p>');
68+
.replace(/\*([^*]+)\*/g, '<em>$1</em>');
69+
70+
// Wrap in paragraph tags
71+
return `<p>${formatted}</p>`;
7172
}
7273

7374
/**

0 commit comments

Comments
 (0)