From e92d0531c7fff99b589f61fb45791c42bf96dca6 Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Wed, 25 Jun 2025 16:04:42 +0200 Subject: [PATCH 1/5] feat: add dedupe and noFetch flag --- .../src/PhishingController.ts | 41 ++++++++++++++++++- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index 2805666295e..6908a1da4d5 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -339,6 +339,11 @@ export class PhishingController extends BaseController< #isProgressC2DomainBlocklistUpdate?: Promise; + readonly #inProgressScans = new Map< + string, + Promise + >(); + /** * Construct a Phishing Controller. * @@ -650,9 +655,13 @@ export class PhishingController extends BaseController< * web URLs. * * @param url - The URL to scan. + * @param noFetch - Whether to only return the cached result or in-flight requests. * @returns The phishing detection scan result. */ - scanUrl = async (url: string): Promise => { + scanUrl = async ( + url: string, + noFetch?: boolean, + ): Promise => { const [hostname, ok] = getHostnameFromWebUrl(url); if (!ok) { return { @@ -667,6 +676,34 @@ export class PhishingController extends BaseController< return cachedResult; } + const inProgressScan = this.#inProgressScans.get(hostname); + if (inProgressScan) { + return inProgressScan; + } + + if (noFetch) { + // At this point we know there is no cached result and no in-progress scan so we can return a default result. + return { hostname: '', recommendedAction: RecommendedAction.None }; + } + + try { + const scanPromise = this.#performScan(hostname); + this.#inProgressScans.set(hostname, scanPromise); + return await scanPromise; + } finally { + // Clean up the in-progress cache when scan completes (success or failure) + this.#inProgressScans.delete(hostname); + } + }; + + /** + * Perform the actual phishing scan for a hostname. + * This method contains the HTTP request logic. + * + * @param hostname - The hostname to scan. + * @returns The phishing detection scan result. + */ + async #performScan(hostname: string): Promise { const apiResponse = await safelyExecuteWithTimeout( async () => { const res = await fetch( @@ -713,7 +750,7 @@ export class PhishingController extends BaseController< this.#urlScanCache.add(hostname, result); return result; - }; + } /** * Scan multiple URLs for phishing in bulk. It will only scan the hostnames of the URLs. From 8e8e32ea96a1c5778aac8297facdf4d55a449189 Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Wed, 25 Jun 2025 16:04:59 +0200 Subject: [PATCH 2/5] test: dedupe --- .../src/PhishingController.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index e96d44681d6..a64d1436085 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -2589,6 +2589,29 @@ describe('PhishingController', () => { expect(response).toMatchObject(mockResponse); expect(scope.isDone()).toBe(true); }); + + it('deduplicates concurrent requests for the same hostname', async () => { + const duplicateTestUrl = 'https://example.com'; + const expectedHostname = 'example.com'; + const scope = nock(PHISHING_DETECTION_BASE_URL) + .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) + .query({ url: expectedHostname }) + .reply(200, mockResponse); + + const [result1, result2, result3] = await Promise.all([ + controller.scanUrl(duplicateTestUrl), + controller.scanUrl(duplicateTestUrl), + controller.scanUrl(duplicateTestUrl), + ]); + + expect(result1).toMatchObject(mockResponse); + expect(result2).toMatchObject(mockResponse); + expect(result3).toMatchObject(mockResponse); + expect(scope.isDone()).toBe(true); + // eslint-disable-next-line import-x/no-named-as-default-member + expect(nock.pendingMocks()).toHaveLength(0); + }); + }); describe('bulkScanUrls', () => { From b5698679e0c1b69ca36b45321bd34661fbb2480e Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Wed, 25 Jun 2025 16:05:07 +0200 Subject: [PATCH 3/5] test: noFetch is true --- .../src/PhishingController.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index a64d1436085..6198b64aee8 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -2612,6 +2612,19 @@ describe('PhishingController', () => { expect(nock.pendingMocks()).toHaveLength(0); }); + it('does not make a request if noFetch is true', async () => { + const scope = nock(PHISHING_DETECTION_BASE_URL) + .get(`/${PHISHING_DETECTION_SCAN_ENDPOINT}`) + .query({ url: 'example.com' }) + .reply(200, mockResponse); + + const response = await controller.scanUrl(testUrl, true); + expect(response).toMatchObject({ + hostname: '', + recommendedAction: RecommendedAction.None, + }); + expect(scope.isDone()).toBe(false); + }); }); describe('bulkScanUrls', () => { From c6d1f7c4ba5f7691a3db03682cc6b5dc1348412f Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Wed, 25 Jun 2025 16:10:39 +0200 Subject: [PATCH 4/5] chore: update cache --- packages/phishing-controller/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/phishing-controller/CHANGELOG.md b/packages/phishing-controller/CHANGELOG.md index 4ef88adffd0..2c494c35e69 100644 --- a/packages/phishing-controller/CHANGELOG.md +++ b/packages/phishing-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added deduplication for outbound requests for the same url for `scanUrl`. ([#6032](https://github.com/MetaMask/core/pull/6032)) +- Added a `noFetch` flag to only return resolved scans or in-flight scans for `scanUrl`. ([6032](https://github.com/MetaMask/core/pull/6032)) + ### Changed - **BREAKING**`scanUrl` hits the v2 endpoint now. Returns `hostname` instead of `domainName` now. ([#5981](https://github.com/MetaMask/core/pull/5981)) From d1a4ff00587076be084f13e722813a6961a6a18f Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Wed, 25 Jun 2025 16:11:59 +0200 Subject: [PATCH 5/5] doc: improve readability of the documentation --- packages/phishing-controller/src/PhishingController.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index 6908a1da4d5..3977d11d3a0 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -655,7 +655,8 @@ export class PhishingController extends BaseController< * web URLs. * * @param url - The URL to scan. - * @param noFetch - Whether to only return the cached result or in-flight requests. + * @param noFetch - An optional flag to prevent making an outbound request. If true, the + * function will return the cached result or in-flight requests. * @returns The phishing detection scan result. */ scanUrl = async (