From 08357d5ccad2f16e298e06d25bf827a18df3c5f8 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 15 Nov 2024 16:01:53 +0100 Subject: [PATCH 1/4] fix: Make ResponseWrapper an instance of Response --- .../coverage.json | 4 +- .../common/BaseSnapExecutor.test.browser.ts | 51 +++++++++++++++++++ .../endowments/endowments.test.browser.ts | 17 ++++++- .../src/common/endowments/network.test.ts | 4 +- .../src/common/endowments/network.ts | 3 +- 5 files changed, 73 insertions(+), 6 deletions(-) diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index bb7c7347df..a60ace0c3e 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { "branches": 80, "functions": 90.06, - "lines": 90.75, - "statements": 90.12 + "lines": 90.77, + "statements": 90.15 } diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts index f75bb23c8d..88208997cb 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts @@ -999,6 +999,57 @@ describe('BaseSnapExecutor', () => { }); }); + it('uses a response for fetch that works with instanceof', async () => { + const CODE = ` + module.exports.onRpcRequest = () => fetch('https://metamask.io').then(res => res instanceof Response); + `; + + const fetchSpy = spy(globalThis, 'fetch'); + + fetchSpy.mockImplementation(async () => { + return new Response('foo'); + }); + + const executor = new TestSnapExecutor(); + await executor.executeSnap(1, MOCK_SNAP_ID, CODE, ['fetch', 'Response']); + + expect(await executor.readCommand()).toStrictEqual({ + jsonrpc: '2.0', + id: 1, + result: 'OK', + }); + + await executor.writeCommand({ + jsonrpc: '2.0', + id: 2, + method: 'snapRpc', + params: [ + MOCK_SNAP_ID, + HandlerType.OnRpcRequest, + MOCK_ORIGIN, + { jsonrpc: '2.0', method: '', params: [] }, + ], + }); + + expect(await executor.readCommand()).toStrictEqual({ + jsonrpc: '2.0', + method: 'OutboundRequest', + params: { source: 'fetch' }, + }); + + expect(await executor.readCommand()).toStrictEqual({ + jsonrpc: '2.0', + method: 'OutboundResponse', + params: { source: 'fetch' }, + }); + + expect(await executor.readCommand()).toStrictEqual({ + id: 2, + jsonrpc: '2.0', + result: true, + }); + }); + it('notifies execution service of out of band errors via unhandledrejection', async () => { const CODE = ` module.exports.onRpcRequest = async () => 'foo'; diff --git a/packages/snaps-execution-environments/src/common/endowments/endowments.test.browser.ts b/packages/snaps-execution-environments/src/common/endowments/endowments.test.browser.ts index 888ce0070d..f88d0a1a46 100644 --- a/packages/snaps-execution-environments/src/common/endowments/endowments.test.browser.ts +++ b/packages/snaps-execution-environments/src/common/endowments/endowments.test.browser.ts @@ -12,7 +12,7 @@ import CryptoEndowment from './crypto'; import date from './date'; import interval from './interval'; import math from './math'; -import network from './network'; +import network, { ResponseWrapper } from './network'; import timeout from './timeout'; // @ts-expect-error - `globalThis.process` is not optional. @@ -218,6 +218,21 @@ describe('endowments', () => { endowments: { fetchAttenuated }, factory: () => undefined, }, + ResponseWrapper: { + endowments: { + Response: ResponseHardened, + ResponseWrapper: harden(ResponseWrapper), + }, + factory: () => + new ResponseWrapper( + new Response(), + { lastTeardown: 0 }, + // eslint-disable-next-line @typescript-eslint/no-empty-function + async () => {}, + // eslint-disable-next-line @typescript-eslint/no-empty-function + async () => {}, + ), + }, }; Object.entries(TEST_ENDOWMENTS).forEach( diff --git a/packages/snaps-execution-environments/src/common/endowments/network.test.ts b/packages/snaps-execution-environments/src/common/endowments/network.test.ts index a5763cc2ae..bf35f7eb44 100644 --- a/packages/snaps-execution-environments/src/common/endowments/network.test.ts +++ b/packages/snaps-execution-environments/src/common/endowments/network.test.ts @@ -1,6 +1,6 @@ import fetchMock from 'jest-fetch-mock'; -import network from './network'; +import network, { ResponseWrapper } from './network'; describe('Network endowments', () => { beforeAll(() => { @@ -187,7 +187,7 @@ describe('Network endowments', () => { const clonedResult = result.clone(); expect(clonedResult).toBeDefined(); expect(await clonedResult.text()).toBe(RESULT); - expect(clonedResult).not.toBeInstanceOf(Response); + expect(clonedResult).toBeInstanceOf(ResponseWrapper); }); it('should return when json is called', async () => { diff --git a/packages/snaps-execution-environments/src/common/endowments/network.ts b/packages/snaps-execution-environments/src/common/endowments/network.ts index 7dccdf47f5..158969b5cd 100644 --- a/packages/snaps-execution-environments/src/common/endowments/network.ts +++ b/packages/snaps-execution-environments/src/common/endowments/network.ts @@ -7,7 +7,7 @@ import type { EndowmentFactoryOptions } from './commonEndowmentFactory'; * This class wraps a Response object. * That way, a teardown process can stop any processes left. */ -class ResponseWrapper implements Response { +export class ResponseWrapper extends Response { readonly #teardownRef: { lastTeardown: number }; #ogResponse: Response; @@ -22,6 +22,7 @@ class ResponseWrapper implements Response { onStart: () => Promise, onFinish: () => Promise, ) { + super(); this.#ogResponse = ogResponse; this.#teardownRef = teardownRef; this.#onStart = onStart; From 44e44c78f48cde5bbaeb4b8b7b19bc433eeb2970 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 19 Nov 2024 10:54:41 +0100 Subject: [PATCH 2/4] Use different strategy --- packages/snaps-execution-environments/coverage.json | 4 ++-- .../src/common/endowments/network.test.ts | 3 ++- .../src/common/endowments/network.ts | 11 ++++++++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index a60ace0c3e..77218158d9 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { - "branches": 80, - "functions": 90.06, + "branches": 80.27, + "functions": 90.13, "lines": 90.77, "statements": 90.15 } diff --git a/packages/snaps-execution-environments/src/common/endowments/network.test.ts b/packages/snaps-execution-environments/src/common/endowments/network.test.ts index bf35f7eb44..2dca0b215a 100644 --- a/packages/snaps-execution-environments/src/common/endowments/network.test.ts +++ b/packages/snaps-execution-environments/src/common/endowments/network.test.ts @@ -180,13 +180,14 @@ describe('Network endowments', () => { const RESULT = 'OK'; fetchMock.mockOnce(async () => Promise.resolve(RESULT)); - const { fetch } = network.factory(factoryOptions); + const { fetch, Response } = network.factory(factoryOptions); const result = await fetch('foo.com'); expect(result.bodyUsed).toBe(false); const clonedResult = result.clone(); expect(clonedResult).toBeDefined(); expect(await clonedResult.text()).toBe(RESULT); + expect(clonedResult).toBeInstanceOf(Response); expect(clonedResult).toBeInstanceOf(ResponseWrapper); }); diff --git a/packages/snaps-execution-environments/src/common/endowments/network.ts b/packages/snaps-execution-environments/src/common/endowments/network.ts index 158969b5cd..3046165e5e 100644 --- a/packages/snaps-execution-environments/src/common/endowments/network.ts +++ b/packages/snaps-execution-environments/src/common/endowments/network.ts @@ -7,7 +7,7 @@ import type { EndowmentFactoryOptions } from './commonEndowmentFactory'; * This class wraps a Response object. * That way, a teardown process can stop any processes left. */ -export class ResponseWrapper extends Response { +export class ResponseWrapper { readonly #teardownRef: { lastTeardown: number }; #ogResponse: Response; @@ -22,7 +22,6 @@ export class ResponseWrapper extends Response { onStart: () => Promise, onFinish: () => Promise, ) { - super(); this.#ogResponse = ogResponse; this.#teardownRef = teardownRef; this.#onStart = onStart; @@ -146,6 +145,12 @@ export class ResponseWrapper extends Response { } } +class AlteredResponse extends Response { + static [Symbol.hasInstance](instance: any) { + return instance instanceof Response || instance instanceof ResponseWrapper; + } +} + /** * Create a network endowment, consisting of a `fetch` function. * This allows us to provide a teardown function, so that we can cancel @@ -298,7 +303,7 @@ const createNetwork = ({ notify }: EndowmentFactoryOptions = {}) => { // These endowments are not (and should never be) available by default. Request: harden(Request), Headers: harden(Headers), - Response: harden(Response), + Response: harden(AlteredResponse), teardownFunction, }; }; From c77adcd1aaddca2c8df98eee59451272c837e7bb Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 19 Nov 2024 11:35:03 +0100 Subject: [PATCH 3/4] Fix accidental change --- .../src/common/endowments/network.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snaps-execution-environments/src/common/endowments/network.ts b/packages/snaps-execution-environments/src/common/endowments/network.ts index 3046165e5e..fe8d8919c9 100644 --- a/packages/snaps-execution-environments/src/common/endowments/network.ts +++ b/packages/snaps-execution-environments/src/common/endowments/network.ts @@ -7,7 +7,7 @@ import type { EndowmentFactoryOptions } from './commonEndowmentFactory'; * This class wraps a Response object. * That way, a teardown process can stop any processes left. */ -export class ResponseWrapper { +export class ResponseWrapper implements Response { readonly #teardownRef: { lastTeardown: number }; #ogResponse: Response; From 89024f63c7be24e3460326e98630e4099d329fc1 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 19 Nov 2024 12:54:55 +0100 Subject: [PATCH 4/4] Address PR comments --- .../src/common/endowments/endowments.test.browser.ts | 6 ++---- .../src/common/endowments/network.ts | 5 ++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/snaps-execution-environments/src/common/endowments/endowments.test.browser.ts b/packages/snaps-execution-environments/src/common/endowments/endowments.test.browser.ts index f88d0a1a46..501f054545 100644 --- a/packages/snaps-execution-environments/src/common/endowments/endowments.test.browser.ts +++ b/packages/snaps-execution-environments/src/common/endowments/endowments.test.browser.ts @@ -227,10 +227,8 @@ describe('endowments', () => { new ResponseWrapper( new Response(), { lastTeardown: 0 }, - // eslint-disable-next-line @typescript-eslint/no-empty-function - async () => {}, - // eslint-disable-next-line @typescript-eslint/no-empty-function - async () => {}, + async () => undefined, + async () => undefined, ), }, }; diff --git a/packages/snaps-execution-environments/src/common/endowments/network.ts b/packages/snaps-execution-environments/src/common/endowments/network.ts index fe8d8919c9..c39fc51272 100644 --- a/packages/snaps-execution-environments/src/common/endowments/network.ts +++ b/packages/snaps-execution-environments/src/common/endowments/network.ts @@ -145,8 +145,11 @@ export class ResponseWrapper implements Response { } } +// We redefine the global Response class to overwrite [Symbol.hasInstance]. +// This fixes problems where the response from `fetch` would not pass +// instance of checks, leading to failures in WASM bindgen. class AlteredResponse extends Response { - static [Symbol.hasInstance](instance: any) { + static [Symbol.hasInstance](instance: unknown) { return instance instanceof Response || instance instanceof ResponseWrapper; } }