diff --git a/CHANGELOG.md b/CHANGELOG.md index f80e230..27eed7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fix + +- Verify resolvers throw errors on empty result and error is thrown on failed transformations + +### Changed + +- Simplify loader implementation for easier readability +- Move resolver error throwing from loader into resolver + ## [0.3.14] - 2022-02-23 ### Fix diff --git a/examples/cloudflare-pages/app/routes/api/image.ts b/examples/cloudflare-pages/app/routes/api/image.ts index 875b619..51494de 100644 --- a/examples/cloudflare-pages/app/routes/api/image.ts +++ b/examples/cloudflare-pages/app/routes/api/image.ts @@ -12,12 +12,11 @@ const cache = new MemoryCache({ }); export const loader: LoaderFunction = ({ request, context }) => { - const SELF_URL = context?.SELF_URL || context?.env?.SELF_URL; + const SELF_URL = context?.env?.SELF_URL || context?.SELF_URL; const resolver: Resolver = async (asset, url, options, basePath) => { if (asset.startsWith("/") && (asset.length === 1 || asset[1] !== "/")) { - const fetchUrl = new URL(url, SELF_URL).toString(); - const imageResponse = await context.ASSETS.fetch(fetchUrl); + const imageResponse = await context.ASSETS.fetch(url, request.clone()); const arrBuff = await imageResponse.arrayBuffer(); const buffer = new Uint8Array(arrBuff); diff --git a/src/server/loaders/imageLoader.ts b/src/server/loaders/imageLoader.ts index 6cecb75..e9f9aac 100644 --- a/src/server/loaders/imageLoader.ts +++ b/src/server/loaders/imageLoader.ts @@ -82,118 +82,119 @@ export const imageLoader: AssetLoader = async ( } const cacheKey = reqUrl.search; + let isNewImage = true; + let shouldTransform = true; + let loadedImg: Uint8Array | undefined; let resultImg: Uint8Array | undefined; + let inputContentType: MimeType | undefined; + let outputContentType: MimeType | undefined; if (cache && (await cache.has(cacheKey))) { const cacheValue = await cache.get(cacheKey); if (cacheValue) { console.log(`Retrieved image [${cacheKey}] from cache.`); - resultImg = cacheValue; + isNewImage = false; + shouldTransform = false; - if (!transformOptions.contentType) { - transformOptions.contentType = mimeFromBuffer(resultImg); - } + loadedImg = cacheValue; + inputContentType = mimeFromBuffer(loadedImg); } } - if (!resultImg) { - let res; - - try { - res = await resolver( - src, - assetUrl.toString(), - transformOptions, - basePath - ); - - if (!res || !res.buffer) { - throw new RemixImageError("Requested image not found!", 404); - } - if (transformOptions.contentType == null) { - transformOptions.contentType = res.contentType; - } + if (!loadedImg) { + const res = await resolver( + src, + assetUrl.toString(), + transformOptions, + basePath + ); - console.log( - `Fetched image [${cacheKey}] directly using resolver: ${resolver.name}.` - ); + console.log( + `Fetched image [${cacheKey}] directly using resolver: ${resolver.name}.` + ); + isNewImage = true; + shouldTransform = true; - resultImg = res.buffer; - } catch (error) { - throw new RemixImageError("Failed to retrieve requested image!", 500); - } + loadedImg = res.buffer; + inputContentType = res.contentType; + } - if (skipFormats?.has(res.contentType)) { - console.log(`Skipping transformation of mime type: ${res.contentType}`); - } else if (transformer != null) { - let curTransformer = transformer; - - if (!transformer.supportedInputs.has(res.contentType)) { - if ( - useFallbackTransformer && - transformer !== fallbackTransformer && - fallbackTransformer.supportedInputs.has(res.contentType) && - fallbackTransformer.supportedOutputs.has( - transformOptions.contentType - ) - ) { - console.error( - `Transformer does not allow this input content type: ${transformOptions.contentType}! Falling back to transformer: ${fallbackTransformer.name}` - ); - curTransformer = fallbackTransformer; - } else { - throw new UnsupportedImageError( - `Transformer does not allow this input content type: ${res.contentType}!` - ); - } - } + if (!loadedImg || !inputContentType) { + throw new RemixImageError("Failed to transform requested image!", 500); + } + if (!outputContentType) { + outputContentType = inputContentType; + } + + if (skipFormats?.has(inputContentType)) { + console.log(`Skipping transformation of mime type: ${inputContentType}`); + } else if (shouldTransform && transformer != null) { + let curTransformer = transformer; + + if (!transformer.supportedInputs.has(inputContentType)) { if ( - !curTransformer.supportedOutputs.has(transformOptions.contentType) + useFallbackTransformer && + transformer !== fallbackTransformer && + fallbackTransformer.supportedInputs.has(inputContentType) ) { - if ( - useFallbackFormat && - curTransformer.supportedOutputs.has(fallbackFormat) - ) { - console.error( - `Transformer does not allow this output content type: ${transformOptions.contentType}! Falling back to mime type: ${fallbackFormat}` - ); - transformOptions.contentType = fallbackFormat; - } else { - throw new UnsupportedImageError( - `Transformer does not allow this output content type: ${transformOptions.contentType}!` - ); - } + console.error( + `Transformer does not allow this input content type: ${inputContentType}! Falling back to transformer: ${fallbackTransformer.name}` + ); + curTransformer = fallbackTransformer; + } else { + throw new UnsupportedImageError( + `Transformer does not allow this input content type: ${inputContentType}!` + ); } + } - resultImg = await curTransformer.transform( - { - url: assetUrl.toString(), - data: res.buffer, - contentType: res.contentType, - }, - transformOptions - ); - - console.log( - `Successfully transformed image using transformer: ${curTransformer.name}` - ); + if (!curTransformer.supportedOutputs.has(outputContentType)) { + if ( + useFallbackFormat && + curTransformer.supportedOutputs.has(fallbackFormat) + ) { + console.error( + `Transformer does not allow this output content type: ${outputContentType}! Falling back to mime type: ${fallbackFormat}` + ); + outputContentType = fallbackFormat; + } else { + throw new UnsupportedImageError( + `Transformer does not allow this output content type: ${outputContentType}!` + ); + } } + + resultImg = await curTransformer.transform( + { + url: assetUrl.toString(), + data: loadedImg, + contentType: inputContentType!, + }, + { + ...transformOptions, + contentType: outputContentType!, + } + ); + + console.log( + `Successfully transformed image using transformer: ${curTransformer.name}` + ); } if (!resultImg) { throw new RemixImageError("Failed to transform requested image!", 500); } - if (cache) { + if (isNewImage && cache) { await cache.set(cacheKey, resultImg); } return imageResponse( resultImg, 200, - transformOptions.contentType!, + outputContentType, cache ? `private, max-age=${cache.config.ttl}, max-stale=${cache.config.tbd}` : `public, max-age=${60 * 60 * 24 * 365}` diff --git a/src/server/resolvers/fetchResolver.ts b/src/server/resolvers/fetchResolver.ts index 3f769a6..3455d17 100644 --- a/src/server/resolvers/fetchResolver.ts +++ b/src/server/resolvers/fetchResolver.ts @@ -1,3 +1,4 @@ +import { RemixImageError } from "../../types/error"; import { MimeType } from "../../types/file"; import type { Resolver } from "../../types/resolver"; @@ -9,8 +10,17 @@ export const fetchResolver: Resolver = async (_asset, url) => { }); const imageResponse = await fetch(imgRequest); + + if (!imageResponse.ok) { + throw new RemixImageError("Failed to fetch image!"); + } + const arrBuff = await imageResponse.arrayBuffer(); + if (!arrBuff || arrBuff.byteLength < 2) { + throw new RemixImageError("Invalid image retrieved from resolver!"); + } + const buffer = new Uint8Array(arrBuff); const contentType = imageResponse.headers.get("content-type")! as MimeType; diff --git a/src/server/resolvers/fsResolver.ts b/src/server/resolvers/fsResolver.ts index a52ed0c..acdf25a 100644 --- a/src/server/resolvers/fsResolver.ts +++ b/src/server/resolvers/fsResolver.ts @@ -2,13 +2,19 @@ import fs from "fs"; import path from "path"; import isSvg from "is-svg"; import mimeFromBuffer from "mime-tree"; -import { MimeType, UnsupportedImageError } from "../../types"; +import { RemixImageError, UnsupportedImageError } from "../../types/error"; +import { MimeType } from "../../types/file"; import type { Resolver } from "../../types/resolver"; export const fsResolver: Resolver = async (asset, _url, _options, basePath) => { const filePath = path.resolve(basePath, asset.slice(1)); const buffer = fs.readFileSync(filePath); + + if (!buffer || buffer.byteLength < 2) { + throw new RemixImageError("Invalid image retrieved from resolver!"); + } + let contentType: MimeType | null = null; try { contentType = mimeFromBuffer(buffer); diff --git a/src/server/resolvers/kvResolver.ts b/src/server/resolvers/kvResolver.ts index 8b8d493..95ee45f 100644 --- a/src/server/resolvers/kvResolver.ts +++ b/src/server/resolvers/kvResolver.ts @@ -2,7 +2,7 @@ import type { Options as KvAssetHandlerOptions } from "@cloudflare/kv-asset-hand import { getAssetFromKV, NotFoundError } from "@cloudflare/kv-asset-handler"; import isSvg from "is-svg"; import mimeFromBuffer from "mime-tree"; -import { MimeType, UnsupportedImageError } from "../../types"; +import { MimeType, RemixImageError, UnsupportedImageError } from "../../types"; import type { Resolver } from "../../types/resolver"; export interface FetchEvent { @@ -58,6 +58,10 @@ export const kvResolver: Resolver = async (_asset, url) => { const arrBuff = await imageResponse.arrayBuffer(); + if (!arrBuff || arrBuff.byteLength < 2) { + throw new RemixImageError("Invalid image retrieved from resolver!"); + } + const buffer = new Uint8Array(arrBuff); let contentType: MimeType | null = null; try {