Skip to content

Commit 9ed99b8

Browse files
jaffrepaulgithub-advanced-security[bot]getsantry[bot]
authored
Fix image path issue & add test coverage for new lightbox (#14616)
<!-- Use this checklist to make sure your PR is ready for merge. You may delete any sections you don't need. --> ## DESCRIBE YOUR PR Adding full component testing after finding a malformed image URL in prod. ## IS YOUR CHANGE URGENT? Help us prioritize incoming PRs by letting us know when the change needs to go live. - [ ] Urgent deadline (GA date, etc.): <!-- ENTER DATE HERE --> - [ ] Other deadline: <!-- ENTER DATE HERE --> - [ ] None: Not urgent, can wait up to 1 week+ ## SLA - Teamwork makes the dream work, so please add a reviewer to your PRs. - Please give the docs team up to 1 week to review your PR unless you've added an urgent due date to it. Thanks in advance for your help! ## PRE-MERGE CHECKLIST *Make sure you've checked the following before merging your changes:* - [ ] Checked Vercel preview for correctness, including links - [ ] PR was reviewed and approved by any necessary SMEs (subject matter experts) - [ ] PR was reviewed and approved by a member of the [Sentry docs team](https://github.com/orgs/getsentry/teams/docs) ## LEGAL BOILERPLATE <!-- Sentry employees and contractors can delete or ignore this section. --> Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms. ## EXTRA RESOURCES - [Sentry Docs contributor guide](https://docs.sentry.io/contributing/) --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
1 parent e34aa43 commit 9ed99b8

File tree

5 files changed

+448
-13
lines changed

5 files changed

+448
-13
lines changed
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
import {beforeEach, describe, expect, it, vi} from 'vitest';
2+
3+
import {
4+
cleanUrl,
5+
DIMENSION_PATTERN,
6+
extractHash,
7+
parseDimensionsFromHash,
8+
} from 'sentry-docs/components/docImage';
9+
import {serverContext} from 'sentry-docs/serverContext';
10+
11+
// Mock the serverContext
12+
vi.mock('sentry-docs/serverContext', () => ({
13+
serverContext: vi.fn(),
14+
}));
15+
16+
// Mock the ImageLightbox component
17+
vi.mock('../imageLightbox', () => ({
18+
ImageLightbox: vi.fn(({src, alt, width, height, imgPath}) => ({
19+
type: 'ImageLightbox',
20+
props: {src, alt, width, height, imgPath},
21+
})),
22+
}));
23+
24+
const mockServerContext = serverContext as any;
25+
26+
describe('DocImage Helper Functions', () => {
27+
beforeEach(() => {
28+
mockServerContext.mockReturnValue({
29+
path: '/docs/test-page',
30+
});
31+
});
32+
33+
describe('dimension validation bounds', () => {
34+
it('should validate dimensions within acceptable range', () => {
35+
// Test actual boundary conditions used in parseDimensionsFromHash
36+
expect(parseDimensionsFromHash('/image.jpg#1x1')).toEqual([1, 1]); // minimum valid
37+
expect(parseDimensionsFromHash('/image.jpg#10000x10000')).toEqual([10000, 10000]); // maximum valid
38+
expect(parseDimensionsFromHash('/image.jpg#0x600')).toEqual([]); // below minimum
39+
expect(parseDimensionsFromHash('/image.jpg#10001x5000')).toEqual([]); // above maximum
40+
});
41+
});
42+
43+
describe('dimension pattern regex', () => {
44+
it('should match valid dimension patterns', () => {
45+
expect(DIMENSION_PATTERN.test('800x600')).toBe(true);
46+
expect(DIMENSION_PATTERN.test('1920x1080')).toBe(true);
47+
expect(DIMENSION_PATTERN.test('10000x10000')).toBe(true);
48+
});
49+
50+
it('should not match invalid dimension patterns', () => {
51+
expect(DIMENSION_PATTERN.test('800x')).toBe(false);
52+
expect(DIMENSION_PATTERN.test('x600')).toBe(false);
53+
expect(DIMENSION_PATTERN.test('800')).toBe(false);
54+
expect(DIMENSION_PATTERN.test('800x600x400')).toBe(false);
55+
expect(DIMENSION_PATTERN.test('abc800x600')).toBe(false);
56+
expect(DIMENSION_PATTERN.test('section-heading')).toBe(false);
57+
});
58+
});
59+
60+
describe('hash extraction', () => {
61+
it('should extract hash from URLs', () => {
62+
expect(extractHash('/image.jpg#800x600')).toBe('800x600');
63+
expect(extractHash('./img/issue_page.png#1200x800')).toBe('1200x800');
64+
expect(extractHash('https://example.com/image.jpg#section')).toBe('section');
65+
expect(extractHash('/image.jpg')).toBe('');
66+
});
67+
});
68+
69+
describe('dimension parsing from hash', () => {
70+
it('should parse valid dimensions from URL hash', () => {
71+
expect(parseDimensionsFromHash('/image.jpg#800x600')).toEqual([800, 600]);
72+
expect(parseDimensionsFromHash('/image.jpg#1920x1080')).toEqual([1920, 1080]);
73+
expect(parseDimensionsFromHash('/image.jpg#10000x10000')).toEqual([10000, 10000]);
74+
expect(parseDimensionsFromHash('./img/issue_page.png#1200x800')).toEqual([
75+
1200, 800,
76+
]);
77+
});
78+
79+
it('should return empty array for invalid dimensions', () => {
80+
const invalidCases = [
81+
'/image.jpg#0x600',
82+
'/image.jpg#10001x5000',
83+
'/image.jpg#800x',
84+
'/image.jpg#section-heading',
85+
'/image.jpg',
86+
'./img/error_level.png#malformed',
87+
];
88+
89+
invalidCases.forEach(url => {
90+
expect(parseDimensionsFromHash(url)).toEqual([]);
91+
});
92+
});
93+
});
94+
95+
describe('URL cleaning', () => {
96+
it('should remove dimension hashes from URLs', () => {
97+
const testCases = [
98+
{input: '/image.jpg#800x600', expected: '/image.jpg'},
99+
{
100+
input: 'https://example.com/image.jpg#1920x1080',
101+
expected: 'https://example.com/image.jpg',
102+
},
103+
{input: './img/issue_page.png#1200x800', expected: './img/issue_page.png'},
104+
{input: './img/error_level.png#32x32', expected: './img/error_level.png'},
105+
];
106+
107+
testCases.forEach(({input, expected}) => {
108+
expect(cleanUrl(input)).toBe(expected);
109+
});
110+
});
111+
112+
it('should preserve non-dimension hashes', () => {
113+
const testCases = [
114+
'./img/issue_page.png#section-heading',
115+
'/image.jpg#important-section',
116+
'/image.jpg#anchor',
117+
];
118+
119+
testCases.forEach(url => {
120+
expect(cleanUrl(url)).toBe(url);
121+
});
122+
});
123+
124+
it('should handle URLs without hashes', () => {
125+
const testCases = [
126+
'/image.jpg',
127+
'https://example.com/image.jpg',
128+
'./img/issue_page.png',
129+
];
130+
131+
testCases.forEach(url => {
132+
expect(cleanUrl(url)).toBe(url);
133+
});
134+
});
135+
});
136+
137+
describe('Issues page integration scenarios', () => {
138+
const issuesPageImages = [
139+
'./img/issue_page.png#1200x800',
140+
'./img/error_level.png#32x32',
141+
'./img/issue_sort.png#600x400',
142+
];
143+
144+
it('should handle Issues page image paths correctly', () => {
145+
issuesPageImages.forEach(path => {
146+
const hash = extractHash(path);
147+
const cleanedUrl = cleanUrl(path);
148+
const dimensions = parseDimensionsFromHash(path);
149+
150+
expect(hash).toMatch(DIMENSION_PATTERN);
151+
expect(cleanedUrl).not.toContain('#');
152+
expect(dimensions).toHaveLength(2);
153+
expect(dimensions[0]).toBeGreaterThan(0);
154+
expect(dimensions[1]).toBeGreaterThan(0);
155+
});
156+
});
157+
158+
it('should handle malformed relative paths gracefully', () => {
159+
const malformedPaths = [
160+
'./img/issue_page.png#800x',
161+
'./img/error_level.png#invalid',
162+
'./img/issue_sort.png#section-anchor',
163+
];
164+
165+
malformedPaths.forEach(path => {
166+
expect(cleanUrl(path)).toBe(path); // Should not clean non-dimension hashes
167+
expect(parseDimensionsFromHash(path)).toEqual([]); // Should return empty array
168+
});
169+
});
170+
});
171+
});

src/components/docImage.tsx renamed to src/components/docImage/index.tsx

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,41 +3,42 @@ import path from 'path';
33
import {isExternalImage} from 'sentry-docs/config/images';
44
import {serverContext} from 'sentry-docs/serverContext';
55

6-
import {ImageLightbox} from './imageLightbox';
6+
import {ImageLightbox} from '../imageLightbox';
77

88
// Helper function to safely parse dimension values
99
const parseDimension = (value: string | number | undefined): number | undefined => {
10-
if (typeof value === 'number' && value > 0 && value <= 10000) return value;
10+
if (typeof value === 'number' && value > 0 && value <= MAX_DIMENSION) return value;
1111
if (typeof value === 'string') {
1212
const parsed = parseInt(value, 10);
13-
return parsed > 0 && parsed <= 10000 ? parsed : undefined;
13+
return parsed > 0 && parsed <= MAX_DIMENSION ? parsed : undefined;
1414
}
1515
return undefined;
1616
};
1717

1818
// Dimension pattern regex - used to identify dimension hashes like "800x600"
19-
const DIMENSION_PATTERN = /^(\d+)x(\d+)$/;
19+
export const DIMENSION_PATTERN = /^(\d+)x(\d+)$/;
20+
export const MAX_DIMENSION = 10000;
2021

2122
// Helper function to extract hash from URL string (works with both relative and absolute URLs)
22-
const extractHash = (url: string): string => {
23+
export const extractHash = (url: string): string => {
2324
const hashIndex = url.indexOf('#');
2425
return hashIndex !== -1 ? url.slice(hashIndex + 1) : '';
2526
};
2627

2728
// Helper function to check if a hash contains dimension information
28-
const isDimensionHash = (hash: string): boolean => {
29+
export const isDimensionHash = (hash: string): boolean => {
2930
return DIMENSION_PATTERN.test(hash);
3031
};
3132

3233
// Helper function to parse dimensions from URL hash
33-
const parseDimensionsFromHash = (url: string): number[] => {
34+
export const parseDimensionsFromHash = (url: string): number[] => {
3435
const hash = extractHash(url);
3536
const match = hash.match(DIMENSION_PATTERN);
3637

3738
if (match) {
3839
const width = parseInt(match[1], 10);
3940
const height = parseInt(match[2], 10);
40-
return width > 0 && width <= 10000 && height > 0 && height <= 10000
41+
return width > 0 && width <= MAX_DIMENSION && height > 0 && height <= MAX_DIMENSION
4142
? [width, height]
4243
: [];
4344
}
@@ -46,7 +47,7 @@ const parseDimensionsFromHash = (url: string): number[] => {
4647
};
4748

4849
// Helper function to remove dimension hash from URL while preserving fragment identifiers
49-
const cleanUrl = (url: string): string => {
50+
export const cleanUrl = (url: string): string => {
5051
const hash = extractHash(url);
5152

5253
// If no hash or hash is not a dimension pattern, return original URL
@@ -78,7 +79,9 @@ export default function DocImage({
7879
// For internal images, process the path
7980
if (!isExternal) {
8081
if (src.startsWith('./')) {
81-
finalSrc = path.join('/mdx-images', src);
82+
// Remove ./ prefix and properly join with mdx-images path
83+
const cleanSrc = src.slice(2);
84+
finalSrc = path.join('/mdx-images', cleanSrc);
8285
} else if (!src?.startsWith('/') && !src?.includes('://')) {
8386
finalSrc = `/${pagePath.join('/')}/${src}`;
8487
}

0 commit comments

Comments
 (0)