In a previous PR, `mozjpeg` was set to true which might be the reason
output jpeg spikes cpu.
- https://github.com/vercel/next.js/pull/65846
I made a benchmark and found that disabling MozJPEG saved 72% CPU, with
average outputs 31% larger.
- https://github.com/lovell/sharp/issues/4603
So this PR adds a new experimental flag to enable (or rather disable)
mozjpeg and try with real workloads. This is useful becaues in many
cases, cpu is more costly than bandwidth since most CDNs provide [near
unlimited bandwidth](https://vercel.com/blog/introducing-flat-rate-cdn).
This PR removes some unused code from the image optimizer.
- Remove redundant MIME validation
- Remove unreachable fallback error
These used to be necessary until PR
https://github.com/vercel/next.js/pull/82118 removed the fallback.
Before 82118:
```js
upstreamType = detectContentType(upstreamBuffer) || imageUpstream.contentType?.toLowerCase().trim()
```
After 82118:
```js
upstreamType = detectContentType(upstreamBuffer)
```
Now there is never a case where upstreamType is invalid since its
already been validated, there is no more fallback.
This PR is strictly a refactor, no logic should change.
The idea is that we eventually want to run the image optimizer transform
step in a child process. This PR refactors the code to make a new
`transform.ts` that is lightweight and should reduce some of the
overhead of loading a large module graph when spawning a new process.
- 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>
We were missing some test coverage for image optimization. This PR adds
tests for many missing image formats.
Additionally, this PR also ensures the sharp encoders match that
allowlist from detectContentType() such that future versions of sharp
that enable new encoders by default are not automatically enabled in
next/image.
Practically, there is no change to the end user because
detectContentType() runs before sharp so that is the true allowlist, but
this gives us a little more control to align the sharp allowlist as well
(since sharp can release new features in semver minor).
- `detectContentType` test was too aggressive about asserting completion
in 1ms, let's give it several tries.
- We weren't correctly setting up pnpm overrides in tests, which breaks
things whenever canary comes out.
In a previous PR https://github.com/vercel/next.js/pull/88183, we added
`images.maximumResponseBody` but in only applied to external images.
This PR ensures the same config also applies to internal images.
```
FAIL Turbopack test/unit/image-optimizer/lru-disk-eviction.test.ts
LRU disk eviction
√ should evict oldest entries on initialization (133 ms)
√ should evict old entries when new entries are set (124 ms)
× should promote entries on get() to prevent eviction (8 ms)
√ should return the same LRU instance on subsequent calls (1 ms)
√ should deduplicate concurrent init calls (2 ms)
√ should handle empty cache directory (2 ms)
√ should handle non-existent cache directory (2 ms)
● LRU disk eviction › should promote entries on get() to prevent eviction
EPERM: operation not permitted, rmdir 'C:\Users\ADMINI~1\AppData\Local\Temp\next-lru-test-dOXRaF\y'
```
This PR adds an LRU disk cache so that reads and writes from the Image
Optimization API will evict old entries based on the value of
`images.maximumDiskCacheSize` configuration.
The LRU ensures that cache reads bump the entry to the top so that they
don't get evicted - only the least recently used entries get evicted.
When `next start` is run, if there is an existing disk cache the we will
replay the files in order to populate the LRU and respect
`images.maximumDiskCacheSize` if it was changed.
If no configuration is provided, default to 50% available disk space.
Fixes#89620
Fix `findClosestQuality` returning `0` when the `quality` prop on
`<Image>` is set to a low value (e.g. `quality={1}`), which causes the
image optimization server to reject the request with a 400 error.
### Cause
The `reduce` call in `findClosestQuality` used `0` as its initial
accumulator. Since `0` is not a valid quality value (must be 1-100), it
would win the "closest" comparison for any `quality <= 37` with the
default `qualities: [75]` config. This produced `q=0` in the image URL,
which the server rejects.
### Fix Description
Changed the `reduce` initializer from `0` to `config.qualities[0]`. This
is safe because the early return above already handles the empty array
case, so `config.qualities[0]` is always defined when reached.
Added a unit test for `quality=1` with `qualities=[75]` to verify it
returns `75` instead of `0`.
---------
Co-authored-by: Steven <steven@ceriously.com>
Based on our metrics, the P99.9 for a source image is 47 MB so we can
adjust the default setting to be much lower (by changing from from 300
MB to 50 MB) in favor of reducing memory.
## Background
We have default allowlists for the `url` (localPatterns/remotePatterns
config) and `w` (imageSizes/deviceSizes config) but we don't have a
default allowlist for `q` (currently opt-in with via qualities config).
## What's changing?
BREAKING CHANGE: This PR is a breaking change that sets the default
allowlist to `qualities: [75]`, meaning that anything other than 75 is
invalid. Since most images don't set the quality prop and therefore
default to 75, most apps will be unaffected.
However, we can be even more gracious when users upgrade because we can
coerce the value automatically to 75 (or rather any value in `qualities`
allowlist). In dev, this will print a warning explaining why the prop
was not observed. This also works if you had `qualities` configured but
removed 75, since the closet matching value in the array will be used
instead.
As a follow up to PR https://github.com/vercel/next.js/pull/82538 this
PR adds an experimental flag to optionally skip `sharp.metadata()` and
rely only on the JS implementation for format detection.
Add support for detecting more src image formats via magic number. The
src image formats are handled as follows:
- `image/jxl` - serve as is (since safari can render it)
- `image/heic` - serve as is (since safari can render it)
- `image/jp2` - serve as is (since safari can render it)
- `application/pdf` - error (since no browser will render it)
- `image/pic` - error (since no browser will render it)
We also fallback to `sharp().metadata()` if we can't detect the magic
number to ensure correctness.
Configuring `remotePatterns` can be a hassle because you have to define
each of the parts. And if you forget a part, its a wildcard meaning it
will match anything. This is usually not desirable since most remote
images don't allow query strings.
This PR adds support for using an array of `URL` objects when defining
`images.remotePatterns`.
```js
module.exports = {
images: {
remotePatterns: [
new URL('https://res.cloudinary.com/my-account/**'),
new URL('https://s3.my-account.*.amazonaws.com/**'),
],
},
}
```
Note that wildcards must be explicit and anything missing is assumed to
no longer match.
The `revalidate` property of the `ctx` object that is passed into the
incremental cache by the patched `fetch` as well as `unstable_cache` is
unused since it was introduced in #43659. It was just added because of
how the method signature for `set()` was changed back then.
However, for those kinds of cache entries, the `revalidate` context
property is never used, and instead the `revalidate` property of the
passed-in `data` is used.
To avoid further confusion (e.g. in [this
question](https://github.com/vercel/next.js/pull/76207#discussion_r1968478141)),
this PR improves the method signatures and types of the incremental
cache so that the different call-site use cases can be clearly
discriminated, and superfluous context properties can be omitted.
This adds support for `images.localPatterns` config to allow specific
local images to be optimized and (more importantly) block anything that
doesn't match a pattern.
This PR adds a new feature to the existing `remotePatterns` allowlist
configuration, that enables the ability to filter on `search` (aka query
string).
The most common usage will likely be `search: ''`, which means no query
string allowed.
You can also set the exact query string such as `search: '?v=1'` which
can be useful to invalidate the cache one time for a specific version
without opening the door to any version. Note the leading question mark
to match the behavior of `new URL(url).search`.
### What?
- Reduces redundant image optimizations if the upstream image is
unchanged, there is no need to run the optimization again as you can
just use the previously optimized image.
- Changes `getHash`-function to return `base64url` instead of custom
variant of standard `base64`. This can be removed if deemed not
necessary.
- Moves all image ETag calculation logic (and introduce
`getImageEtag`-function to do it) to happen inside `image-optimizer` for
better consistency.
### Why?
Currently when an image is requested and it becomes stale, the server
will trigger a full image optimization for that image, using a
significant spike in CPU-usage for routes that have multiple images
(e.g. an image carousel).
### How?
By calculating and storing the upstream etag in the cache entry, we can
utilize the previously cached entry to check if the upstream image has
remained the same, making it possible to reuse the previously cached
entry again, as the image optimization would produce the same results
anyways.
---------
Co-authored-by: Steven <steven@ceriously.com>
The tests added in https://github.com/vercel/next.js/pull/46219 were
never correctly testing the headers because `detectContentType()` is
called first and only when we can't detect the type from the response
body do we fallback to the `Content-Type` header.
I also refactored some of the tests because the `ctx: any` type was
causing some tests to not run when testing different configuration
options.
Closes NEXT-3321
### Fixing a bug
### What?
Fix remotePatterns when all paths and/or domains are allowed.
### Why?
micromatch creates a very strange regex for all paths -
`/^(?:(?!\.)(?:(?:(?!(?:^|[\\/])\.).)*?)[\\/]?)$/`. That is, paths
cannot start with a dot or contain a slash followed by a dot.
Interestingly, here are some valid paths:
- /a/a.a/6a00d8341c4fbe53ef02c8d3a82122200d-600wi
- ////a/a.a/6a00d8341c4fbe53ef02c8d3a82122200d-600wi
- ///:?%;№%/a/a.a/6a00d8341c4fbe53ef02c8d3a82122200d-600wi.\/
- /:./6a00d8341c4fbe53ef02c8d3a82122200d-600wi.\/
And here are some invalid ones:
- /.a/6a00d8341c4fbe53ef02c8d3a82122200d-600wi
- /a/.a/6a00d8341c4fbe53ef02c8d3a82122200d-600wi
- ./a/6a00d8341c4fbe53ef02c8d3a82122200d-600wi
I don't think this check makes any sense.
### How?
If the user allows all (`**`) - it means any path or domain will be
considered valid.
- Fixes#60483
- Fixes#58139
- Fixes#46903
---------
Co-authored-by: Steven <steven@ceriously.com>
## Description
This PR implements a new configuration object in `next.config.js` called `experimental.images.remotePatterns`.
This will eventually deprecate `images.domains` because it covers the same use cases and more by allowing wildcard pattern matching on `hostname` and `pathname` and also allows restricting `protocol` and `port`.
## Feature
- [x] Implements an existing feature request.
- [x] Related issues linked
- [x] Unit tests added
- [x] Integration tests added
- [x] Documentation added
- [x] Telemetry added. In case of a feature if it's used or not.
- [x] Errors have helpful link attached, see `contributing.md`
## Related
- Fixes#27925
- Closes#18429
- Closes#18632
- Closes#18730
- Closes#27345
* Move unit tests to one folder
* Migrate unit tests to TypeScript
* add test types to lint
* Ensure ts(x) tests are run with util
* Add tsx extension to jest config
* bump
Steven