diff --git a/src/storage/renderer/image.test.ts b/src/storage/renderer/image.test.ts index f39fd4f3e..24224fcba 100644 --- a/src/storage/renderer/image.test.ts +++ b/src/storage/renderer/image.test.ts @@ -315,34 +315,44 @@ describe('ImageRenderer gravity transformations', () => { }) describe('ImageRenderer fetch client', () => { - it('streams a real image through Node fetch and the Undici 8 retry dispatcher', async () => { - const server = createServer((_req, res) => { - res.writeHead(200, { 'content-type': 'image/webp', 'content-length': '13' }) - res.end('rendered-body') - }) - await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) - vi.doUnmock('undici') - vi.stubGlobal('fetch', nativeFetch) - const fetchSpy = vi.spyOn(globalThis, 'fetch') - try { - const { ImageRenderer } = await loadRendererModule( - { imgProxyURL: `http://127.0.0.1:${(server.address() as AddressInfo).port}` }, - { mockUndici: false } - ) - const result = await new ImageRenderer(createBackend('local:///tmp/cat.png')).getAsset( - createRequest(), - createRenderOptions() - ) - await expect(readStream(result.body)).resolves.toBe('rendered-body') - } finally { - const init = fetchSpy.mock.calls[0]?.[1] as - | { dispatcher?: import('undici').Dispatcher } - | undefined - await init?.dispatcher?.destroy() - server.closeAllConnections() - await new Promise((resolve) => server.close(() => resolve())) + it.each([ + ['local:///tmp/cat.png', '/public/plain/local%3A%2F%2F%2Ftmp%2Fcat.png'], + ['local:///tmp/cat@2x.png', '/public/plain/local%3A%2F%2F%2Ftmp%2Fcat%402x.png'], + ['local:///tmp/cat@@2x.png', '/public/plain/local%3A%2F%2F%2Ftmp%2Fcat%40%402x.png'], + ])( + 'streams %s through native fetch without exposing imgproxy delimiters', + async (source, expectedPath) => { + const receivedPaths: Array = [] + const server = createServer((req, res) => { + receivedPaths.push(req.url) + res.writeHead(200, { 'content-type': 'image/webp', 'content-length': '13' }) + res.end('rendered-body') + }) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + vi.doUnmock('undici') + vi.stubGlobal('fetch', nativeFetch) + const fetchSpy = vi.spyOn(globalThis, 'fetch') + try { + const { ImageRenderer } = await loadRendererModule( + { imgProxyURL: `http://127.0.0.1:${(server.address() as AddressInfo).port}` }, + { mockUndici: false } + ) + const result = await new ImageRenderer(createBackend(source)).getAsset( + createRequest(), + createRenderOptions() + ) + await expect(readStream(result.body)).resolves.toBe('rendered-body') + expect(receivedPaths).toEqual([expectedPath]) + } finally { + const init = fetchSpy.mock.calls[0]?.[1] as + | { dispatcher?: import('undici').Dispatcher } + | undefined + await init?.dispatcher?.destroy() + server.closeAllConnections() + await new Promise((resolve) => server.close(() => resolve())) + } } - }) + ) it('fetches the transformed image with native fetch and maps the streamed response metadata', async () => { const fetchMock = stubSuccessfulImageFetch({ @@ -1275,7 +1285,7 @@ describe('ImageRenderer fetch client', () => { .get('https://imgproxy.example.test') .intercept({ method: 'GET', - path: '/base/public/plain/local:///tmp/cat.png', + path: '/base/public/plain/local%3A%2F%2F%2Ftmp%2Fcat.png', }) .reply(statusCode, body) .times(6) @@ -1435,7 +1445,7 @@ describe('ImageRenderer fetch client', () => { pool .intercept({ method: 'GET', - path: '/base/public/plain/local:///tmp/cat.png', + path: '/base/public/plain/local%3A%2F%2F%2Ftmp%2Fcat.png', }) .reply(429, 'retry later', { headers: { @@ -1445,7 +1455,7 @@ describe('ImageRenderer fetch client', () => { pool .intercept({ method: 'GET', - path: '/base/public/plain/local:///tmp/cat.png', + path: '/base/public/plain/local%3A%2F%2F%2Ftmp%2Fcat.png', }) .reply(200, 'rendered-body', { headers: { @@ -1495,7 +1505,7 @@ describe('ImageRenderer fetch client', () => { pool .intercept({ method: 'GET', - path: '/base/public/plain/local:///tmp/cat.png', + path: '/base/public/plain/local%3A%2F%2F%2Ftmp%2Fcat.png', }) .reply(429, 'retry later', { headers: { @@ -1507,7 +1517,7 @@ describe('ImageRenderer fetch client', () => { pool .intercept({ method: 'GET', - path: '/base/public/plain/local:///tmp/cat.png', + path: '/base/public/plain/local%3A%2F%2F%2Ftmp%2Fcat.png', }) .reply(200, 'rendered-body', { headers: { @@ -1559,7 +1569,7 @@ describe('ImageRenderer fetch client', () => { pool .intercept({ method: 'GET', - path: '/base/public/plain/local:///tmp/cat.png', + path: '/base/public/plain/local%3A%2F%2F%2Ftmp%2Fcat.png', }) .replyWithError( Object.assign(new Error(errorMessage), { code: errorCode, name: errorName }) @@ -1567,7 +1577,7 @@ describe('ImageRenderer fetch client', () => { pool .intercept({ method: 'GET', - path: '/base/public/plain/local:///tmp/cat.png', + path: '/base/public/plain/local%3A%2F%2F%2Ftmp%2Fcat.png', }) .reply(200, 'rendered-body', { headers: { @@ -3021,7 +3031,7 @@ describe('ImageRenderer fetch client', () => { .get('https://imgproxy.example.test') .intercept({ method: 'GET', - path: '/base/public/width:50/resizing_type:fill/plain/local:///tmp/cat.png', + path: '/base/public/width:50/resizing_type:fill/plain/local%3A%2F%2F%2Ftmp%2Fcat.png', }) .replyWithError( Object.assign(new Error(errorMessage), { diff --git a/src/storage/renderer/image.ts b/src/storage/renderer/image.ts index 5c7e0f43d..97c0eea6e 100644 --- a/src/storage/renderer/image.ts +++ b/src/storage/renderer/image.ts @@ -498,7 +498,7 @@ export class ImageRenderer extends Renderer { ...transformations, ...transformLimits, 'plain', - privateURL.startsWith('local://') ? privateURL : encodeURIComponent(privateURL), + encodeURIComponent(privateURL), ] let assetBody: Readable | undefined diff --git a/src/test/render-routes.test.ts b/src/test/render-routes.test.ts index df0689170..77692d18d 100644 --- a/src/test/render-routes.test.ts +++ b/src/test/render-routes.test.ts @@ -187,7 +187,7 @@ describe('image rendering routes', () => { expect(response.body).toBe(renderedBodyText) expect(S3Backend.prototype.privateAssetUrl).toHaveBeenCalledTimes(1) expect(getSpy).toHaveBeenCalledWith( - `/public/height:100/width:100/resizing_type:fill/plain/local:///${projectRoot}/data/sadcat.jpg`, + `/public/height:100/width:100/resizing_type:fill/plain/${encodeURIComponent(`local:///${projectRoot}/data/sadcat.jpg`)}`, expect.objectContaining({ signal: expect.any(AbortSignal), }) @@ -267,7 +267,7 @@ describe('image rendering routes', () => { expect(response.statusCode).toBe(200) expect(S3Backend.prototype.privateAssetUrl).toHaveBeenCalledTimes(1) expect(getSpy).toHaveBeenCalledWith( - `/public/height:100/width:100/resizing_type:fill/plain/local:///${projectRoot}/data/sadcat.jpg`, + `/public/height:100/width:100/resizing_type:fill/plain/${encodeURIComponent(`local:///${projectRoot}/data/sadcat.jpg`)}`, expect.objectContaining({ headers: { accept: 'image/avif,image/webp', @@ -490,7 +490,7 @@ describe('image rendering routes', () => { expect(response.statusCode).toBe(200) expect(response.body).toBe(renderedBodyText) expect(getSpy).toHaveBeenCalledWith( - `/public/height:100/width:100/resizing_type:fill/${expectedGravity}/plain/local:///${projectRoot}/data/sadcat.jpg`, + `/public/height:100/width:100/resizing_type:fill/${expectedGravity}/plain/${encodeURIComponent(`local:///${projectRoot}/data/sadcat.jpg`)}`, expect.objectContaining({ signal: expect.any(AbortSignal) }) ) } @@ -554,7 +554,7 @@ describe('image rendering routes', () => { expect(S3Backend.prototype.privateAssetUrl).toHaveBeenCalledTimes(1) const expectFormat = format === 'origin' ? '' : `/format:${format}` expect(getSpy).toHaveBeenCalledWith( - `/public/height:100/width:100/resizing_type:fill${expectFormat}/plain/local:///${projectRoot}/data/sadcat.jpg`, + `/public/height:100/width:100/resizing_type:fill${expectFormat}/plain/${encodeURIComponent(`local:///${projectRoot}/data/sadcat.jpg`)}`, expect.objectContaining({ signal: expect.any(AbortSignal), }) @@ -600,7 +600,7 @@ describe('image rendering routes', () => { expect(response.statusCode).toBe(200) expect(S3Backend.prototype.privateAssetUrl).toHaveBeenCalledTimes(1) expect(getSpy).toHaveBeenCalledWith( - `/public/height:100/width:100/resizing_type:fit/plain/local:///${projectRoot}/data/sadcat.jpg`, + `/public/height:100/width:100/resizing_type:fit/plain/${encodeURIComponent(`local:///${projectRoot}/data/sadcat.jpg`)}`, expect.objectContaining({ signal: expect.any(AbortSignal), }) @@ -652,7 +652,7 @@ describe('image rendering routes', () => { expect(response.statusCode).toBe(200) // Origin suppresses format negotiation; an unset format preserves it. expect(getSpy).toHaveBeenCalledWith( - `/public/height:100/width:100/resizing_type:fill/plain/local:///${projectRoot}/data/sadcat.jpg`, + `/public/height:100/width:100/resizing_type:fill/plain/${encodeURIComponent(`local:///${projectRoot}/data/sadcat.jpg`)}`, expect.objectContaining({ headers: expectedHeaders, signal: expect.any(AbortSignal), @@ -702,7 +702,7 @@ describe('image rendering routes', () => { expect(response.statusCode).toBe(200) expect(S3Backend.prototype.privateAssetUrl).toHaveBeenCalledTimes(1) expect(getSpy).toHaveBeenCalledWith( - `/public/height:100/width:100/resizing_type:fit/plain/local:///${projectRoot}/data/sadcat.jpg`, + `/public/height:100/width:100/resizing_type:fit/plain/${encodeURIComponent(`local:///${projectRoot}/data/sadcat.jpg`)}`, expect.objectContaining({ signal: expect.any(AbortSignal), })