Skip to content

fix(render): encode local source URLs for imgproxy #1642

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hsusul wants to merge 2 commits into
base: master
Choose a base branch
from
+53 −43
Open

fix(render): encode local source URLs for imgproxy #1642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5777c06
test(render): cover local filenames with imgproxy delimiters
hsusul Nov 1, 2026
079e8ca
fix(render): encode local source URLs for imgproxy
hsusul Nov 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 45 additions & 35 deletions src/storage/renderer/image.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>((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<void>((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<string | undefined> = []
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<void>((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<void>((resolve) => server.close(() => resolve()))
}
}
})
)

it('fetches the transformed image with native fetch and maps the streamed response metadata', async () => {
const fetchMock = stubSuccessfulImageFetch({
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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: {
Expand All @@ -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: {
Expand Down Expand Up @@ -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: {
Expand All @@ -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: {
Expand Down Expand Up @@ -1559,15 +1569,15 @@ 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 })
)
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: {
Expand Down Expand Up @@ -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), {
Expand Down
2 changes: 1 addition & 1 deletion src/storage/renderer/image.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ export class ImageRenderer extends Renderer {
...transformations,
...transformLimits,
'plain',
privateURL.startsWith('local://') ? privateURL : encodeURIComponent(privateURL),
encodeURIComponent(privateURL),
]
let assetBody: Readable | undefined

Expand Down
14 changes: 7 additions & 7 deletions src/test/render-routes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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) })
)
}
Expand Down Expand Up @@ -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),
})
Expand Down Expand Up @@ -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),
})
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
})
Expand Down