Skip to content
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.

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

Support wildcard patterns for upstream remote images #27925

Closed
styfle opened this issue Aug 10, 2021 · 5 comments · Fixed by #36245
Closed

Support wildcard patterns for upstream remote images #27925

styfle opened this issue Aug 10, 2021 · 5 comments · Fixed by #36245
Assignees
Labels
Image (next/image) Related to Next.js Image Optimization.

Comments

@styfle
Copy link
Member

styfle commented Aug 10, 2021

Abstract

Add support for wildcard patterns when configuring the allowlist for upstream remote images.

Background

We currently support a list of domains in next.config.js to configure which hosts are allowed to serve source images that can then be optimized by Next.js. Without this restriction, someone could hot-link to your Image Optimization API with their images instead of yours website’s images and you would have to eat the cost of optimizing their images.

Problem We Need To Solve

The domains implementation is too restrictive because it only allows an exact match on the hostname. It doesn’t allow subdomains or any path matching. So even allowing a single external image linking to social network or CMS would open up the door for any image hosted on that domain to be allowed to optimize.

We would like to solve the following use cases:

  • **.fbcdn.net - Match all subdomains recursively (Facebook Avatars)
  • avatar-management--avatars.*.prod.public.atl-paas.net - Match only part of the domain which represents a region (Bitbucket Avatars)
  • www.datocms-assets.com/13375P34K/** - Match domain but also match all paths after the account number (DatoCMS)
  • https://assets.vercel.com - Match protocol and domain (today we allow both http/https)

Proposed Solution

Provide a new property, remotePatterns, and deprecate domains. This would still support exact matches to cover the use case of domains, but it would also provide a way to define wildcard subdomains, path patterns, or even be extended in the future to support other parts of the URL.

module.exports = {
  images: {
    remotePatterns: [
      // exact match `example.com`
      { hostname: 'example.com' },
      // wildcard subdomains to match all facebook avatars
      { hostname: '**.fbcdn.net' },
      // wildcard the region to match all bitbucket avatars
      { hostname: 'avatar-management--avatars.*.prod.public.atl-paas.net' },
      // exact match hostname but wildcard path to match the account
      { hostname: 'www.datocms-assets.com', pathname: '/13375P34K/**' },
      // exact match protocol and domain
      { protocol: 'https', hostname: 'assets.vercel.com' },
    ]
  }
}

The parts of the URL that are not defined would implicitly be considered wildcards. For example, since query is not defined, it could match anything.

We should require at least hostname is defined, while other props can be optional.

We’ll also want to impose a maximum limit to the array, so we’ll start with 50 to match the current limit of domains.

Alternatives Considered

  • Reusing the domains property could be nice because users know to look there already, however the name doesn’t make sense once pathname or protocol is included and its not clear it supports pattern matching.
  • Using the full URL string, such as https://www.datocms-assets.com/13375P34K/**, instead of breaking it into parts would make for a nicer experience but it would lead to ambiguity. For example, what if the user wanted to match port or querystring in the future? Wildcards are placeholders so its not clear which part of the URL should be a pattern and which part should be literal.
  • Reusing the existing pattern matching syntax from [rewrites](https://nextjs.org/docs/api-reference/next.config.js/rewrites), such as https://www.datocms-assets.com/13375P34K/:path*. This would work great if we can utilize the same syntax for host but that could be challenging since the host uses a . separator as opposed to /.
  • Using PCRE (regex) would be verbose and prone to error because . (and many other chars) need to be escaped.
  • Using a rewrite is possible today for some of these use cases but not all (doesn't work for hostname patterns).
  • Using the new URLPattern object but its still experimental and subject to change.

Related

@styfle styfle added this to the backlog milestone Aug 10, 2021
@styfle styfle self-assigned this Aug 10, 2021
@styfle styfle changed the title Add support for wildcard patterns in image.domains Add support for wildcard patterns in images.domains Aug 10, 2021
@styfle styfle changed the title Add support for wildcard patterns in images.domains Support wildcard patterns in images.domains Aug 10, 2021
@timneutkens timneutkens added the Image (next/image) Related to Next.js Image Optimization. label Nov 17, 2021
@FDiskas
Copy link

FDiskas commented Feb 2, 2022

Hi,
Thanks for proposal.
What about this one https://github.com/isaacs/minimatch

NPM internally use that. So well tested and documented and also will be supported in the future

@styfle
Copy link
Member Author

styfle commented Feb 2, 2022

Does it work with periods instead of forward slashes?

I think the best solution is to write our own parser since we’ll have full control and can extend it later. I would imagine we need to add support for hostname patterns and pathname patterns, but its not clear if the port matters for example.

Also, we don't want to inherit any vulnerabilities of minimatch if we don't have to.

@neeraj3029

This comment was marked as off-topic.

@styfle styfle removed this from the backlog milestone Apr 4, 2022
@styfle styfle changed the title Support wildcard patterns in images.domains Support wildcard patterns for upstream remote images Apr 5, 2022
@kodiakhq kodiakhq bot closed this as completed in #36245 May 5, 2022
kodiakhq bot pushed a commit that referenced this issue May 5, 2022
…#36245)

## 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
@styfle
Copy link
Member Author

styfle commented May 6, 2022

This feature is available on the canary channel npm i next@canary using the experimental remotePatterns config.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2022

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Image (next/image) Related to Next.js Image Optimization.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants