From bc95d22118adb295d266764b8abd1e488cf82464 Mon Sep 17 00:00:00 2001 From: Julien Barbay Date: Mon, 24 Jul 2023 14:12:30 +0700 Subject: [PATCH 01/16] feat(assets): support remote images --- packages/astro/src/assets/generate.ts | 32 ++++++------- packages/astro/src/assets/internal.ts | 6 ++- packages/astro/src/assets/types.ts | 2 +- .../astro/src/assets/vite-plugin-assets.ts | 48 +++++++++++++------ 4 files changed, 56 insertions(+), 32 deletions(-) diff --git a/packages/astro/src/assets/generate.ts b/packages/astro/src/assets/generate.ts index d6cb02e560f3..90d81a02a27f 100644 --- a/packages/astro/src/assets/generate.ts +++ b/packages/astro/src/assets/generate.ts @@ -27,10 +27,6 @@ export async function generateImage( options: ImageTransform, filepath: string ): Promise { - if (!isESMImportedImage(options.src)) { - return undefined; - } - let useCache = true; const assetsCacheDir = new URL('assets/', buildOpts.settings.config.cacheDir); @@ -70,22 +66,26 @@ export async function generateImage( } // The original file's path (the `src` attribute of the ESM imported image passed by the user) - const originalImagePath = options.src.src; - - const fileData = await fs.promises.readFile( - new URL( - '.' + - prependForwardSlash( - join(buildOpts.settings.config.build.assets, basename(originalImagePath)) - ), - serverRoot - ) - ); + // if it's a string we assumed it was cached before in the cacheDir/fetch folder. + const fileURL = isESMImportedImage(options.src) + ? new URL( + '.' + + prependForwardSlash( + join(buildOpts.settings.config.build.assets, basename(options.src.src)) + ), + serverRoot + ) + : new URL( + join('.', 'remote-assets', new URL(options.src).pathname), + buildOpts.settings.config.cacheDir + ); + + const fileData: Buffer = await fs.promises.readFile(fileURL); const imageService = (await getConfiguredImageService()) as LocalImageService; const resultData = await imageService.transform( fileData, - { ...options, src: originalImagePath }, + { ...options, src: isESMImportedImage(options.src) ? options.src.src : options.src }, buildOpts.settings.config.image.service.config ); diff --git a/packages/astro/src/assets/internal.ts b/packages/astro/src/assets/internal.ts index 635d0a5e7a9a..b2e79b08accb 100644 --- a/packages/astro/src/assets/internal.ts +++ b/packages/astro/src/assets/internal.ts @@ -6,6 +6,10 @@ export function isESMImportedImage(src: ImageMetadata | string): src is ImageMet return typeof src === 'object'; } +export function isRemoteImage(src: ImageMetadata | string): src is string { + return typeof src === 'string'; +} + export async function getConfiguredImageService(): Promise { if (!globalThis?.astroAsset?.imageService) { const { default: service }: { default: ImageService } = await import( @@ -45,7 +49,7 @@ export async function getImage( // In build and for local services, we need to collect the requested parameters so we can generate the final images if (isLocalService(service) && globalThis.astroAsset.addStaticImage) { - imageURL = globalThis.astroAsset.addStaticImage(validatedOptions); + imageURL = await globalThis.astroAsset.addStaticImage(validatedOptions); } return { diff --git a/packages/astro/src/assets/types.ts b/packages/astro/src/assets/types.ts index d4a8c4c89590..30d17f96adfe 100644 --- a/packages/astro/src/assets/types.ts +++ b/packages/astro/src/assets/types.ts @@ -11,7 +11,7 @@ declare global { // eslint-disable-next-line no-var var astroAsset: { imageService?: ImageService; - addStaticImage?: ((options: ImageTransform) => string) | undefined; + addStaticImage?: ((options: ImageTransform) => string | Promise) | undefined; staticImages?: Map; }; } diff --git a/packages/astro/src/assets/vite-plugin-assets.ts b/packages/astro/src/assets/vite-plugin-assets.ts index 6a29d02f0b0d..5355b30bd4a3 100644 --- a/packages/astro/src/assets/vite-plugin-assets.ts +++ b/packages/astro/src/assets/vite-plugin-assets.ts @@ -1,10 +1,11 @@ +import { writeFile, mkdir } from 'node:fs/promises'; import { bold } from 'kleur/colors'; import MagicString from 'magic-string'; import { fileURLToPath } from 'node:url'; import type * as vite from 'vite'; import { normalizePath } from 'vite'; import type { AstroPluginOptions, ImageTransform } from '../@types/astro'; -import { error } from '../core/logger/core.js'; +import { info, error } from '../core/logger/core.js'; import { appendForwardSlash, joinPaths, @@ -12,7 +13,7 @@ import { removeQueryString, } from '../core/path.js'; import { VIRTUAL_MODULE_ID, VIRTUAL_SERVICE_ID } from './consts.js'; -import { isESMImportedImage } from './internal.js'; +import { isRemoteImage } from './internal.js'; import { emitESMImage } from './utils/emitAsset.js'; import { hashTransform, propsToFilename } from './utils/transformToPath.js'; @@ -95,7 +96,7 @@ export default function assets({ return; } - globalThis.astroAsset.addStaticImage = (options) => { + globalThis.astroAsset.addStaticImage = async (options) => { if (!globalThis.astroAsset.staticImages) { globalThis.astroAsset.staticImages = new Map< string, @@ -103,25 +104,44 @@ export default function assets({ >(); } + // in case of remote images + if (isRemoteImage(options.src)) { + const remoteCacheDir = new URL('remote-assets/', settings.config.cacheDir); + await mkdir(remoteCacheDir, { recursive: true }); + + const remoteFileURL = new URL(options.src); + const cachedFileURL = new URL('.' + remoteFileURL.pathname, remoteCacheDir); + + info( + logging, + 'astro:assets', + `${bold('caching remote asset')} ${remoteFileURL.href} -> ${cachedFileURL.href}` + ); + const res = await fetch(remoteFileURL); + const imgBytes = await res.arrayBuffer(); + await writeFile(cachedFileURL, Buffer.from(imgBytes)); + } + const hash = hashTransform(options, settings.config.image.service.entrypoint); let filePath: string; if (globalThis.astroAsset.staticImages.has(hash)) { filePath = globalThis.astroAsset.staticImages.get(hash)!.path; + } else if (isRemoteImage(options.src)) { + const { pathname } = new URL(options.src); + filePath = options.format + ? pathname.slice(0, pathname.lastIndexOf('.') + 1) + options.format + : pathname; } else { - // If the image is not imported, we can return the path as-is, since static references - // should only point ot valid paths for builds or remote images - if (!isESMImportedImage(options.src)) { - return options.src; - } - - filePath = prependForwardSlash( - joinPaths(settings.config.build.assets, propsToFilename(options, hash)) - ); - globalThis.astroAsset.staticImages.set(hash, { path: filePath, options: options }); + filePath = propsToFilename(options, hash); } - if (settings.config.build.assetsPrefix) { + filePath = prependForwardSlash(joinPaths(settings.config.build.assets, filePath)); + globalThis.astroAsset.staticImages.set(hash, { path: filePath, options: options }); + + if (isRemoteImage(options.src)) { + return filePath; + } else if (settings.config.build.assetsPrefix) { return joinPaths(settings.config.build.assetsPrefix, filePath); } else { return prependForwardSlash(joinPaths(settings.config.base, filePath)); From 2ae51ef7da7f7677ab694b4fd92712450d296c78 Mon Sep 17 00:00:00 2001 From: Julien Barbay Date: Mon, 24 Jul 2023 15:58:42 +0700 Subject: [PATCH 02/16] fix: recursivity issue when reusing asset with hash --- .../astro/src/assets/vite-plugin-assets.ts | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/astro/src/assets/vite-plugin-assets.ts b/packages/astro/src/assets/vite-plugin-assets.ts index 5355b30bd4a3..f75fb8ab26f5 100644 --- a/packages/astro/src/assets/vite-plugin-assets.ts +++ b/packages/astro/src/assets/vite-plugin-assets.ts @@ -1,4 +1,5 @@ import { writeFile, mkdir } from 'node:fs/promises'; +import { existsSync } from 'node:fs'; import { bold } from 'kleur/colors'; import MagicString from 'magic-string'; import { fileURLToPath } from 'node:url'; @@ -112,20 +113,24 @@ export default function assets({ const remoteFileURL = new URL(options.src); const cachedFileURL = new URL('.' + remoteFileURL.pathname, remoteCacheDir); - info( - logging, - 'astro:assets', - `${bold('caching remote asset')} ${remoteFileURL.href} -> ${cachedFileURL.href}` - ); - const res = await fetch(remoteFileURL); - const imgBytes = await res.arrayBuffer(); - await writeFile(cachedFileURL, Buffer.from(imgBytes)); + // there's no async api for exists :( + if (!existsSync(cachedFileURL)) { + info( + logging, + 'astro:assets', + `${bold('downloading remote asset')} ${remoteFileURL.href} -> ${cachedFileURL.href}` + ); + const res = await fetch(remoteFileURL); + const imgBytes = await res.arrayBuffer(); + await writeFile(cachedFileURL, Buffer.from(imgBytes)); + } } const hash = hashTransform(options, settings.config.image.service.entrypoint); let filePath: string; - if (globalThis.astroAsset.staticImages.has(hash)) { + const hasHash = globalThis.astroAsset.staticImages.has(hash); + if (hasHash) { filePath = globalThis.astroAsset.staticImages.get(hash)!.path; } else if (isRemoteImage(options.src)) { const { pathname } = new URL(options.src); @@ -136,8 +141,10 @@ export default function assets({ filePath = propsToFilename(options, hash); } - filePath = prependForwardSlash(joinPaths(settings.config.build.assets, filePath)); - globalThis.astroAsset.staticImages.set(hash, { path: filePath, options: options }); + if (!hasHash) { + filePath = prependForwardSlash(joinPaths(settings.config.build.assets, filePath)); + globalThis.astroAsset.staticImages.set(hash, { path: filePath, options: options }); + } if (isRemoteImage(options.src)) { return filePath; From f483d6dd22e743360cb9b36f236ae83d0377680f Mon Sep 17 00:00:00 2001 From: Julien Barbay Date: Mon, 24 Jul 2023 16:23:34 +0700 Subject: [PATCH 03/16] feat: add support images in baseService --- packages/astro/src/assets/services/service.ts | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/packages/astro/src/assets/services/service.ts b/packages/astro/src/assets/services/service.ts index d3479c8803fa..b6521ae5e5a3 100644 --- a/packages/astro/src/assets/services/service.ts +++ b/packages/astro/src/assets/services/service.ts @@ -203,20 +203,29 @@ export const baseService: Omit = { }; }, getURL(options: ImageTransform) { - // Both our currently available local services don't handle remote images, so we return the path as is. - if (!isESMImportedImage(options.src)) { - return options.src; - } + const PARAMS: Record = { + w: 'width', + h: 'height', + q: 'quality', + f: 'format', + }; - const searchParams = new URLSearchParams(); - searchParams.append('href', options.src.src); + const searchParams = Object.entries(PARAMS).reduce((params, [param, key]) => { + options[key] && params.append(param, options[key].toString()); + return params; + }, new URLSearchParams()); - options.width && searchParams.append('w', options.width.toString()); - options.height && searchParams.append('h', options.height.toString()); - options.quality && searchParams.append('q', options.quality.toString()); - options.format && searchParams.append('f', options.format); + if (isESMImportedImage(options.src)) { + searchParams.append('href', options.src.src); + } else if (options.src.startsWith('http')) { + searchParams.append('href', options.src); + } else { + // ignore non http strings + return options.src; + } - return joinPaths(import.meta.env.BASE_URL, '/_image?') + searchParams; + const imageEndpoint = joinPaths(import.meta.env.BASE_URL, '/_image'); + return `${imageEndpoint}?${searchParams}`; }, parseURL(url) { const params = url.searchParams; From 4cd00247f1d8493abe1b2afd1b659b2f7317482c Mon Sep 17 00:00:00 2001 From: Julien Barbay Date: Tue, 25 Jul 2023 14:28:39 +0700 Subject: [PATCH 04/16] feat: remote pattern algorithms feat: remote pattern image config feat: block non allowed remote urls sq --- packages/astro/src/@types/astro.ts | 71 ++++++++++- packages/astro/src/assets/internal.ts | 22 +++- packages/astro/src/assets/services/service.ts | 29 +++-- .../astro/src/assets/utils/remotePattern.ts | 64 ++++++++++ .../astro/src/assets/vite-plugin-assets.ts | 13 +- packages/astro/src/core/config/schema.ts | 26 ++++ .../test/units/assets/remote-pattern.test.js | 111 ++++++++++++++++++ 7 files changed, 319 insertions(+), 17 deletions(-) create mode 100644 packages/astro/src/assets/utils/remotePattern.ts create mode 100644 packages/astro/test/units/assets/remote-pattern.test.js diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index fb34954ac743..bfb85d23c500 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -22,6 +22,7 @@ import type { AstroCookies } from '../core/cookies'; import type { LogOptions } from '../core/logger/core'; import type { AstroComponentFactory, AstroComponentInstance } from '../runtime/server'; import type { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from './../core/constants.js'; +import type { RemotePattern } from '../assets/utils/remotePattern'; export type { MarkdownHeading, MarkdownMetadata, @@ -44,6 +45,7 @@ export type { ImageQualityPreset, ImageTransform, } from '../assets/types'; +export type { RemotePattern } from '../assets/utils/remotePattern'; export type { SSRManifest } from '../core/app/types'; export type { AstroCookies } from '../core/cookies'; @@ -364,10 +366,10 @@ export interface ViteUserConfig extends vite.UserConfig { ssr?: vite.SSROptions; } -export interface ImageServiceConfig { +export interface ImageServiceConfig = Record> { // eslint-disable-next-line @typescript-eslint/ban-types entrypoint: 'astro/assets/services/sharp' | 'astro/assets/services/squoosh' | (string & {}); - config?: Record; + config?: T; } /** @@ -1004,6 +1006,71 @@ export interface AstroUserConfig { * ``` */ service: ImageServiceConfig; + + /** + * @docs + * @name image.domains (Experimental) + * @type {Array} + * @default `{domains: []}` + * @version 2.9.2 + * @description + * Defines a list of domains allowed to be optimized locally + * + * The value should be an array of domain strings of images allowed to be optimized locally. + * Wildcards are not permitted (see image.remotePatterns). + * + * ```js + * { + * image: { + * // Example: Enable the Sharp-based image service + * domains: ['astro.build'], + * }, + * } + * ``` + */ + domains?: string[]; + + /** + * @docs + * @name image.remotePatterns (Experimental) + * @type {Array} + * @default `{remotePatterns: []}` + * @version 2.9.2 + * @description + * Defines a list of url patterns allowed to be optimized locally + * + * The patterns can contain 4 properties: + * 1. protocol + * 2. hostname + * 3. port + * 4. pathname + * + * Each property will be matched individually to the URL object representing the remote asset to be loaded. + * The rules for matching are: + * + * 1. protocol & port: strict equal.(`===`) + * 2. hostname: + * - if the hostname starts with '**.', all subdomains will be allowed ('endsWith') + * - if the hostname starts with '*.', only one level of subdomain will be allowed + * - strict equal otherwise + * 3. pathname: + * - if the pathname ends with '/**', all sub routes will be allowed ('startsWith') + * - if the pathname ends with '/*', only one level of sub route will be allowed + * - strict equal otherwise + * + * ```js + * { + * image: { + * // Example: statically process all images from your aws s3 bucket + * remotePatterns: [{ + * protocol: 'https', + * hostname: '**.amazonaws.com', + * }], + * }, + * } + * ``` + */ + remotePatterns?: Partial[]; }; /** diff --git a/packages/astro/src/assets/internal.ts b/packages/astro/src/assets/internal.ts index b2e79b08accb..00388b0869c3 100644 --- a/packages/astro/src/assets/internal.ts +++ b/packages/astro/src/assets/internal.ts @@ -1,6 +1,8 @@ +import type { AstroConfig } from '../@types/astro.js'; import { AstroError, AstroErrorData } from '../core/errors/index.js'; import { isLocalService, type ImageService } from './services/service.js'; import type { GetImageResult, ImageMetadata, ImageTransform } from './types.js'; +import { matchPattern, matchHostname } from './utils/remotePattern.js'; export function isESMImportedImage(src: ImageMetadata | string): src is ImageMetadata { return typeof src === 'object'; @@ -10,6 +12,21 @@ export function isRemoteImage(src: ImageMetadata | string): src is string { return typeof src === 'string'; } +export function isRemoteAllowed( + src: string, + { + domains = [], + remotePatterns = [], + }: Partial> +) { + const url = new URL(src); + + return ( + domains.some((domain) => matchHostname(url, domain)) || + remotePatterns.some((remotePattern) => matchPattern(url, remotePattern)) + ); +} + export async function getConfiguredImageService(): Promise { if (!globalThis?.astroAsset?.imageService) { const { default: service }: { default: ImageService } = await import( @@ -31,7 +48,8 @@ export async function getConfiguredImageService(): Promise { export async function getImage( options: ImageTransform, - serviceConfig: Record + serviceConfig: Record, + assetsConfig: AstroConfig['image'] ): Promise { if (!options || typeof options !== 'object') { throw new AstroError({ @@ -45,7 +63,7 @@ export async function getImage( ? await service.validateOptions(options, serviceConfig) : options; - let imageURL = await service.getURL(validatedOptions, serviceConfig); + let imageURL = await service.getURL(validatedOptions, serviceConfig, assetsConfig); // In build and for local services, we need to collect the requested parameters so we can generate the final images if (isLocalService(service) && globalThis.astroAsset.addStaticImage) { diff --git a/packages/astro/src/assets/services/service.ts b/packages/astro/src/assets/services/service.ts index b6521ae5e5a3..aa9f476d178a 100644 --- a/packages/astro/src/assets/services/service.ts +++ b/packages/astro/src/assets/services/service.ts @@ -1,7 +1,8 @@ +import type { AstroConfig } from '../../@types/astro.js'; import { AstroError, AstroErrorData } from '../../core/errors/index.js'; import { joinPaths } from '../../core/path.js'; import { VALID_SUPPORTED_FORMATS } from '../consts.js'; -import { isESMImportedImage } from '../internal.js'; +import { isESMImportedImage, isRemoteAllowed } from '../internal.js'; import type { ImageOutputFormat, ImageTransform } from '../types.js'; export type ImageService = LocalImageService | ExternalImageService; @@ -23,7 +24,7 @@ export function parseQuality(quality: string): string | number { return result; } -interface SharedServiceProps { +interface SharedServiceProps = Record> { /** * Return the URL to the endpoint or URL your images are generated from. * @@ -32,7 +33,11 @@ interface SharedServiceProps { * For external services, this should point to the URL your images are coming from, for instance, `/_vercel/image` * */ - getURL: (options: ImageTransform, serviceConfig: Record) => string | Promise; + getURL: ( + options: ImageTransform, + serviceConfig: T, + assetsConfig: AstroConfig['image'] + ) => string | Promise; /** * Return any additional HTML attributes separate from `src` that your service requires to show the image properly. * @@ -41,7 +46,7 @@ interface SharedServiceProps { */ getHTMLAttributes?: ( options: ImageTransform, - serviceConfig: Record + serviceConfig: T ) => Record | Promise>; /** * Validate and return the options passed by the user. @@ -53,18 +58,20 @@ interface SharedServiceProps { */ validateOptions?: ( options: ImageTransform, - serviceConfig: Record + serviceConfig: T ) => ImageTransform | Promise; } -export type ExternalImageService = SharedServiceProps; +export type ExternalImageService = Record> = + SharedServiceProps; export type LocalImageTransform = { src: string; [key: string]: any; }; -export interface LocalImageService extends SharedServiceProps { +export interface LocalImageService = Record> + extends SharedServiceProps { /** * Parse the requested parameters passed in the URL from `getURL` back into an object to be used later by `transform`. * @@ -72,7 +79,7 @@ export interface LocalImageService extends SharedServiceProps { */ parseURL: ( url: URL, - serviceConfig: Record + serviceConfig: T ) => LocalImageTransform | undefined | Promise | Promise; /** * Performs the image transformations on the input image and returns both the binary data and @@ -81,7 +88,7 @@ export interface LocalImageService extends SharedServiceProps { transform: ( inputBuffer: Buffer, transform: LocalImageTransform, - serviceConfig: Record + serviceConfig: T ) => Promise<{ data: Buffer; format: ImageOutputFormat }>; } @@ -202,7 +209,7 @@ export const baseService: Omit = { decoding: attributes.decoding ?? 'async', }; }, - getURL(options: ImageTransform) { + getURL(options, serviceConfig, assetsConfig) { const PARAMS: Record = { w: 'width', h: 'height', @@ -217,7 +224,7 @@ export const baseService: Omit = { if (isESMImportedImage(options.src)) { searchParams.append('href', options.src.src); - } else if (options.src.startsWith('http')) { + } else if (isRemoteAllowed(options.src, assetsConfig)) { searchParams.append('href', options.src); } else { // ignore non http strings diff --git a/packages/astro/src/assets/utils/remotePattern.ts b/packages/astro/src/assets/utils/remotePattern.ts new file mode 100644 index 000000000000..c8d88c68e0ad --- /dev/null +++ b/packages/astro/src/assets/utils/remotePattern.ts @@ -0,0 +1,64 @@ +export type RemotePattern = { + hostname?: string; + pathname?: string; + protocol?: string; + port?: string; +}; + +export function matchPattern(url: URL, remotePattern: RemotePattern) { + return ( + matchProtocol(url, remotePattern.protocol) && + matchHostname(url, remotePattern.hostname, true) && + matchPort(url, remotePattern.port) && + matchPathname(url, remotePattern.pathname, true) + ); +} + +export function matchPort(url: URL, port?: string) { + return !port || port === url.port; +} + +export function matchProtocol(url: URL, protocol?: string) { + return !protocol || protocol === url.protocol.slice(0, -1); +} + +export function matchHostname(url: URL, hostname?: string, allowWildcard?: boolean) { + if (!hostname) { + return true; + } else if (!allowWildcard || !hostname.startsWith('*')) { + console.log(hostname, url.hostname); + return hostname === url.hostname; + } else if (hostname.startsWith('**.')) { + const slicedHostname = hostname.slice(2); // ** length + return slicedHostname !== url.hostname && url.hostname.endsWith(slicedHostname); + } else if (hostname.startsWith('*.')) { + const slicedHostname = hostname.slice(1); // * length + const additionalSubdomains = url.hostname + .replace(slicedHostname, '') + .split('.') + .filter(Boolean); + return additionalSubdomains.length === 1; + } + + return false; +} + +export function matchPathname(url: URL, pathname?: string, allowWildcard?: boolean) { + if (!pathname) { + return true; + } else if (!allowWildcard || !pathname.endsWith('*')) { + return pathname === url.pathname; + } else if (pathname.endsWith('/**')) { + const slicedPathname = pathname.slice(0, -2); // ** length + return slicedPathname !== url.pathname && url.pathname.startsWith(slicedPathname); + } else if (pathname.endsWith('/*')) { + const slicedPathname = pathname.slice(0, -1); // * length + const additionalPathChunks = url.pathname + .replace(slicedPathname, '') + .split('/') + .filter(Boolean); + return additionalPathChunks.length === 1; + } + + return false; +} diff --git a/packages/astro/src/assets/vite-plugin-assets.ts b/packages/astro/src/assets/vite-plugin-assets.ts index f75fb8ab26f5..248b3072ff77 100644 --- a/packages/astro/src/assets/vite-plugin-assets.ts +++ b/packages/astro/src/assets/vite-plugin-assets.ts @@ -14,7 +14,7 @@ import { removeQueryString, } from '../core/path.js'; import { VIRTUAL_MODULE_ID, VIRTUAL_SERVICE_ID } from './consts.js'; -import { isRemoteImage } from './internal.js'; +import { isRemoteAllowed, isRemoteImage } from './internal.js'; import { emitESMImage } from './utils/emitAsset.js'; import { hashTransform, propsToFilename } from './utils/transformToPath.js'; @@ -88,7 +88,8 @@ export default function assets({ export { default as Image } from "astro/components/Image.astro"; export const imageServiceConfig = ${JSON.stringify(settings.config.image.service.config)}; - export const getImage = async (options) => await getImageInternal(options, imageServiceConfig); + export const astroAssetsConfig = ${JSON.stringify(settings.config.image)}; + export const getImage = async (options) => await getImageInternal(options, imageServiceConfig, astroAssetsConfig); `; } }, @@ -105,6 +106,14 @@ export default function assets({ >(); } + // hard skip on non allowed remote images + if ( + typeof options.src === 'string' && + !isRemoteAllowed(options.src, settings.config.image) + ) { + return options.src; + } + // in case of remote images if (isRemoteImage(options.src)) { const remoteCacheDir = new URL('remote-assets/', settings.config.cacheDir); diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index 34c1e554903c..cfd0b2b1e772 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -181,9 +181,35 @@ export const AstroConfigSchema = z.object({ ]), config: z.record(z.any()).default({}), }), + domains: z.array(z.string()).default([]), + remotePatterns: z + .array( + z.object({ + protocol: z.string().optional(), + hostname: z + .string() + .refine( + (val) => !val.includes('*') || val.startsWith('*.') || val.startsWith('**.'), + { + message: 'wildcards can only be placed at the beginning of the hostname', + } + ) + .optional(), + port: z.string().optional(), + pathname: z + .string() + .refine((val) => !val.includes('*') || val.endsWith('/*') || val.endsWith('/**'), { + message: 'wildcards can only be placed at the end of a pathname', + }) + .optional(), + }) + ) + .default([]), }) .default({ service: { entrypoint: 'astro/assets/services/squoosh', config: {} }, + domains: [], + remotePatterns: [], }), markdown: z .object({ diff --git a/packages/astro/test/units/assets/remote-pattern.test.js b/packages/astro/test/units/assets/remote-pattern.test.js new file mode 100644 index 000000000000..62a411e3a659 --- /dev/null +++ b/packages/astro/test/units/assets/remote-pattern.test.js @@ -0,0 +1,111 @@ +import { expect } from 'chai'; +import { + matchProtocol, + matchPort, + matchHostname, + matchPathname, + matchPattern, +} from '../../../dist/assets/utils/remotePattern.js'; + +describe('astro/src/assets/utils/remotePattern', () => { + const url1 = new URL('https://docs.astro.build/en/getting-started'); + const url2 = new URL('http://preview.docs.astro.build:8080/'); + const url3 = new URL('https://astro.build/'); + const url4 = new URL('https://example.co/'); + + describe('remote pattern matchers', () => { + it('matches protocol', async () => { + // undefined + expect(matchProtocol(url1)).to.be.true; + + // defined, true/false + expect(matchProtocol(url1, 'http')).to.be.false; + expect(matchProtocol(url1, 'https')).to.be.true; + }); + + it('matches port', async () => { + // undefined + expect(matchPort(url1)).to.be.true; + + // defined, but port is empty (default port used in URL) + expect(matchPort(url1, '')).to.be.true; + + // defined and port is custom + expect(matchPort(url2, '8080')).to.be.true; + }); + + it('matches hostname (no wildcards)', async () => { + // undefined + expect(matchHostname(url1)).to.be.true; + + // defined, true/false + expect(matchHostname(url1, 'astro.build')).to.be.false; + expect(matchHostname(url1, 'docs.astro.build')).to.be.true; + }); + + it('matches hostname (with wildcards)', async () => { + // defined, true/false + expect(matchHostname(url1, 'docs.astro.build', true)).to.be.true; + expect(matchHostname(url1, '**.astro.build', true)).to.be.true; + expect(matchHostname(url1, '*.astro.build', true)).to.be.true; + + expect(matchHostname(url2, '*.astro.build', true)).to.be.false; + expect(matchHostname(url2, '**.astro.build', true)).to.be.true; + + expect(matchHostname(url3, 'astro.build', true)).to.be.true; + expect(matchHostname(url3, '*.astro.build', true)).to.be.false; + expect(matchHostname(url3, '**.astro.build', true)).to.be.false; + }); + + it('matches pathname (no wildcards)', async () => { + // undefined + expect(matchPathname(url1)).to.be.true; + + // defined, true/false + expect(matchPathname(url1, '/')).to.be.false; + expect(matchPathname(url1, '/en/getting-started')).to.be.true; + }); + + it('matches pathname (with wildcards)', async () => { + // defined, true/false + expect(matchPathname(url1, '/en/**', true)).to.be.true; + expect(matchPathname(url1, '/en/*', true)).to.be.true; + expect(matchPathname(url1, '/**', true)).to.be.true; + + expect(matchPathname(url2, '/**', true)).to.be.false; + expect(matchPathname(url2, '/*', true)).to.be.false; + }); + + it('matches patterns', async () => { + expect(matchPattern(url1, {})).to.be.true; + + expect( + matchPattern(url1, { + protocol: 'https', + }) + ).to.be.true; + + expect( + matchPattern(url1, { + protocol: 'https', + hostname: '**.astro.build', + }) + ).to.be.true; + + expect( + matchPattern(url1, { + protocol: 'https', + hostname: '**.astro.build', + pathname: '/en/**', + }) + ).to.be.true; + + expect( + matchPattern(url4, { + protocol: 'https', + hostname: 'example.com', + }) + ).to.be.false; + }); + }); +}); From d8a294c36d326792c84d1a63f78364318a67695b Mon Sep 17 00:00:00 2001 From: Julien Barbay Date: Tue, 25 Jul 2023 15:02:39 +0700 Subject: [PATCH 05/16] fix: removed some logging --- packages/astro/src/assets/utils/remotePattern.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/astro/src/assets/utils/remotePattern.ts b/packages/astro/src/assets/utils/remotePattern.ts index c8d88c68e0ad..7708b42e7380 100644 --- a/packages/astro/src/assets/utils/remotePattern.ts +++ b/packages/astro/src/assets/utils/remotePattern.ts @@ -26,7 +26,6 @@ export function matchHostname(url: URL, hostname?: string, allowWildcard?: boole if (!hostname) { return true; } else if (!allowWildcard || !hostname.startsWith('*')) { - console.log(hostname, url.hostname); return hostname === url.hostname; } else if (hostname.startsWith('**.')) { const slicedHostname = hostname.slice(2); // ** length From 3c111a73cd2fd8fea407a93280097e236b89dfcf Mon Sep 17 00:00:00 2001 From: Julien Barbay Date: Tue, 25 Jul 2023 23:36:32 +0700 Subject: [PATCH 06/16] fix: href should be first --- packages/astro/src/assets/services/service.ts | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/astro/src/assets/services/service.ts b/packages/astro/src/assets/services/service.ts index aa9f476d178a..95da83b5d712 100644 --- a/packages/astro/src/assets/services/service.ts +++ b/packages/astro/src/assets/services/service.ts @@ -210,27 +210,27 @@ export const baseService: Omit = { }; }, getURL(options, serviceConfig, assetsConfig) { - const PARAMS: Record = { - w: 'width', - h: 'height', - q: 'quality', - f: 'format', - }; - - const searchParams = Object.entries(PARAMS).reduce((params, [param, key]) => { - options[key] && params.append(param, options[key].toString()); - return params; - }, new URLSearchParams()); + const searchParams = new URLSearchParams(); if (isESMImportedImage(options.src)) { searchParams.append('href', options.src.src); } else if (isRemoteAllowed(options.src, assetsConfig)) { searchParams.append('href', options.src); } else { - // ignore non http strings return options.src; } + const PARAMS: Record = { + w: 'width', + h: 'height', + q: 'quality', + f: 'format', + }; + + Object.entries(PARAMS).forEach(([param, key]) => { + options[key] && searchParams.append(param, options[key].toString()); + }); + const imageEndpoint = joinPaths(import.meta.env.BASE_URL, '/_image'); return `${imageEndpoint}?${searchParams}`; }, From 6ad376ddb09ed160f659f6146fe5b75a1d2cc89c Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Wed, 16 Aug 2023 14:13:51 +0200 Subject: [PATCH 07/16] refactor: fix tests and refactor remote support into generate --- packages/astro/package.json | 2 + packages/astro/src/@types/astro.ts | 4 +- packages/astro/src/assets/build/generate.ts | 172 ++++++++++++++++++ packages/astro/src/assets/build/remote.ts | 55 ++++++ packages/astro/src/assets/generate.ts | 127 ------------- packages/astro/src/assets/image-endpoint.ts | 10 +- packages/astro/src/assets/internal.ts | 27 ++- packages/astro/src/assets/services/service.ts | 27 +-- packages/astro/src/assets/types.ts | 2 +- .../astro/src/assets/utils/transformToPath.ts | 11 +- .../astro/src/assets/vite-plugin-assets.ts | 60 +----- packages/astro/src/core/build/generate.ts | 2 +- packages/astro/test/test-image-service.js | 4 +- pnpm-lock.yaml | 6 + 14 files changed, 290 insertions(+), 219 deletions(-) create mode 100644 packages/astro/src/assets/build/generate.ts create mode 100644 packages/astro/src/assets/build/remote.ts delete mode 100644 packages/astro/src/assets/generate.ts diff --git a/packages/astro/package.json b/packages/astro/package.json index fd23ea87bc0a..aaede8e1d561 100644 --- a/packages/astro/package.json +++ b/packages/astro/package.json @@ -147,6 +147,7 @@ "github-slugger": "^2.0.0", "gray-matter": "^4.0.3", "html-escaper": "^3.0.3", + "http-cache-semantics": "^4.1.1", "js-yaml": "^4.1.0", "kleur": "^4.1.4", "magic-string": "^0.30.2", @@ -186,6 +187,7 @@ "@types/estree": "^0.0.51", "@types/hast": "^2.3.4", "@types/html-escaper": "^3.0.0", + "@types/http-cache-semantics": "^4.0.1", "@types/js-yaml": "^4.0.5", "@types/mime": "^2.0.3", "@types/mocha": "^9.1.1", diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index b2a22739b067..89b199d95fb7 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -13,6 +13,7 @@ import type { AddressInfo } from 'node:net'; import type * as rollup from 'rollup'; import type { TsConfigJson } from 'tsconfig-resolver'; import type * as vite from 'vite'; +import type { RemotePattern } from '../assets/utils/remotePattern'; import type { SerializedSSRManifest } from '../core/app/types'; import type { PageBuildData } from '../core/build/types'; import type { AstroConfigType } from '../core/config'; @@ -21,7 +22,6 @@ import type { AstroCookies } from '../core/cookies'; import type { LogOptions, LoggerLevel } from '../core/logger/core'; import type { AstroComponentFactory, AstroComponentInstance } from '../runtime/server'; import type { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from './../core/constants.js'; -import type { RemotePattern } from '../assets/utils/remotePattern'; export type { MarkdownHeading, MarkdownMetadata, @@ -1963,7 +1963,7 @@ export interface SSRLoadedRenderer extends AstroRenderer { export type HookParameters< Hook extends keyof AstroIntegration['hooks'], - Fn = AstroIntegration['hooks'][Hook] + Fn = AstroIntegration['hooks'][Hook], > = Fn extends (...args: any) => any ? Parameters[0] : never; export interface AstroIntegration { diff --git a/packages/astro/src/assets/build/generate.ts b/packages/astro/src/assets/build/generate.ts new file mode 100644 index 000000000000..532766206a3f --- /dev/null +++ b/packages/astro/src/assets/build/generate.ts @@ -0,0 +1,172 @@ +import fs, { readFileSync } from 'node:fs'; +import { basename, join } from 'node:path/posix'; +import type { StaticBuildOptions } from '../../core/build/types.js'; +import { warn } from '../../core/logger/core.js'; +import { prependForwardSlash } from '../../core/path.js'; +import { isServerLikeOutput } from '../../prerender/utils.js'; +import { getConfiguredImageService, isESMImportedImage, isRemoteImage } from '../internal.js'; +import type { LocalImageService } from '../services/service.js'; +import type { ImageTransform } from '../types.js'; +import { loadRemoteImage, type RemoteCacheEntry } from './remote.js'; + +interface GenerationDataUncached { + cached: false; + weight: { + before: number; + after: number; + }; +} + +interface GenerationDataCached { + cached: true; +} + +type GenerationData = GenerationDataUncached | GenerationDataCached; + +export async function generateImage( + buildOpts: StaticBuildOptions, + options: ImageTransform, + filepath: string +): Promise { + let useCache = true; + const assetsCacheDir = new URL('assets/', buildOpts.settings.config.cacheDir); + + // Ensure that the cache directory exists + try { + await fs.promises.mkdir(assetsCacheDir, { recursive: true }); + } catch (err) { + warn( + buildOpts.logging, + 'astro:assets', + `An error was encountered while creating the cache directory. Proceeding without caching. Error: ${err}` + ); + useCache = false; + } + + let serverRoot: URL, clientRoot: URL; + if (isServerLikeOutput(buildOpts.settings.config)) { + serverRoot = buildOpts.settings.config.build.server; + clientRoot = buildOpts.settings.config.build.client; + } else { + serverRoot = buildOpts.settings.config.outDir; + clientRoot = buildOpts.settings.config.outDir; + } + + const finalFileURL = new URL('.' + filepath, clientRoot); + const finalFolderURL = new URL('./', finalFileURL); + const cacheFile = basename(filepath) + (isRemoteImage(options.src) ? '.json' : ''); + const cachedFileURL = new URL(cacheFile, assetsCacheDir); + + const isLocalImage = isESMImportedImage(options.src); + + await fs.promises.mkdir(finalFolderURL, { recursive: true }); + + // CHeck if we have a cached entry first + try { + if (isLocalImage) { + await fs.promises.copyFile(cachedFileURL, finalFileURL); + + return { + cached: true, + }; + } else { + const JSONData = JSON.parse(readFileSync(cachedFileURL, 'utf-8')) as RemoteCacheEntry; + + console.log(JSONData.expires, Date.now()); + + // If the cache entry is not expired, use it + if (JSONData.expires < Date.now()) { + await fs.promises.writeFile(finalFileURL, Buffer.from(JSONData.data, 'base64')); + + return { + cached: true, + }; + } + } + } catch (e) { + // no-op. We'll generate the image below if copying from cache failed + } + + // The original filepath or URL from the image transform + const originalImagePath = isESMImportedImage(options.src) ? options.src.src : options.src; + + let imageData; + let resultData: { data: Buffer | undefined; expires: number | undefined } = { + data: undefined, + expires: undefined, + }; + if (isLocalImage) { + imageData = await fs.promises.readFile( + new URL( + '.' + + prependForwardSlash( + join(buildOpts.settings.config.build.assets, basename(originalImagePath)) + ), + serverRoot + ) + ); + } else { + const remoteImage = await loadRemoteImage(originalImagePath); + + if (!remoteImage) { + throw new Error(`Could not load remote image ${originalImagePath}.`); + } + + resultData.expires = remoteImage?.expires; + imageData = remoteImage?.data; + } + + const imageService = (await getConfiguredImageService()) as LocalImageService; + resultData.data = ( + await imageService.transform( + imageData, + { ...options, src: originalImagePath }, + buildOpts.settings.config.image + ) + ).data; + + if (useCache) { + try { + if (isLocalImage) { + await fs.promises.writeFile(cachedFileURL, resultData.data); + await fs.promises.copyFile(cachedFileURL, finalFileURL); + } else { + await fs.promises.writeFile( + cachedFileURL, + JSON.stringify({ + data: Buffer.from(resultData.data).toString('base64'), + expires: resultData.expires, + }) + ); + await fs.promises.writeFile(finalFileURL, resultData.data); + } + } catch (e) { + warn( + buildOpts.logging, + 'astro:assets', + `An error was encountered while creating the cache directory. Proceeding without caching. Error: ${e}` + ); + await fs.promises.writeFile(finalFileURL, resultData.data); + } + } else { + await fs.promises.writeFile(finalFileURL, resultData.data); + } + + return { + cached: false, + weight: { + before: Math.trunc(imageData.byteLength / 1024), + after: Math.trunc(Buffer.from(resultData.data).byteLength / 1024), + }, + }; +} + +export function getStaticImageList(): Iterable< + [string, { path: string; options: ImageTransform }] +> { + if (!globalThis?.astroAsset?.staticImages) { + return []; + } + + return globalThis.astroAsset.staticImages?.entries(); +} diff --git a/packages/astro/src/assets/build/remote.ts b/packages/astro/src/assets/build/remote.ts new file mode 100644 index 000000000000..57206a553f39 --- /dev/null +++ b/packages/astro/src/assets/build/remote.ts @@ -0,0 +1,55 @@ +import CachePolicy from 'http-cache-semantics'; + +export type RemoteCacheEntry = { data: string; expires: number }; + +export async function loadRemoteImage(src: string) { + try { + if (src.startsWith('//')) { + src = `https:${src}`; + } + + const req = new Request(src); + const res = await fetch(req); + + if (!res.ok) { + return undefined; + } + + // calculate an expiration date based on the response's TTL + const policy = new CachePolicy(webToCachePolicyRequest(req), webToCachePolicyResponse(res)); + const expires = policy.storable() ? policy.timeToLive() : 0; + + return { + data: Buffer.from(await res.arrayBuffer()), + expires: Date.now() + expires, + }; + } catch (err: unknown) { + console.error(err); + return undefined; + } +} + +function webToCachePolicyRequest({ url, method, headers: _headers }: Request): CachePolicy.Request { + let headers: CachePolicy.Headers = {}; + // Be defensive here due to a cookie header bug in node@18.14.1 + undici + try { + headers = Object.fromEntries(_headers.entries()); + } catch {} + return { + method, + url, + headers, + }; +} + +function webToCachePolicyResponse({ status, headers: _headers }: Response): CachePolicy.Response { + let headers: CachePolicy.Headers = {}; + // Be defensive here due to a cookie header bug in node@18.14.1 + undici + try { + headers = Object.fromEntries(_headers.entries()); + } catch {} + return { + status, + headers, + }; +} diff --git a/packages/astro/src/assets/generate.ts b/packages/astro/src/assets/generate.ts deleted file mode 100644 index 90d81a02a27f..000000000000 --- a/packages/astro/src/assets/generate.ts +++ /dev/null @@ -1,127 +0,0 @@ -import fs from 'node:fs'; -import { basename, join } from 'node:path/posix'; -import type { StaticBuildOptions } from '../core/build/types.js'; -import { warn } from '../core/logger/core.js'; -import { prependForwardSlash } from '../core/path.js'; -import { isServerLikeOutput } from '../prerender/utils.js'; -import { getConfiguredImageService, isESMImportedImage } from './internal.js'; -import type { LocalImageService } from './services/service.js'; -import type { ImageTransform } from './types.js'; - -interface GenerationDataUncached { - cached: false; - weight: { - before: number; - after: number; - }; -} - -interface GenerationDataCached { - cached: true; -} - -type GenerationData = GenerationDataUncached | GenerationDataCached; - -export async function generateImage( - buildOpts: StaticBuildOptions, - options: ImageTransform, - filepath: string -): Promise { - let useCache = true; - const assetsCacheDir = new URL('assets/', buildOpts.settings.config.cacheDir); - - // Ensure that the cache directory exists - try { - await fs.promises.mkdir(assetsCacheDir, { recursive: true }); - } catch (err) { - warn( - buildOpts.logging, - 'astro:assets', - `An error was encountered while creating the cache directory. Proceeding without caching. Error: ${err}` - ); - useCache = false; - } - - let serverRoot: URL, clientRoot: URL; - if (isServerLikeOutput(buildOpts.settings.config)) { - serverRoot = buildOpts.settings.config.build.server; - clientRoot = buildOpts.settings.config.build.client; - } else { - serverRoot = buildOpts.settings.config.outDir; - clientRoot = buildOpts.settings.config.outDir; - } - - const finalFileURL = new URL('.' + filepath, clientRoot); - const finalFolderURL = new URL('./', finalFileURL); - const cachedFileURL = new URL(basename(filepath), assetsCacheDir); - - try { - await fs.promises.copyFile(cachedFileURL, finalFileURL); - - return { - cached: true, - }; - } catch (e) { - // no-op - } - - // The original file's path (the `src` attribute of the ESM imported image passed by the user) - // if it's a string we assumed it was cached before in the cacheDir/fetch folder. - const fileURL = isESMImportedImage(options.src) - ? new URL( - '.' + - prependForwardSlash( - join(buildOpts.settings.config.build.assets, basename(options.src.src)) - ), - serverRoot - ) - : new URL( - join('.', 'remote-assets', new URL(options.src).pathname), - buildOpts.settings.config.cacheDir - ); - - const fileData: Buffer = await fs.promises.readFile(fileURL); - - const imageService = (await getConfiguredImageService()) as LocalImageService; - const resultData = await imageService.transform( - fileData, - { ...options, src: isESMImportedImage(options.src) ? options.src.src : options.src }, - buildOpts.settings.config.image.service.config - ); - - await fs.promises.mkdir(finalFolderURL, { recursive: true }); - - if (useCache) { - try { - await fs.promises.writeFile(cachedFileURL, resultData.data); - await fs.promises.copyFile(cachedFileURL, finalFileURL); - } catch (e) { - warn( - buildOpts.logging, - 'astro:assets', - `An error was encountered while creating the cache directory. Proceeding without caching. Error: ${e}` - ); - await fs.promises.writeFile(finalFileURL, resultData.data); - } - } else { - await fs.promises.writeFile(finalFileURL, resultData.data); - } - - return { - cached: false, - weight: { - before: Math.trunc(fileData.byteLength / 1024), - after: Math.trunc(resultData.data.byteLength / 1024), - }, - }; -} - -export function getStaticImageList(): Iterable< - [string, { path: string; options: ImageTransform }] -> { - if (!globalThis?.astroAsset?.staticImages) { - return []; - } - - return globalThis.astroAsset.staticImages?.entries(); -} diff --git a/packages/astro/src/assets/image-endpoint.ts b/packages/astro/src/assets/image-endpoint.ts index 0553272c219c..2e55cb599f08 100644 --- a/packages/astro/src/assets/image-endpoint.ts +++ b/packages/astro/src/assets/image-endpoint.ts @@ -5,7 +5,7 @@ import { getConfiguredImageService } from './internal.js'; import { isLocalService } from './services/service.js'; import { etag } from './utils/etag.js'; // @ts-expect-error -import { imageServiceConfig } from 'astro:assets'; +import { imageConfig } from 'astro:assets'; async function loadRemoteImage(src: URL) { try { @@ -33,7 +33,7 @@ export const get: APIRoute = async ({ request }) => { } const url = new URL(request.url); - const transform = await imageService.parseURL(url, imageServiceConfig); + const transform = await imageService.parseURL(url, imageConfig); if (!transform?.src) { throw new Error('Incorrect transform returned by `parseURL`'); @@ -51,11 +51,7 @@ export const get: APIRoute = async ({ request }) => { return new Response('Not Found', { status: 404 }); } - const { data, format } = await imageService.transform( - inputBuffer, - transform, - imageServiceConfig - ); + const { data, format } = await imageService.transform(inputBuffer, transform, imageConfig); return new Response(data, { status: 200, diff --git a/packages/astro/src/assets/internal.ts b/packages/astro/src/assets/internal.ts index 4a759b482321..8bc3ec52aabc 100644 --- a/packages/astro/src/assets/internal.ts +++ b/packages/astro/src/assets/internal.ts @@ -33,8 +33,13 @@ export function isRemoteAllowed( domains = [], remotePatterns = [], }: Partial> -) { - const url = new URL(src); +): boolean | undefined { + let url: URL; + try { + url = new URL(src); + } catch (e) { + return undefined; + } return ( domains.some((domain) => matchHostname(url, domain)) || @@ -63,8 +68,7 @@ export async function getConfiguredImageService(): Promise { export async function getImage( options: ImageTransform | UnresolvedImageTransform, - serviceConfig: Record, - assetsConfig: AstroConfig['image'] + imageConfig: AstroConfig['image'] ): Promise { if (!options || typeof options !== 'object') { throw new AstroError({ @@ -85,14 +89,19 @@ export async function getImage( }; const validatedOptions = service.validateOptions - ? await service.validateOptions(resolvedOptions, serviceConfig) + ? await service.validateOptions(resolvedOptions, imageConfig) : resolvedOptions; - let imageURL = await service.getURL(validatedOptions, serviceConfig, assetsConfig); + let imageURL = await service.getURL(validatedOptions, imageConfig); // In build and for local services, we need to collect the requested parameters so we can generate the final images - if (isLocalService(service) && globalThis.astroAsset.addStaticImage) { - imageURL = await globalThis.astroAsset.addStaticImage(validatedOptions); + if ( + isLocalService(service) && + globalThis.astroAsset.addStaticImage && + // If `getURL` returned the same URL as the user provided, it means the service doesn't need to do anything + !(isRemoteImage(validatedOptions.src) && imageURL === validatedOptions.src) + ) { + imageURL = globalThis.astroAsset.addStaticImage(validatedOptions); } return { @@ -101,7 +110,7 @@ export async function getImage( src: imageURL, attributes: service.getHTMLAttributes !== undefined - ? service.getHTMLAttributes(validatedOptions, serviceConfig) + ? service.getHTMLAttributes(validatedOptions, imageConfig) : {}, }; } diff --git a/packages/astro/src/assets/services/service.ts b/packages/astro/src/assets/services/service.ts index 95da83b5d712..5af4a898be8b 100644 --- a/packages/astro/src/assets/services/service.ts +++ b/packages/astro/src/assets/services/service.ts @@ -24,6 +24,10 @@ export function parseQuality(quality: string): string | number { return result; } +type ImageConfig = Omit & { + service: { entrypoint: string; config: T }; +}; + interface SharedServiceProps = Record> { /** * Return the URL to the endpoint or URL your images are generated from. @@ -33,11 +37,7 @@ interface SharedServiceProps = Record * For external services, this should point to the URL your images are coming from, for instance, `/_vercel/image` * */ - getURL: ( - options: ImageTransform, - serviceConfig: T, - assetsConfig: AstroConfig['image'] - ) => string | Promise; + getURL: (options: ImageTransform, imageConfig: ImageConfig) => string | Promise; /** * Return any additional HTML attributes separate from `src` that your service requires to show the image properly. * @@ -46,7 +46,7 @@ interface SharedServiceProps = Record */ getHTMLAttributes?: ( options: ImageTransform, - serviceConfig: T + imageConfig: ImageConfig ) => Record | Promise>; /** * Validate and return the options passed by the user. @@ -58,7 +58,7 @@ interface SharedServiceProps = Record */ validateOptions?: ( options: ImageTransform, - serviceConfig: T + imageConfig: ImageConfig ) => ImageTransform | Promise; } @@ -79,7 +79,7 @@ export interface LocalImageService = Record ) => LocalImageTransform | undefined | Promise | Promise; /** * Performs the image transformations on the input image and returns both the binary data and @@ -88,7 +88,7 @@ export interface LocalImageService = Record ) => Promise<{ data: Buffer; format: ImageOutputFormat }>; } @@ -209,25 +209,26 @@ export const baseService: Omit = { decoding: attributes.decoding ?? 'async', }; }, - getURL(options, serviceConfig, assetsConfig) { + getURL(options, imageConfig) { const searchParams = new URLSearchParams(); if (isESMImportedImage(options.src)) { searchParams.append('href', options.src.src); - } else if (isRemoteAllowed(options.src, assetsConfig)) { + } else if (isRemoteAllowed(options.src, imageConfig)) { searchParams.append('href', options.src); } else { + // If it's not an imported image, nor is it allowed using the current domains or remote patterns, we'll just return the original URL return options.src; } - const PARAMS: Record = { + const params: Record = { w: 'width', h: 'height', q: 'quality', f: 'format', }; - Object.entries(PARAMS).forEach(([param, key]) => { + Object.entries(params).forEach(([param, key]) => { options[key] && searchParams.append(param, options[key].toString()); }); diff --git a/packages/astro/src/assets/types.ts b/packages/astro/src/assets/types.ts index 12e504330504..9c5990cb77ae 100644 --- a/packages/astro/src/assets/types.ts +++ b/packages/astro/src/assets/types.ts @@ -11,7 +11,7 @@ declare global { // eslint-disable-next-line no-var var astroAsset: { imageService?: ImageService; - addStaticImage?: ((options: ImageTransform) => string | Promise) | undefined; + addStaticImage?: ((options: ImageTransform) => string) | undefined; staticImages?: Map; }; } diff --git a/packages/astro/src/assets/utils/transformToPath.ts b/packages/astro/src/assets/utils/transformToPath.ts index 04ddee0a193f..d5535137be04 100644 --- a/packages/astro/src/assets/utils/transformToPath.ts +++ b/packages/astro/src/assets/utils/transformToPath.ts @@ -5,14 +5,13 @@ import { isESMImportedImage } from '../internal.js'; import type { ImageTransform } from '../types.js'; export function propsToFilename(transform: ImageTransform, hash: string) { - if (!isESMImportedImage(transform.src)) { - return transform.src; - } - - let filename = removeQueryString(transform.src.src); + let filename = removeQueryString( + isESMImportedImage(transform.src) ? transform.src.src : transform.src + ); const ext = extname(filename); filename = basename(filename, ext); - const outputExt = transform.format ? `.${transform.format}` : ext; + + let outputExt = transform.format ? `.${transform.format}` : ext; return `/${filename}_${hash}${outputExt}`; } diff --git a/packages/astro/src/assets/vite-plugin-assets.ts b/packages/astro/src/assets/vite-plugin-assets.ts index 7b1d232c2a98..0f00e0ecb569 100644 --- a/packages/astro/src/assets/vite-plugin-assets.ts +++ b/packages/astro/src/assets/vite-plugin-assets.ts @@ -1,12 +1,10 @@ -import { writeFile, mkdir } from 'node:fs/promises'; -import { existsSync } from 'node:fs'; import { bold } from 'kleur/colors'; import MagicString from 'magic-string'; import { fileURLToPath } from 'node:url'; import type * as vite from 'vite'; import { normalizePath } from 'vite'; import type { AstroPluginOptions, ImageTransform } from '../@types/astro'; -import { info, error } from '../core/logger/core.js'; +import { error } from '../core/logger/core.js'; import { appendForwardSlash, joinPaths, @@ -14,7 +12,6 @@ import { removeQueryString, } from '../core/path.js'; import { VIRTUAL_MODULE_ID, VIRTUAL_SERVICE_ID } from './consts.js'; -import { isRemoteAllowed, isRemoteImage } from './internal.js'; import { emitESMImage } from './utils/emitAsset.js'; import { hashTransform, propsToFilename } from './utils/transformToPath.js'; @@ -87,9 +84,8 @@ export default function assets({ import { getImage as getImageInternal } from "astro/assets"; export { default as Image } from "astro/components/Image.astro"; - export const imageServiceConfig = ${JSON.stringify(settings.config.image.service.config)}; - export const astroAssetsConfig = ${JSON.stringify(settings.config.image)}; - export const getImage = async (options) => await getImageInternal(options, imageServiceConfig, astroAssetsConfig); + export const imageConfig = ${JSON.stringify(settings.config.image)}; + export const getImage = async (options) => await getImageInternal(options, imageConfig); `; } }, @@ -98,7 +94,7 @@ export default function assets({ return; } - globalThis.astroAsset.addStaticImage = async (options) => { + globalThis.astroAsset.addStaticImage = (options) => { if (!globalThis.astroAsset.staticImages) { globalThis.astroAsset.staticImages = new Map< string, @@ -106,58 +102,20 @@ export default function assets({ >(); } - // hard skip on non allowed remote images - if ( - typeof options.src === 'string' && - !isRemoteAllowed(options.src, settings.config.image) - ) { - return options.src; - } - - // in case of remote images - if (isRemoteImage(options.src)) { - const remoteCacheDir = new URL('remote-assets/', settings.config.cacheDir); - await mkdir(remoteCacheDir, { recursive: true }); - - const remoteFileURL = new URL(options.src); - const cachedFileURL = new URL('.' + remoteFileURL.pathname, remoteCacheDir); - - // there's no async api for exists :( - if (!existsSync(cachedFileURL)) { - info( - logging, - 'astro:assets', - `${bold('downloading remote asset')} ${remoteFileURL.href} -> ${cachedFileURL.href}` - ); - const res = await fetch(remoteFileURL); - const imgBytes = await res.arrayBuffer(); - await writeFile(cachedFileURL, Buffer.from(imgBytes)); - } - } - const hash = hashTransform(options, settings.config.image.service.entrypoint); let filePath: string; - const hasHash = globalThis.astroAsset.staticImages.has(hash); - if (hasHash) { + if (globalThis.astroAsset.staticImages.has(hash)) { filePath = globalThis.astroAsset.staticImages.get(hash)!.path; - } else if (isRemoteImage(options.src)) { - const { pathname } = new URL(options.src); - filePath = options.format - ? pathname.slice(0, pathname.lastIndexOf('.') + 1) + options.format - : pathname; } else { - filePath = propsToFilename(options, hash); - } + filePath = prependForwardSlash( + joinPaths(settings.config.build.assets, propsToFilename(options, hash)) + ); - if (!hasHash) { - filePath = prependForwardSlash(joinPaths(settings.config.build.assets, filePath)); globalThis.astroAsset.staticImages.set(hash, { path: filePath, options: options }); } - if (isRemoteImage(options.src)) { - return filePath; - } else if (settings.config.build.assetsPrefix) { + if (settings.config.build.assetsPrefix) { return joinPaths(settings.config.build.assetsPrefix, filePath); } else { return prependForwardSlash(joinPaths(settings.config.base, filePath)); diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 708295296399..a78a46883ab8 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -19,7 +19,7 @@ import type { import { generateImage as generateImageInternal, getStaticImageList, -} from '../../assets/generate.js'; +} from '../../assets/build/generate.js'; import { eachPageDataFromEntryPoint, eachRedirectPageData, diff --git a/packages/astro/test/test-image-service.js b/packages/astro/test/test-image-service.js index ebdbb0765d40..bcf623caa809 100644 --- a/packages/astro/test/test-image-service.js +++ b/packages/astro/test/test-image-service.js @@ -17,8 +17,8 @@ export default { ...baseService, getHTMLAttributes(options, serviceConfig) { options['data-service'] = 'my-custom-service'; - if (serviceConfig.foo) { - options['data-service-config'] = serviceConfig.foo; + if (serviceConfig.service.config.foo) { + options['data-service-config'] = serviceConfig.service.config.foo; } return baseService.getHTMLAttributes(options); }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c4cc9712c3a8..c040dfb22f25 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -578,6 +578,9 @@ importers: html-escaper: specifier: ^3.0.3 version: 3.0.3 + http-cache-semantics: + specifier: ^4.1.1 + version: 4.1.1 js-yaml: specifier: ^4.1.0 version: 4.1.0 @@ -690,6 +693,9 @@ importers: '@types/html-escaper': specifier: ^3.0.0 version: 3.0.0 + '@types/http-cache-semantics': + specifier: ^4.0.1 + version: 4.0.1 '@types/js-yaml': specifier: ^4.0.5 version: 4.0.5 From 024741a0a9a177319f1e2f2ec662e67cbc34246e Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Wed, 16 Aug 2023 15:10:24 +0200 Subject: [PATCH 08/16] feat: update Vercel service --- packages/astro/src/assets/image-endpoint.ts | 7 ++++++- packages/integrations/vercel/src/image/build-service.ts | 2 +- packages/integrations/vercel/src/image/dev-service.ts | 2 +- packages/integrations/vercel/src/image/shared.ts | 4 ++-- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/astro/src/assets/image-endpoint.ts b/packages/astro/src/assets/image-endpoint.ts index 2e55cb599f08..15e22106704c 100644 --- a/packages/astro/src/assets/image-endpoint.ts +++ b/packages/astro/src/assets/image-endpoint.ts @@ -1,7 +1,7 @@ import mime from 'mime/lite.js'; import type { APIRoute } from '../@types/astro.js'; import { isRemotePath } from '../core/path.js'; -import { getConfiguredImageService } from './internal.js'; +import { getConfiguredImageService, isRemoteAllowed } from './internal.js'; import { isLocalService } from './services/service.js'; import { etag } from './utils/etag.js'; // @ts-expect-error @@ -45,6 +45,11 @@ export const get: APIRoute = async ({ request }) => { const sourceUrl = isRemotePath(transform.src) ? new URL(transform.src) : new URL(transform.src, url.origin); + + if (isRemoteAllowed(transform.src, imageConfig) === false) { + return new Response('Forbidden', { status: 403 }); + } + inputBuffer = await loadRemoteImage(sourceUrl); if (!inputBuffer) { diff --git a/packages/integrations/vercel/src/image/build-service.ts b/packages/integrations/vercel/src/image/build-service.ts index 973ceb22a7aa..63a37a5fee89 100644 --- a/packages/integrations/vercel/src/image/build-service.ts +++ b/packages/integrations/vercel/src/image/build-service.ts @@ -3,7 +3,7 @@ import { isESMImportedImage, sharedValidateOptions } from './shared'; const service: ExternalImageService = { validateOptions: (options, serviceOptions) => - sharedValidateOptions(options, serviceOptions, 'production'), + sharedValidateOptions(options, serviceOptions.service.config, 'production'), getHTMLAttributes(options) { const { inputtedWidth, ...props } = options; diff --git a/packages/integrations/vercel/src/image/dev-service.ts b/packages/integrations/vercel/src/image/dev-service.ts index 04df9932a504..be6360fe3f8b 100644 --- a/packages/integrations/vercel/src/image/dev-service.ts +++ b/packages/integrations/vercel/src/image/dev-service.ts @@ -5,7 +5,7 @@ import { sharedValidateOptions } from './shared'; const service: LocalImageService = { validateOptions: (options, serviceOptions) => - sharedValidateOptions(options, serviceOptions, 'development'), + sharedValidateOptions(options, serviceOptions.service.config, 'development'), getHTMLAttributes(options, serviceOptions) { const { inputtedWidth, ...props } = options; diff --git a/packages/integrations/vercel/src/image/shared.ts b/packages/integrations/vercel/src/image/shared.ts index 0b6db2037805..473750fae404 100644 --- a/packages/integrations/vercel/src/image/shared.ts +++ b/packages/integrations/vercel/src/image/shared.ts @@ -89,10 +89,10 @@ export function getImageConfig( export function sharedValidateOptions( options: ImageTransform, - serviceOptions: Record, + serviceConfig: Record, mode: 'development' | 'production' ) { - const vercelImageOptions = serviceOptions as VercelImageConfig; + const vercelImageOptions = serviceConfig as VercelImageConfig; if ( mode === 'development' && From f998e7bfe252839c338555a8a802ab271be24691 Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Wed, 16 Aug 2023 15:26:35 +0200 Subject: [PATCH 09/16] chore: changeset --- .changeset/itchy-pants-grin.md | 5 +++++ .changeset/sour-frogs-shout.md | 25 +++++++++++++++++++++ packages/astro/src/assets/image-endpoint.ts | 2 +- 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 .changeset/itchy-pants-grin.md create mode 100644 .changeset/sour-frogs-shout.md diff --git a/.changeset/itchy-pants-grin.md b/.changeset/itchy-pants-grin.md new file mode 100644 index 000000000000..2ab292f27d30 --- /dev/null +++ b/.changeset/itchy-pants-grin.md @@ -0,0 +1,5 @@ +--- +'@astrojs/vercel': patch +--- + +Update image support to work with latest version of Astro diff --git a/.changeset/sour-frogs-shout.md b/.changeset/sour-frogs-shout.md new file mode 100644 index 000000000000..4d1b374d75d7 --- /dev/null +++ b/.changeset/sour-frogs-shout.md @@ -0,0 +1,25 @@ +--- +'astro': patch +--- + +Added support for optimizing remote images to `astro:assets`. This comes with two new parameters to specify which domains and host patterns are authorized for remote images. + +For example, the following configuration will only allow remote images from `astro.build` to be optimized: + +```ts +export default defineConfig({ + image: { + domains: ["astro.build/"], + } +}); +``` + +The following configuration will only allo remote images from HTTPs hosts: + +```ts +export default defineConfig({ + image: { + remotePatterns: [{ protocol: "https" }], + } +}); +``` diff --git a/packages/astro/src/assets/image-endpoint.ts b/packages/astro/src/assets/image-endpoint.ts index 15e22106704c..fa62cbdd128a 100644 --- a/packages/astro/src/assets/image-endpoint.ts +++ b/packages/astro/src/assets/image-endpoint.ts @@ -46,7 +46,7 @@ export const get: APIRoute = async ({ request }) => { ? new URL(transform.src) : new URL(transform.src, url.origin); - if (isRemoteAllowed(transform.src, imageConfig) === false) { + if (isRemotePath(transform.src) && isRemoteAllowed(transform.src, imageConfig) === false) { return new Response('Forbidden', { status: 403 }); } From 27afc46dc9e32aeadbc1c38f31ed27829a046575 Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Wed, 16 Aug 2023 17:38:49 +0200 Subject: [PATCH 10/16] fix: answer to code feedback --- packages/astro/src/assets/build/generate.ts | 62 +++++++++++---------- packages/astro/src/assets/build/remote.ts | 43 +++++++------- 2 files changed, 52 insertions(+), 53 deletions(-) diff --git a/packages/astro/src/assets/build/generate.ts b/packages/astro/src/assets/build/generate.ts index 532766206a3f..b78800a4382a 100644 --- a/packages/astro/src/assets/build/generate.ts +++ b/packages/astro/src/assets/build/generate.ts @@ -4,9 +4,9 @@ import type { StaticBuildOptions } from '../../core/build/types.js'; import { warn } from '../../core/logger/core.js'; import { prependForwardSlash } from '../../core/path.js'; import { isServerLikeOutput } from '../../prerender/utils.js'; -import { getConfiguredImageService, isESMImportedImage, isRemoteImage } from '../internal.js'; +import { getConfiguredImageService, isESMImportedImage } from '../internal.js'; import type { LocalImageService } from '../services/service.js'; -import type { ImageTransform } from '../types.js'; +import type { ImageMetadata, ImageTransform } from '../types.js'; import { loadRemoteImage, type RemoteCacheEntry } from './remote.js'; interface GenerationDataUncached { @@ -52,16 +52,18 @@ export async function generateImage( clientRoot = buildOpts.settings.config.outDir; } + const isLocalImage = isESMImportedImage(options.src); + const finalFileURL = new URL('.' + filepath, clientRoot); const finalFolderURL = new URL('./', finalFileURL); - const cacheFile = basename(filepath) + (isRemoteImage(options.src) ? '.json' : ''); - const cachedFileURL = new URL(cacheFile, assetsCacheDir); - const isLocalImage = isESMImportedImage(options.src); + // For remote images, instead of saving the image directly, we save a JSON file with the image data and expiration date from the server + const cacheFile = basename(filepath) + (isLocalImage ? '' : '.json'); + const cachedFileURL = new URL(cacheFile, assetsCacheDir); await fs.promises.mkdir(finalFolderURL, { recursive: true }); - // CHeck if we have a cached entry first + // Check if we have a cached entry first try { if (isLocalImage) { await fs.promises.copyFile(cachedFileURL, finalFileURL); @@ -72,8 +74,6 @@ export async function generateImage( } else { const JSONData = JSON.parse(readFileSync(cachedFileURL, 'utf-8')) as RemoteCacheEntry; - console.log(JSONData.expires, Date.now()); - // If the cache entry is not expired, use it if (JSONData.expires < Date.now()) { await fs.promises.writeFile(finalFileURL, Buffer.from(JSONData.data, 'base64')); @@ -83,18 +83,25 @@ export async function generateImage( }; } } - } catch (e) { - // no-op. We'll generate the image below if copying from cache failed + } catch (e: any) { + if (e.code !== 'ENOENT') { + throw new Error(`An error was encountered while reading the cache file. Error: ${e}`); + } + // If the cache file doesn't exist, just move on, and we'll generate it } // The original filepath or URL from the image transform - const originalImagePath = isESMImportedImage(options.src) ? options.src.src : options.src; + const originalImagePath = isLocalImage + ? (options.src as ImageMetadata).src + : (options.src as string); let imageData; let resultData: { data: Buffer | undefined; expires: number | undefined } = { data: undefined, expires: undefined, }; + + // If the image is local, we can just read it directly, otherwise we need to download it if (isLocalImage) { imageData = await fs.promises.readFile( new URL( @@ -107,13 +114,8 @@ export async function generateImage( ); } else { const remoteImage = await loadRemoteImage(originalImagePath); - - if (!remoteImage) { - throw new Error(`Could not load remote image ${originalImagePath}.`); - } - - resultData.expires = remoteImage?.expires; - imageData = remoteImage?.data; + resultData.expires = remoteImage.expires; + imageData = remoteImage.data; } const imageService = (await getConfiguredImageService()) as LocalImageService; @@ -125,11 +127,11 @@ export async function generateImage( ) ).data; - if (useCache) { - try { + try { + // Write the cache entry + if (useCache) { if (isLocalImage) { await fs.promises.writeFile(cachedFileURL, resultData.data); - await fs.promises.copyFile(cachedFileURL, finalFileURL); } else { await fs.promises.writeFile( cachedFileURL, @@ -138,23 +140,23 @@ export async function generateImage( expires: resultData.expires, }) ); - await fs.promises.writeFile(finalFileURL, resultData.data); } - } catch (e) { - warn( - buildOpts.logging, - 'astro:assets', - `An error was encountered while creating the cache directory. Proceeding without caching. Error: ${e}` - ); - await fs.promises.writeFile(finalFileURL, resultData.data); } - } else { + } catch (e) { + warn( + buildOpts.logging, + 'astro:assets', + `An error was encountered while creating the cache directory. Proceeding without caching. Error: ${e}` + ); + } finally { + // Write the final file await fs.promises.writeFile(finalFileURL, resultData.data); } return { cached: false, weight: { + // Divide by 1024 to get size in kilobytes before: Math.trunc(imageData.byteLength / 1024), after: Math.trunc(Buffer.from(resultData.data).byteLength / 1024), }, diff --git a/packages/astro/src/assets/build/remote.ts b/packages/astro/src/assets/build/remote.ts index 57206a553f39..0197af4e7829 100644 --- a/packages/astro/src/assets/build/remote.ts +++ b/packages/astro/src/assets/build/remote.ts @@ -3,30 +3,27 @@ import CachePolicy from 'http-cache-semantics'; export type RemoteCacheEntry = { data: string; expires: number }; export async function loadRemoteImage(src: string) { - try { - if (src.startsWith('//')) { - src = `https:${src}`; - } - - const req = new Request(src); - const res = await fetch(req); - - if (!res.ok) { - return undefined; - } - - // calculate an expiration date based on the response's TTL - const policy = new CachePolicy(webToCachePolicyRequest(req), webToCachePolicyResponse(res)); - const expires = policy.storable() ? policy.timeToLive() : 0; - - return { - data: Buffer.from(await res.arrayBuffer()), - expires: Date.now() + expires, - }; - } catch (err: unknown) { - console.error(err); - return undefined; + if (src.startsWith('//')) { + src = `https:${src}`; + } + + const req = new Request(src); + const res = await fetch(req); + + if (!res.ok) { + throw new Error( + `Failed to load remote image ${src}. The request did not return a 200 OK response. (received ${res.status}))` + ); } + + // calculate an expiration date based on the response's TTL + const policy = new CachePolicy(webToCachePolicyRequest(req), webToCachePolicyResponse(res)); + const expires = policy.storable() ? policy.timeToLive() : 0; + + return { + data: Buffer.from(await res.arrayBuffer()), + expires: Date.now() + expires, + }; } function webToCachePolicyRequest({ url, method, headers: _headers }: Request): CachePolicy.Request { From 66ce3b8b7c433f441804c369d2bd2affacab42ce Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Wed, 16 Aug 2023 18:18:14 +0200 Subject: [PATCH 11/16] docs: update with docs feedback --- .changeset/sour-frogs-shout.md | 6 ++-- packages/astro/src/@types/astro.ts | 47 +++++++++++++-------------- packages/astro/src/assets/internal.ts | 11 +++---- 3 files changed, 30 insertions(+), 34 deletions(-) diff --git a/.changeset/sour-frogs-shout.md b/.changeset/sour-frogs-shout.md index 4d1b374d75d7..25bee6bf94fe 100644 --- a/.changeset/sour-frogs-shout.md +++ b/.changeset/sour-frogs-shout.md @@ -2,14 +2,15 @@ 'astro': patch --- -Added support for optimizing remote images to `astro:assets`. This comes with two new parameters to specify which domains and host patterns are authorized for remote images. +Added support for optimizing remote images from authorized sources when using `astro:assets`. This comes with two new parameters to specify which domains (`image.domains`) and host patterns (`image.remotePatterns) are authorized for remote images. For example, the following configuration will only allow remote images from `astro.build` to be optimized: ```ts +// astro.config.mjs export default defineConfig({ image: { - domains: ["astro.build/"], + domains: ["astro.build"], } }); ``` @@ -17,6 +18,7 @@ export default defineConfig({ The following configuration will only allo remote images from HTTPs hosts: ```ts +// astro.config.mjs export default defineConfig({ image: { remotePatterns: [{ protocol: "https" }], diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index 89b199d95fb7..6fe28a438fb1 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -1016,19 +1016,19 @@ export interface AstroUserConfig { /** * @docs * @name image.domains (Experimental) - * @type {Array} + * @type {string[]} * @default `{domains: []}` - * @version 2.9.2 + * @version 2.10.10 * @description - * Defines a list of domains allowed to be optimized locally + * Defines a list of permitted image source domains for local image optimization. No other remote images will be optimized by Astro. * - * The value should be an array of domain strings of images allowed to be optimized locally. - * Wildcards are not permitted (see image.remotePatterns). + * This option requires an array of individual domain names as strings. Wildcards are not permitted. Instead, use [`image.remotePatterns`](#imageremotepatterns-experimental) to define a list of allowed source URL patterns. * * ```js + * // astro.config.mjs * { * image: { - * // Example: Enable the Sharp-based image service + * // Example: Allow remote image optimization from a single domain * domains: ['astro.build'], * }, * } @@ -1039,35 +1039,22 @@ export interface AstroUserConfig { /** * @docs * @name image.remotePatterns (Experimental) - * @type {Array} + * @type {RemotePattern[]} * @default `{remotePatterns: []}` - * @version 2.9.2 + * @version 2.10.10 * @description - * Defines a list of url patterns allowed to be optimized locally + * Defines a list of permitted image source URL patterns for local image optimization. * - * The patterns can contain 4 properties: + * `remotePatterns` can be configured with four properties: * 1. protocol * 2. hostname * 3. port * 4. pathname * - * Each property will be matched individually to the URL object representing the remote asset to be loaded. - * The rules for matching are: - * - * 1. protocol & port: strict equal.(`===`) - * 2. hostname: - * - if the hostname starts with '**.', all subdomains will be allowed ('endsWith') - * - if the hostname starts with '*.', only one level of subdomain will be allowed - * - strict equal otherwise - * 3. pathname: - * - if the pathname ends with '/**', all sub routes will be allowed ('startsWith') - * - if the pathname ends with '/*', only one level of sub route will be allowed - * - strict equal otherwise - * * ```js * { * image: { - * // Example: statically process all images from your aws s3 bucket + * // Example: allow processing all images from your aws s3 bucket * remotePatterns: [{ * protocol: 'https', * hostname: '**.amazonaws.com', @@ -1075,6 +1062,16 @@ export interface AstroUserConfig { * }, * } * ``` + * + * You can use wildcards to define the permitted `hostname` and `pathname` values as described below. Otherwise, only the exact values provided will be configured: + * `hostname`: + * - Start with '**.' to allow all subdomains ('endsWith'). + * - Start with '*.' to allow only one level of subdomain. + * + * `pathname`: + * - End with '/**' to allow all sub-routes ('startsWith'). + * - End with '/*' to allow only one level of sub-route. + */ remotePatterns?: Partial[]; }; @@ -1963,7 +1960,7 @@ export interface SSRLoadedRenderer extends AstroRenderer { export type HookParameters< Hook extends keyof AstroIntegration['hooks'], - Fn = AstroIntegration['hooks'][Hook], + Fn = AstroIntegration['hooks'][Hook] > = Fn extends (...args: any) => any ? Parameters[0] : never; export interface AstroIntegration { diff --git a/packages/astro/src/assets/internal.ts b/packages/astro/src/assets/internal.ts index 8bc3ec52aabc..ffc27333fa20 100644 --- a/packages/astro/src/assets/internal.ts +++ b/packages/astro/src/assets/internal.ts @@ -1,3 +1,4 @@ +import { isRemotePath } from '@astrojs/internal-helpers/path'; import type { AstroConfig, AstroSettings } from '../@types/astro.js'; import { AstroError, AstroErrorData } from '../core/errors/index.js'; import { isLocalService, type ImageService } from './services/service.js'; @@ -33,14 +34,10 @@ export function isRemoteAllowed( domains = [], remotePatterns = [], }: Partial> -): boolean | undefined { - let url: URL; - try { - url = new URL(src); - } catch (e) { - return undefined; - } +): boolean { + if (!isRemotePath(src)) return false; + const url = new URL(src); return ( domains.some((domain) => matchHostname(url, domain)) || remotePatterns.some((remotePattern) => matchPattern(url, remotePattern)) From eb6827e0835dfb965f51dfc854b464e37b41a3a1 Mon Sep 17 00:00:00 2001 From: Erika <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 16 Aug 2023 19:29:00 +0200 Subject: [PATCH 12/16] Update .changeset/sour-frogs-shout.md Co-authored-by: Sarah Rainsberger --- .changeset/sour-frogs-shout.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/sour-frogs-shout.md b/.changeset/sour-frogs-shout.md index 25bee6bf94fe..85abb3985f5b 100644 --- a/.changeset/sour-frogs-shout.md +++ b/.changeset/sour-frogs-shout.md @@ -2,7 +2,7 @@ 'astro': patch --- -Added support for optimizing remote images from authorized sources when using `astro:assets`. This comes with two new parameters to specify which domains (`image.domains`) and host patterns (`image.remotePatterns) are authorized for remote images. +Added support for optimizing remote images from authorized sources when using `astro:assets`. This comes with two new parameters to specify which domains (`image.domains`) and host patterns (`image.remotePatterns`) are authorized for remote images. For example, the following configuration will only allow remote images from `astro.build` to be optimized: From dc430a05227282d046c2686bde08e4c3daff0a6b Mon Sep 17 00:00:00 2001 From: Erika <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 16 Aug 2023 19:29:06 +0200 Subject: [PATCH 13/16] Update .changeset/sour-frogs-shout.md Co-authored-by: Sarah Rainsberger --- .changeset/sour-frogs-shout.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/sour-frogs-shout.md b/.changeset/sour-frogs-shout.md index 85abb3985f5b..9006914f1061 100644 --- a/.changeset/sour-frogs-shout.md +++ b/.changeset/sour-frogs-shout.md @@ -15,7 +15,7 @@ export default defineConfig({ }); ``` -The following configuration will only allo remote images from HTTPs hosts: +The following configuration will only allow remote images from HTTPS hosts: ```ts // astro.config.mjs From 804e272cf9d96b983a0b1d8ff26a00dc10214025 Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Thu, 17 Aug 2023 09:57:21 +0200 Subject: [PATCH 14/16] fix: remove // logic for now --- packages/astro/src/assets/build/remote.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/astro/src/assets/build/remote.ts b/packages/astro/src/assets/build/remote.ts index 0197af4e7829..c3d4bb9bae32 100644 --- a/packages/astro/src/assets/build/remote.ts +++ b/packages/astro/src/assets/build/remote.ts @@ -3,10 +3,6 @@ import CachePolicy from 'http-cache-semantics'; export type RemoteCacheEntry = { data: string; expires: number }; export async function loadRemoteImage(src: string) { - if (src.startsWith('//')) { - src = `https:${src}`; - } - const req = new Request(src); const res = await fetch(req); From 86e0918207b48ea96ff6013ef969d4b7ea134d89 Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Thu, 17 Aug 2023 10:00:57 +0200 Subject: [PATCH 15/16] test: add more tests --- packages/astro/test/core-image.test.js | 20 +++++++++++++++++++ .../core-image-ssg/src/pages/remote.astro | 7 +++++++ 2 files changed, 27 insertions(+) create mode 100644 packages/astro/test/fixtures/core-image-ssg/src/pages/remote.astro diff --git a/packages/astro/test/core-image.test.js b/packages/astro/test/core-image.test.js index 8c09de245211..263329cbf2d1 100644 --- a/packages/astro/test/core-image.test.js +++ b/packages/astro/test/core-image.test.js @@ -25,6 +25,7 @@ describe('astro:image', () => { }, image: { service: testImageService({ foo: 'bar' }), + domains: ['avatars.githubusercontent.com'], }, }); @@ -198,6 +199,15 @@ describe('astro:image', () => { $ = cheerio.load(html); }); + it('has proper link and works', async () => { + let $img = $('#remote img'); + + let src = $img.attr('src'); + expect(src.startsWith('/_image?')).to.be.true; + const imageRequest = await fixture.fetch(src); + expect(imageRequest.status).to.equal(200); + }); + it('includes the provided alt', async () => { let $img = $('#remote img'); expect($img.attr('alt')).to.equal('fred'); @@ -587,6 +597,7 @@ describe('astro:image', () => { }, image: { service: testImageService(), + domains: ['avatars.githubusercontent.com'], }, }); // Remove cache directory @@ -604,6 +615,15 @@ describe('astro:image', () => { expect(data).to.be.an.instanceOf(Buffer); }); + it('writes out allowed remote images', async () => { + const html = await fixture.readFile('/remote/index.html'); + const $ = cheerio.load(html); + const src = $('#remote img').attr('src'); + expect(src.length).to.be.greaterThan(0); + const data = await fixture.readFile(src, null); + expect(data).to.be.an.instanceOf(Buffer); + }); + it('writes out images to dist folder with proper extension if no format was passed', async () => { const html = await fixture.readFile('/index.html'); const $ = cheerio.load(html); diff --git a/packages/astro/test/fixtures/core-image-ssg/src/pages/remote.astro b/packages/astro/test/fixtures/core-image-ssg/src/pages/remote.astro new file mode 100644 index 000000000000..e002accc6f74 --- /dev/null +++ b/packages/astro/test/fixtures/core-image-ssg/src/pages/remote.astro @@ -0,0 +1,7 @@ +--- +import { Image } from "astro:assets"; +--- + +
+fred +
From 9665ed50f3aa82dd0e16c68e53ba97ff14fde58f Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Thu, 17 Aug 2023 12:26:08 +0200 Subject: [PATCH 16/16] test: adjust tests --- packages/astro/test/core-image.test.js | 17 ++++++++++------- .../core-image-ssg/src/pages/remote.astro | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/astro/test/core-image.test.js b/packages/astro/test/core-image.test.js index 263329cbf2d1..5d656a6f6a49 100644 --- a/packages/astro/test/core-image.test.js +++ b/packages/astro/test/core-image.test.js @@ -597,7 +597,7 @@ describe('astro:image', () => { }, image: { service: testImageService(), - domains: ['avatars.githubusercontent.com'], + domains: ['astro.build'], }, }); // Remove cache directory @@ -728,12 +728,15 @@ describe('astro:image', () => { }); it('has cache entries', async () => { - const generatedImages = (await fixture.glob('_astro/**/*.webp')).map((path) => - basename(path) - ); - const cachedImages = (await fixture.glob('../node_modules/.astro/assets/**/*.webp')).map( - (path) => basename(path) - ); + const generatedImages = (await fixture.glob('_astro/**/*.webp')) + .map((path) => basename(path)) + .sort(); + const cachedImages = [ + ...(await fixture.glob('../node_modules/.astro/assets/**/*.webp')), + ...(await fixture.glob('../node_modules/.astro/assets/**/*.json')), + ] + .map((path) => basename(path).replace('.webp.json', '.webp')) + .sort(); expect(generatedImages).to.deep.equal(cachedImages); }); diff --git a/packages/astro/test/fixtures/core-image-ssg/src/pages/remote.astro b/packages/astro/test/fixtures/core-image-ssg/src/pages/remote.astro index e002accc6f74..727a15ff0868 100644 --- a/packages/astro/test/fixtures/core-image-ssg/src/pages/remote.astro +++ b/packages/astro/test/fixtures/core-image-ssg/src/pages/remote.astro @@ -3,5 +3,5 @@ import { Image } from "astro:assets"; ---
-fred +fred