feat(assets): support remote images

[y-nk]

Changes

This enable the processing of remote images with the astro:assets. It will download the images into a cache folder and process them as if it was any other image.

EDIT: after discussing with @Princesseuh on the subject, I've added a round of security measures through the domains and remotePatterns configuration options, which works similarly to next/image's one (described here: https://nextjs.org/docs/app/api-reference/components/image#remotepatterns)

Testing

I've forked astro locally, iterated and tested manually both for dev and build mode with a sample route such as:

---
export const prerender = true

import { Image } from 'astro:assets'

import localImage from '~/assets/600x400.png'

const remoteImage = 'https://placehold.co/600x400.png'
---

<>
  <Image src={localImage} width={60} height={40} alt="" />
  <Image src={remoteImage} width={60} height={40} alt="" />
</>

In my config:

{
  image: {
    service: sharpImageService(),
    domains: ['placehold.co'],
  }
}

Docs

/cc @withastro/maintainers-docs for feedback!

I didn't add docs yet because at this point I'm not sure the PR will be approved. I'd rather wait for round of feedbacks and approval of code, settle on API and then add docs before merging 🙏

[changeset-bot]

🦋 Changeset detected

Latest commit: 9665ed5

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[ematipico]

Hi @y-nk , thank you for your contribution. Considering that there isn't an RFC for this feature, could you please describe the feature you're implementing in detail?

The code seems to do much more (remote patterns? domain? etc.) compared the description. This will help me (and other people) to review your code.

[y-nk]

@ematipico sure can do. if you want the full discussion, you can refer to this discord thread. Let me know if the following is clear enough ; if not i'll be glad to explain further.

---

Original problem

The current state of astro:assets does not allow for remote images to be processed. It is a problem for me (who need to import images from a strapi backend) but it has also been pointed out 2 or 3 times over the last week as "something necessary/something we should do" in the discord server. I opened a thread (↑) to discuss the issue and found good support and guidance on how to kickstart this so i thought... why not trying to solve my problem and help the team?

First implementation

The original scope of the PR (thanks to @Princesseuh's guidances and contributions) only proposed to add the processing of remote images as well. Basically, when writing:

import { Image } from 'astro:assets'
<Image src="https://placehold.co/640x480.png />

the result will be an <img /> which url is going through the asset pipeline (/_image?href=)

Wrapping up

After I've done this, I discussed it with @Princesseuh who suggested that for security reasons we should add a level of filtering for remote images, in the same way that next/image works. I personally have no culture for how this feat is implemented, so I thought that the vercel way would be a good starting point.

The next/image feature comes with 2 levels of allowances for enabling the processing of remote images called domains and remotePatterns. You can find the documentation here: https://nextjs.org/docs/app/api-reference/components/image#remotepatterns

Basically, remotePatterns is an object-based url matching algorithm while domains which is simpler by design is a string based strict equality on the domain requesting the image.

So I asked Erika which of the 2 levels she would like, and she answered "domains first, but ultimately we'll need both" so i thought that with a good design i could do both in one shot (and i think i did well).

What's in the PR

The final result is adding remote image processing to astro:assets, but prohibited by default, hidden behind either domains or remotePatterns options, available in the AstroUserConfig under config.image.domains or config.image.remotePatterns.

Side notes

As for the algorithm behind remotePatterns, i did not want to do the vercel way (here) because i think my way is faster (?) and does not require installing a 3rd party package (micromatch). To make sure all is covered I added unit tests, but i'll be happy to add more cases if you require some.

[pijusz]

Hi, just wanted to ask if there is any progress with remote images? As y-nk has mostly finished this feature

[Princesseuh]

Hi, just wanted to ask if there is any progress with remote images? As y-nk has mostly finished this feature

I'm hoping to get to it this week, it seems like the current implementation break some things looking at the test and there's also some docs work needed for it.

[y-nk]

@Princesseuh sorry for not updating earlier as i've been away for a long weekend afk ; i'm back and will take some time later this week to address this.

[tusamni]

@y-nk Thanks for working on this, I've been hoping something like this would happen eventually.

[futurefabric]

Really pleased this work is (hopefully) about to drop, thank you for looking into this 🙏
(I'm v. keen to move to astro:assets ahead of the deprecation of astrojs/image but can't do so without this support for remote images.)

[sarah11918]

Thanks @y-nk and @Princesseuh for this cool feature! I've left some suggestions below for your consideration re: docs! 🙌

[sarah11918]

I'm noticing now that there's no mention in the description as to whether this is exclusively for the <Image /> component, or whether it also works with the <img> tag.

It doesn't have to affect this PR, but for the v3 docs I'm writing, I'd like to know so I can be explicit! Can someone mention me here and let me know one way or the other? Thanks! 🙌

@Princesseuh @y-nk

[Princesseuh]

I'm noticing now that there's no mention in the description as to whether this is exclusively for the <Image /> component, or whether it also works with the <img> tag.

It doesn't have to affect this PR, but for the v3 docs I'm writing, I'd like to know so I can be explicit! Can someone mention me here and let me know one way or the other? Thanks! 🙌

@Princesseuh @y-nk

This is only for image processing, so only when using Image or getImage. For normal img tags, everything is allowed and we do no processing at all.

The technical reasons for all the fluff of domains and remote patterns regarding processing remote images is that the server (not the user's browser) has to fetch the images to process them, so you don't want users to be able to make your server fetch any random URLs

[ematipico]

Thank you for addressing my comments! Let's 🚢 it!

[notjosh]

@y-nk sorry to comment on a closed PR, but is this logic correct? I think it should be >, but I may be missing something obvious.

(but big ❤️❤️❤️ for this PR overall - it's super helpful to have remote images optimised locally!)

[y-nk]

@notjosh i'll be honest i don't remember writing that line. maybe @Princesseuh have more clue about it. from reading the if it seems you're right, but i'm not so sure.

[Princesseuh]

This was fixed in a PR after this one!