Fix snap.manifest.json fetchin from local: ids

Also added no-cache
MetaMask · Dec 5, 2022 · f61e195 · f61e195
1 parent 9d751dc
commit f61e195
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 4 deletions.
diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts
@@ -2067,6 +2067,8 @@ export class SnapController extends BaseController<
 
       return { manifest, files, location };
     } catch (error) {
+      // TODO(ritave): Export `getErrorMessage()` from @metamask/utils and use it here
+      //               https://github.com/MetaMask/utils/blob/62d022ef83c91fa4d150e51913be4441508a0ab1/src/assert.ts
       const message = error instanceof Error ? error.message : error.toString();
       throw new Error(`Failed to fetch Snap "${snapId}": ${message}.`);
     }

diff --git a/packages/snaps-controllers/src/snaps/location/http.test.ts b/packages/snaps-controllers/src/snaps/location/http.test.ts
@@ -43,4 +43,17 @@ describe('HttpLocation', () => {
     const file = await new HttpLocation(new URL(base)).fetch('./foo.js');
     expect(file.data.canonicalPath).toBe(canonical);
   });
+
+  it.each([
+    ['http://foo.bar/foo', 'http://foo.bar/snap.manifest.json'],
+    ['https://foo.bar/foo', 'https://foo.bar/snap.manifest.json'],
+    ['http://foo.bar/foo/', 'http://foo.bar/foo/snap.manifest.json'],
+  ])('fetches manifest from proper location', async (base, actuallyFetched) => {
+    fetchMock.mockResponses(JSON.stringify(getSnapManifest()));
+
+    await new HttpLocation(new URL(base)).manifest();
+
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(fetchMock).toHaveBeenNthCalledWith(1, actuallyFetched, undefined);
+  });
 });
diff --git a/packages/snaps-controllers/src/snaps/location/http.ts b/packages/snaps-controllers/src/snaps/location/http.ts
@@ -3,6 +3,7 @@ import {
   assertIsSnapManifest,
   VirtualFile,
   HttpSnapIdStruct,
+  NpmSnapFileNames,
 } from '@metamask/snaps-utils';
 import { assert, assertStruct } from '@metamask/utils';
 
@@ -14,6 +15,7 @@ export interface HttpOptions {
    * @default fetch
    */
   fetch?: typeof fetch;
+  fetchOptions?: RequestInit;
 }
 
 export class HttpLocation implements SnapLocation {
@@ -34,9 +36,12 @@ export class HttpLocation implements SnapLocation {
 
   private readonly fetchFn: typeof fetch;
 
+  private readonly fetchOptions?: RequestInit;
+
   constructor(url: URL, opts: HttpOptions = {}) {
     assertStruct(url.toString(), HttpSnapIdStruct, 'Invalid Snap Id: ');
     this.fetchFn = opts.fetch ?? globalThis.fetch;
+    this.fetchOptions = opts.fetchOptions;
     this.url = url;
   }
 
@@ -45,8 +50,13 @@ export class HttpLocation implements SnapLocation {
       return this.validatedManifest.clone();
     }
 
-    // jest-fetch-mock doesn't handle new URL(), we need to convert this.url.toString()
-    const contents = await (await this.fetchFn(this.url.toString())).text();
+    // jest-fetch-mock doesn't handle new URL(), we need to convert .toString()
+    const contents = await (
+      await this.fetchFn(
+        new URL(NpmSnapFileNames.Manifest, this.url).toString(),
+        this.fetchOptions,
+      )
+    ).text();
     const manifest = JSON.parse(contents);
     assertIsSnapManifest(manifest);
     const vfile = new VirtualFile<SnapManifest>({
@@ -72,7 +82,7 @@ export class HttpLocation implements SnapLocation {
     }
 
     const canonicalPath = this.toCanonical(relativePath).toString();
-    const response = await this.fetchFn(canonicalPath);
+    const response = await this.fetchFn(canonicalPath, this.fetchOptions);
     const vfile = new VirtualFile({
       value: '',
       path: relativePath,

diff --git a/packages/snaps-controllers/src/snaps/location/local.test.ts b/packages/snaps-controllers/src/snaps/location/local.test.ts
@@ -46,4 +46,37 @@ describe('LocalLocation', () => {
     const file = await new LocalLocation(new URL(base)).fetch('./foo.js');
     expect(file.data.canonicalPath).toBe(canonical);
   });
+
+  it.each([
+    ['local:http://localhost/foo', 'http://localhost/snap.manifest.json'],
+    ['local:https://localhost/foo', 'https://localhost/snap.manifest.json'],
+    ['local:http://localhost/foo/', 'http://localhost/foo/snap.manifest.json'],
+  ])('fetches manifest from proper location', async (base, actuallyFetched) => {
+    fetchMock.mockResponses(JSON.stringify(getSnapManifest()));
+
+    await new LocalLocation(new URL(base)).manifest();
+
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(fetchMock).toHaveBeenNthCalledWith(
+      1,
+      actuallyFetched,
+      expect.anything(),
+    );
+  });
+
+  it.each(['local:http://localhost', 'local:https://localhost'])(
+    'fetches with caching disabled',
+    async (url) => {
+      fetchMock.mockResponses(DEFAULT_SNAP_BUNDLE);
+
+      await new LocalLocation(new URL(url)).fetch('./foo.js');
+
+      expect(fetchMock).toHaveBeenCalledTimes(1);
+      expect(fetchMock).toHaveBeenNthCalledWith(
+        1,
+        expect.anything(),
+        expect.objectContaining({ cache: 'no-cache' }),
+      );
+    },
+  );
 });
diff --git a/packages/snaps-controllers/src/snaps/location/local.ts b/packages/snaps-controllers/src/snaps/location/local.ts
@@ -14,9 +14,15 @@ export class LocalLocation implements SnapLocation {
 
   constructor(url: URL, opts: HttpOptions = {}) {
     assertStruct(url.toString(), LocalSnapIdStruct, 'Invalid Snap Id');
+    // TODO(ritave): Write deepMerge() which merges fetchOptions.
+    assert(
+      opts.fetchOptions === undefined,
+      'Currently adding fetch options to local: is unsupported.',
+    );
+
     this.#http = new HttpLocation(
       new URL(url.toString().slice(SnapIdPrefixes.local.length)),
-      opts,
+      { ...opts, fetchOptions: { cache: 'no-cache' } },
     );
   }