mirror of
https://github.com/bitwarden/browser
synced 2026-02-13 06:54:07 +00:00
prevent blocking on non-web URLs and enable parallel tab checks
- Add early return for non-http(s) protocols (chrome://, about:, file://, etc.) to prevent expensive loadAllUrls() fallback on new tab pages - Replace concatMap with mergeMap(fn, 5) for parallel tab processing so one slow check doesn't block others - Add unit tests for protocol filtering
This commit is contained in:
@@ -110,6 +110,51 @@ describe("PhishingDataService", () => {
|
||||
});
|
||||
|
||||
describe("isPhishingWebAddress", () => {
|
||||
it("should return false for chrome:// URLs without checking IndexedDB", async () => {
|
||||
const url = new URL("chrome://newtab/");
|
||||
const result = await service.isPhishingWebAddress(url);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(mockIndexedDbService.hasUrl).not.toHaveBeenCalled();
|
||||
expect(mockIndexedDbService.loadAllUrls).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should return false for about: URLs without checking IndexedDB", async () => {
|
||||
const url = new URL("about:blank");
|
||||
const result = await service.isPhishingWebAddress(url);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(mockIndexedDbService.hasUrl).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should return false for file:// URLs without checking IndexedDB", async () => {
|
||||
const url = new URL("file:///Users/test/document.html");
|
||||
const result = await service.isPhishingWebAddress(url);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(mockIndexedDbService.hasUrl).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should check http:// URLs normally", async () => {
|
||||
mockIndexedDbService.hasUrl.mockResolvedValue(false);
|
||||
mockIndexedDbService.loadAllUrls.mockResolvedValue([]);
|
||||
|
||||
const url = new URL("http://example.com");
|
||||
await service.isPhishingWebAddress(url);
|
||||
|
||||
expect(mockIndexedDbService.hasUrl).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should check https:// URLs normally", async () => {
|
||||
mockIndexedDbService.hasUrl.mockResolvedValue(false);
|
||||
mockIndexedDbService.loadAllUrls.mockResolvedValue([]);
|
||||
|
||||
const url = new URL("https://example.com");
|
||||
await service.isPhishingWebAddress(url);
|
||||
|
||||
expect(mockIndexedDbService.hasUrl).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should detect a phishing web address using quick hasUrl lookup", async () => {
|
||||
// Mock hasUrl to return true for direct hostname match
|
||||
mockIndexedDbService.hasUrl.mockResolvedValue(true);
|
||||
|
||||
@@ -153,6 +153,12 @@ export class PhishingDataService {
|
||||
* @returns True if the URL is a known phishing web address, false otherwise
|
||||
*/
|
||||
async isPhishingWebAddress(url: URL): Promise<boolean> {
|
||||
// Skip non-http(s) protocols - phishing database only contains web URLs
|
||||
// This prevents expensive fallback checks for chrome://, about:, file://, etc.
|
||||
if (url.protocol !== "http:" && url.protocol !== "https:") {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Quick check for QA/dev test addresses
|
||||
if (this._testWebAddresses.includes(url.href)) {
|
||||
return true;
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
import {
|
||||
concatMap,
|
||||
distinctUntilChanged,
|
||||
EMPTY,
|
||||
filter,
|
||||
map,
|
||||
merge,
|
||||
mergeMap,
|
||||
Subject,
|
||||
switchMap,
|
||||
tap,
|
||||
@@ -63,7 +63,7 @@ export class PhishingDetectionService {
|
||||
tap((message) =>
|
||||
logService.debug(`[PhishingDetectionService] user selected continue for ${message.url}`),
|
||||
),
|
||||
concatMap(async (message) => {
|
||||
mergeMap(async (message) => {
|
||||
const url = new URL(message.url);
|
||||
this._ignoredHostnames.add(url.hostname);
|
||||
await BrowserApi.navigateTabToUrl(message.tabId, url);
|
||||
@@ -88,7 +88,9 @@ export class PhishingDetectionService {
|
||||
prev.ignored === curr.ignored,
|
||||
),
|
||||
tap((event) => logService.debug(`[PhishingDetectionService] processing event:`, event)),
|
||||
concatMap(async ({ tabId, url, ignored }) => {
|
||||
// Use mergeMap for parallel processing - each tab check runs independently
|
||||
// Concurrency limit of 5 prevents overwhelming IndexedDB
|
||||
mergeMap(async ({ tabId, url, ignored }) => {
|
||||
if (ignored) {
|
||||
// The next time this host is visited, block again
|
||||
this._ignoredHostnames.delete(url.hostname);
|
||||
@@ -104,7 +106,7 @@ export class PhishingDetectionService {
|
||||
`?phishingUrl=${url.toString()}`,
|
||||
);
|
||||
await BrowserApi.navigateTabToUrl(tabId, phishingWarningPage);
|
||||
}),
|
||||
}, 5),
|
||||
);
|
||||
|
||||
const onCancelCommand$ = messageListener
|
||||
|
||||
Reference in New Issue
Block a user