fix(resolver): verify resolvers throw errors and error is thrown on f…

…ailed transformations
Josh-McFarlin · Mar 1, 2022 · 3e9d100 · 3e9d100
1 parent e3bbdc0
commit 3e9d100
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 85 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {