Skip to content

Commit

Permalink
fix: image optimizer hangs when invalid image is requested (#33719)
Browse files Browse the repository at this point in the history
When an invalid image is requested, the 'finish' event is never triggered,
which ultimately leads to a 504 Gateway Timeout error.

This is fixed by invoking the 'callback' in `_write()`.

fix #33441



## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
  • Loading branch information
kyliau committed Jan 27, 2022
1 parent 311f66e commit c551a32
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
13 changes: 10 additions & 3 deletions packages/next/server/image-optimizer.ts
Expand Up @@ -253,8 +253,17 @@ export async function imageOptimizer(
mockRes.write = (chunk: Buffer | string) => {
resBuffers.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk))
}
mockRes._write = (chunk: Buffer | string) => {
mockRes._write = (
chunk: Buffer | string,
_encoding: string,
callback: () => void
) => {
mockRes.write(chunk)
// According to Node.js documentation, the callback MUST be invoked to signal that
// the write completed successfully. If this callback is not invoked, the 'finish' event
// will not be emitted.
// https://nodejs.org/docs/latest-v16.x/api/stream.html#writable_writechunk-encoding-callback
callback()
}

const mockHeaders: Record<string, string | string[]> = {}
Expand Down Expand Up @@ -290,7 +299,6 @@ export async function imageOptimizer(
await handleRequest(mockReq, mockRes, nodeUrl.parse(href, true))
await isStreamFinished
res.statusCode = mockRes.statusCode

upstreamBuffer = Buffer.concat(resBuffers)
upstreamType =
detectContentType(upstreamBuffer) || mockRes.getHeader('Content-Type')
Expand Down Expand Up @@ -328,7 +336,6 @@ export async function imageOptimizer(
)
return { finished: true }
}

if (!upstreamType.startsWith('image/')) {
res.statusCode = 400
res.end("The requested resource isn't a valid image.")
Expand Down
8 changes: 8 additions & 0 deletions test/integration/image-optimizer/test/index.test.js
Expand Up @@ -779,6 +779,14 @@ function runTests({
expect(await res.text()).toBe("The requested resource isn't a valid image.")
})

it('should error if the image file does not exist', async () => {
const query = { url: '/does_not_exist.jpg', w, q: 80 }
const opts = { headers: { accept: 'image/webp' } }
const res = await fetchViaHTTP(appPort, '/_next/image', query, opts)
expect(res.status).toBe(400)
expect(await res.text()).toBe("The requested resource isn't a valid image.")
})

it('should handle concurrent requests', async () => {
await fs.remove(imagesDir)
const query = { url: '/test.png', w, q: 80 }
Expand Down

0 comments on commit c551a32

Please sign in to comment.