mirror of
https://github.com/vercel/next.js.git
synced 2026-09-20 02:25:18 +08:00
Muhammad Zeeshan
fad317505d
- Fixes https://github.com/vercel/next.js/issues/82357 - Closes https://github.com/vercel/next.js/pull/96985 ### What? `fetchInternalImage()` only rejects the internal response when `statusCode` is falsy, and `MockedResponse` defaults it to `200`, so nothing really gets rejected there. A `307` coming from a `redirects()` entry in `next.config`, or a `404` for a path that isn't there, both count as a successful image fetch. That body then reaches `detectContentType()`, which returns `null`, and all you get is: ``` The requested resource isn't a valid image for /path/to/image.png received null ``` Nothing in that line points back at the response that actually came in, so a redirect looks exactly like a corrupt image. ### Why? `fetchExternalImage()` right above already covers this with `if (!res.ok)` and logs `res.status`. The internal path is the only fetch in the file without that check, so the same failure gets reported at the fetch for remote images, and three steps later as "not a valid image" for local ones. ### How? The guard now rejects anything outside the 2xx range, which is what `res.ok` means, and the log line carries the status next to the href. The thrown `ImageError` message stays generic, same as the external path, so nothing new shows up in the HTTP response body. Because that throw passes `mocked.res.statusCode` into `ImageError`, a non-2xx internal response now reports its own status instead of always landing on `400`. `ImageError` still maps anything under 400 to `500`, so a redirect answers `500` and a `404` stays `404`. That is what `fetchExternalImage()` has always done, and it lines the two up: a missing local image now returns `404` just like a missing remote one. Three assertions in `test/e2e/image-optimizer/util.ts` shift because of that: - `should not forward cookie header`: `/api/conditional-cookie` answers `401` when the cookie is missing, so `400` becomes `401` - `should error if the image file does not exist`: `/does_not_exist.jpg` returns `404` now, with the internal response message - `should error if the resource isn't a valid image`: this one requested `/test.txt`, but the fixture in `public/` is `text.txt`, so it was quietly exercising a missing file instead of a non-image file. Pointed at the file that exists, it keeps its original `400` and finally tests what its name says Two unit tests cover the new guard, one `307` and one `404`. Every other test in that file sets `statusCode = 200`, so they are untouched. #96985 is open against the same report with a different angle: it threads the status through `ImageUpstream` so the existing "isn't a valid image" log can print it. This one stops the request at the fetch, so the redirect never reaches the optimizer at all. Whichever one fits better, feel free to close mine. ### Verification - `jest test/unit/image-optimizer/`: 139/139, and the two new tests fail against a build without the guard - `pnpm test-dev test/e2e/image-optimizer/content-disposition-type.test.ts`: 97/97 - `pnpm test-start test/e2e/image-optimizer/content-disposition-type.test.ts`: 97/97 - `prettier --check` clean on the three changed files <!-- NEXT_JS_LLM --> Co-authored-by: Steven <steven@ceriously.com>