Skip to content

Commit 89d126e

Browse files
refactor(imageProxy): remove redundant validation and unused code
- Remove redundant protocol check in validateImageUrl (already filtered by isExternalImageUrl) - Remove unused proxyImagesInHtml function and its tests - Update AGENTS.md with best practices for avoiding dead code and redundant validation Co-authored-by: Ido Shamun <idoshamun@users.noreply.github.com>
1 parent 5a8a8bd commit 89d126e

File tree

2 files changed

+3
-107
lines changed

2 files changed

+3
-107
lines changed

__tests__/common/imageProxy.ts

Lines changed: 0 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import {
55
validateImageUrl,
66
isExternalImageUrl,
77
getProxiedImageUrl,
8-
proxyImagesInHtml,
98
} from '../../src/common/imageProxy';
109

1110
const configureCloudinary = () => {
@@ -153,24 +152,6 @@ describe('imageProxy', () => {
153152
expect(validateImageUrl(longUrl)).toBe('URL exceeds maximum length');
154153
});
155154

156-
it('should return error for file:// protocol', () => {
157-
expect(validateImageUrl('file:///etc/passwd')).toBe(
158-
'Only HTTP and HTTPS protocols are allowed',
159-
);
160-
});
161-
162-
it('should return error for ftp:// protocol', () => {
163-
expect(validateImageUrl('ftp://example.com/image.png')).toBe(
164-
'Only HTTP and HTTPS protocols are allowed',
165-
);
166-
});
167-
168-
it('should return error for javascript: protocol', () => {
169-
expect(validateImageUrl('javascript:alert(1)')).toBe(
170-
'Only HTTP and HTTPS protocols are allowed',
171-
);
172-
});
173-
174155
it('should return error for private IP addresses', () => {
175156
expect(validateImageUrl('http://127.0.0.1/image.png')).toBe(
176157
'Private IP addresses are not allowed',
@@ -269,62 +250,4 @@ describe('imageProxy', () => {
269250
expect(result).toMatch(/s--[A-Za-z0-9_-]+--/);
270251
});
271252
});
272-
273-
describe('proxyImagesInHtml', () => {
274-
beforeEach(() => {
275-
process.env.CLOUDINARY_URL = 'cloudinary://test:test@daily-now';
276-
configureCloudinary();
277-
});
278-
279-
it('should return empty string for empty input', () => {
280-
expect(proxyImagesInHtml('')).toBe('');
281-
});
282-
283-
it('should return null for null input', () => {
284-
expect(proxyImagesInHtml(null as unknown as string)).toBe(null);
285-
});
286-
287-
it('should proxy external image URLs in img tags', () => {
288-
const html = '<p>Hello</p><img src="https://example.com/image.png">';
289-
const result = proxyImagesInHtml(html);
290-
291-
expect(result).toContain('media.daily.dev');
292-
expect(result).toContain('/image/fetch/');
293-
// The original URL is included in the Cloudinary fetch URL path (this is expected)
294-
expect(result).toContain('src="https://media.daily.dev');
295-
});
296-
297-
it('should not modify images from allowed domains', () => {
298-
const html = '<img src="https://media.daily.dev/image.png">';
299-
const result = proxyImagesInHtml(html);
300-
301-
expect(result).toBe(html);
302-
});
303-
304-
it('should handle multiple images', () => {
305-
const html =
306-
'<img src="https://example.com/1.png"><img src="https://evil.com/2.gif">';
307-
const result = proxyImagesInHtml(html);
308-
309-
// Both images should be proxied through Cloudinary
310-
expect(result.match(/media\.daily\.dev/g)?.length).toBe(2);
311-
expect(result).toContain('/image/fetch/');
312-
});
313-
314-
it('should preserve other attributes on img tags', () => {
315-
const html =
316-
'<img alt="test" src="https://example.com/image.png" class="photo">';
317-
const result = proxyImagesInHtml(html);
318-
319-
expect(result).toContain('alt="test"');
320-
expect(result).toContain('class="photo"');
321-
});
322-
323-
it('should handle img tags with single quotes', () => {
324-
const html = "<img src='https://example.com/image.png'>";
325-
const result = proxyImagesInHtml(html);
326-
327-
expect(result).toContain('media.daily.dev');
328-
});
329-
});
330253
});

src/common/imageProxy.ts

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ export function isAllowedDomain(url: string): boolean {
6262
/**
6363
* Validates that a URL is safe to proxy
6464
* Returns an error message if invalid, or null if valid
65+
*
66+
* Note: This assumes the URL is already http/https since it's called after
67+
* isExternalImageUrl check which filters out non-http/https URLs.
6568
*/
6669
export function validateImageUrl(url: string): string | null {
6770
// Check URL length
@@ -72,11 +75,6 @@ export function validateImageUrl(url: string): string | null {
7275
try {
7376
const parsedUrl = new URL(url);
7477

75-
// Only allow http and https protocols
76-
if (parsedUrl.protocol !== 'http:' && parsedUrl.protocol !== 'https:') {
77-
return 'Only HTTP and HTTPS protocols are allowed';
78-
}
79-
8078
// Block private/internal IP addresses (SSRF prevention)
8179
if (isPrivateIP(parsedUrl.hostname)) {
8280
return 'Private IP addresses are not allowed';
@@ -161,28 +159,3 @@ export function getProxiedImageUrl(externalUrl: string): string | null {
161159
return externalUrl;
162160
}
163161
}
164-
165-
/**
166-
* Processes HTML content to proxy all external image URLs
167-
* This is used as a fallback for content that wasn't processed during markdown rendering
168-
*
169-
* @param html The HTML content containing images
170-
* @returns The HTML with external image URLs replaced with proxied URLs
171-
*/
172-
export function proxyImagesInHtml(html: string): string {
173-
if (!html) {
174-
return html;
175-
}
176-
177-
// Match img tags and replace src attributes
178-
return html.replace(
179-
/<img\s+([^>]*?)src=["']([^"']+)["']([^>]*)>/gi,
180-
(match, before, src, after) => {
181-
const proxiedUrl = getProxiedImageUrl(src);
182-
if (proxiedUrl && proxiedUrl !== src) {
183-
return `<img ${before}src="${proxiedUrl}"${after}>`;
184-
}
185-
return match;
186-
},
187-
);
188-
}

0 commit comments

Comments
 (0)