From 431740879349de33f999f9e5e0ca03c5ab3eb714 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Wed, 21 Feb 2024 15:06:09 +0100 Subject: [PATCH 01/23] fix: remove network request deferred --- .../domains/network/NetworkRequest.ts | 197 +++++------------- .../domains/network/NetworkStorage.spec.ts | 64 +++--- .../domains/network/NetworkStorage.ts | 16 +- tests/browsing_context/test_reload.py | 4 - 4 files changed, 106 insertions(+), 175 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index ccb3968ba9..089e6f273b 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -26,10 +26,9 @@ import { Network, type BrowsingContext, ChromiumBidi, + type NetworkEvent, } from '../../../protocol/protocol.js'; import {assert} from '../../../utils/assert.js'; -import {Deferred} from '../../../utils/Deferred.js'; -import type {Result} from '../../../utils/result.js'; import type {CdpTarget} from '../context/CdpTarget.js'; import type {EventManager} from '../session/EventManager.js'; @@ -78,14 +77,18 @@ export class NetworkRequest { paused?: Protocol.Fetch.RequestPausedEvent; } = {}; - #beforeRequestSentDeferred = new Deferred>(); - #responseStartedDeferred = new Deferred>(); - #responseCompletedDeferred = new Deferred>(); - #eventManager: EventManager; #networkStorage: NetworkStorage; #cdpTarget: CdpTarget; + #emittedEvents: Record = { + [ChromiumBidi.Network.EventNames.AuthRequired]: false, + [ChromiumBidi.Network.EventNames.BeforeRequestSent]: false, + [ChromiumBidi.Network.EventNames.FetchError]: false, + [ChromiumBidi.Network.EventNames.ResponseCompleted]: false, + [ChromiumBidi.Network.EventNames.ResponseStarted]: false, + }; + constructor( id: Network.Request, eventManager: EventManager, @@ -116,8 +119,8 @@ export class NetworkRequest { return ( this.#response.info?.url ?? this.#request.info?.request.url ?? - this.#request.paused?.request.url ?? - this.#response.paused?.request.url + this.#response.paused?.request.url ?? + this.#request.paused?.request.url ); } @@ -142,8 +145,6 @@ export class NetworkRequest { } handleRedirect(event: Protocol.Network.RequestWillBeSentEvent): void { - this.#queueResponseStartedEvent(); - this.#queueResponseCompletedEvent(); this.#response.hasExtraInfo = event.redirectHasExtraInfo; this.#response.info = event.redirectResponse!; this.#emitEventsIfReady(true); @@ -174,9 +175,8 @@ export class NetworkRequest { ? requestInterceptionCompleted : requestExtraInfoCompleted) ) { - this.#beforeRequestSentDeferred.resolve({ - kind: 'success', - value: undefined, + this.#emitEvent(() => { + return this.#getBeforeRequestEvent(); }); } @@ -195,15 +195,12 @@ export class NetworkRequest { (responseInterceptionExpected && Boolean(this.#response.paused)) || !responseInterceptionExpected; - if (responseInterceptionExpected && Boolean(this.#response.paused)) { - this.#responseStartedDeferred.resolve({ - kind: 'success', - value: undefined, - }); - } else if (this.#response.info) { - this.#responseStartedDeferred.resolve({ - kind: 'success', - value: undefined, + if ( + (responseInterceptionExpected && Boolean(this.#response.paused)) || + this.#response.info + ) { + this.#emitEvent(() => { + return this.#getResponseStartedEvent(); }); } @@ -212,16 +209,14 @@ export class NetworkRequest { responseExtraInfoCompleted && responseInterceptionCompleted ) { - this.#responseCompletedDeferred.resolve({ - kind: 'success', - value: undefined, + this.#emitEvent(() => { + return this.#getResponseReceivedEvent(); }); } } onRequestWillBeSentEvent(event: Protocol.Network.RequestWillBeSentEvent) { this.#request.info = event; - this.#queueBeforeRequestSentEvent(); this.#emitEventsIfReady(); } @@ -242,8 +237,6 @@ export class NetworkRequest { onResponseReceivedEvent(event: Protocol.Network.ResponseReceivedEvent) { this.#response.hasExtraInfo = event.hasExtraInfo; this.#response.info = event.response; - this.#queueResponseStartedEvent(); - this.#queueResponseCompletedEvent(); this.#emitEventsIfReady(); } @@ -253,33 +246,15 @@ export class NetworkRequest { } onLoadingFailedEvent(event: Protocol.Network.LoadingFailedEvent) { - this.#beforeRequestSentDeferred.resolve({ - kind: 'success', - value: undefined, - }); - this.#responseStartedDeferred.resolve({ - kind: 'error', - error: new Error('Network event loading failed'), - }); - this.#responseCompletedDeferred.resolve({ - kind: 'error', - error: new Error('Network event loading failed'), + this.#emitEvent(() => { + return { + method: ChromiumBidi.Network.EventNames.FetchError, + params: { + ...this.#getBaseEventParams(), + errorText: event.errorText, + }, + }; }); - - this.#queueEventPromise( - Promise.resolve({kind: 'success', value: undefined}), - () => { - return { - params: { - ...this.#getBaseEventParams(), - errorText: event.errorText, - }, - method: ChromiumBidi.Network.EventNames.FetchError, - type: 'event' as const, - }; - }, - ChromiumBidi.Network.EventNames.FetchError - ); } /** @see https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#method-failRequest */ @@ -306,8 +281,7 @@ export class NetworkRequest { if (!this.blocked) { void this.continueResponse(); } else if (!this.#response.info) { - this.#queueResponseStartedEvent(); - this.#queueResponseCompletedEvent(); + this.#emitEventsIfReady(); } } else { this.#request.paused = event; @@ -325,22 +299,17 @@ export class NetworkRequest { void this.continueWithAuth(); } - this.#queueEventPromise( - Promise.resolve({kind: 'success', value: undefined}), - () => { - return { - params: { - ...this.#getBaseEventParams(Network.InterceptPhase.AuthRequired), - // TODO: Why is this on the Spec - // How are we suppose to know the response if we are blocked by Auth - response: {} as any, - }, - method: ChromiumBidi.Network.EventNames.AuthRequired, - type: 'event' as const, - }; - }, - ChromiumBidi.Network.EventNames.AuthRequired - ); + this.#emitEvent((): Network.AuthRequired => { + return { + method: ChromiumBidi.Network.EventNames.AuthRequired, + params: { + ...this.#getBaseEventParams(Network.InterceptPhase.AuthRequired), + // TODO: Why is this on the Spec + // How are we suppose to know the response if we are blocked by Auth + response: {} as any, + }, + }; + }); } /** @see https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#method-continueRequest */ @@ -420,13 +389,7 @@ export class NetworkRequest { } dispose() { - const result = { - kind: 'error' as const, - error: new Error('Network processor detached'), - }; - this.#beforeRequestSentDeferred.resolve(result); - this.#responseStartedDeferred.resolve(result); - this.#responseCompletedDeferred.resolve(result); + // TODO: ??? } get #context() { @@ -436,38 +399,25 @@ export class NetworkRequest { /** Returns the HTTP status code associated with this request if any. */ get statusCode(): number { return ( - this.#response.info?.status ?? this.#response.extraInfo?.statusCode ?? -1 // TODO: Throw an exception or use some other status code? + this.#response.paused?.responseStatusCode ?? + this.#response.extraInfo?.statusCode ?? + this.#response.info?.status ?? + -1 // TODO: Throw an exception or use some other status code? ); } - #queueEventPromise( - deferred: Promise>, - getEventData: () => ChromiumBidi.Event, - eventName: ChromiumBidi.Network.EventNames - ) { - if (this.#isIgnoredEvent()) { + #emitEvent(getEventData: () => NetworkEvent) { + const event = getEventData(); + if (this.#isIgnoredEvent() || this.#emittedEvents[event.method]) { return; } - this.#eventManager.registerPromiseEvent( - deferred.then((result) => { - if (result.kind === 'success') { - try { - return { - kind: 'success', - value: getEventData(), - }; - } catch (error) { - return { - kind: 'error', - error: error instanceof Error ? error : new Error('Unknown'), - }; - } - } - return result; + this.#emittedEvents[event.method] = true; + this.#eventManager.registerEvent( + Object.assign(event, { + type: 'event' as const, }), - this.#context, - eventName + this.#context ); } @@ -556,18 +506,6 @@ export class NetworkRequest { }; } - #queueBeforeRequestSentEvent() { - this.#queueEventPromise( - this.#beforeRequestSentDeferred, - () => { - return Object.assign(this.#getBeforeRequestEvent(), { - type: 'event' as const, - }); - }, - ChromiumBidi.Network.EventNames.BeforeRequestSent - ); - } - #getBeforeRequestEvent(): Network.BeforeRequestSent { assert(this.#request.info, 'RequestWillBeSentEvent is not set'); @@ -584,18 +522,6 @@ export class NetworkRequest { }; } - #queueResponseStartedEvent() { - this.#queueEventPromise( - this.#responseStartedDeferred, - () => { - return Object.assign(this.#getResponseStartedEvent(), { - type: 'event' as const, - }); - }, - ChromiumBidi.Network.EventNames.ResponseStarted - ); - } - #getResponseStartedEvent(): Network.ResponseStarted { assert(this.#request.info, 'RequestWillBeSentEvent is not set'); assert( @@ -626,8 +552,7 @@ export class NetworkRequest { this.#response.paused?.request.url ?? NetworkRequest.#unknown, protocol: this.#response.info?.protocol ?? '', - status: - this.statusCode || this.#response.paused?.responseStatusCode || 0, + status: this.statusCode, statusText: this.#response.info?.statusText || this.#response.paused?.responseStatusText || @@ -651,18 +576,6 @@ export class NetworkRequest { }; } - #queueResponseCompletedEvent() { - this.#queueEventPromise( - this.#responseCompletedDeferred, - () => { - return Object.assign(this.#getResponseReceivedEvent(), { - type: 'event' as const, - }); - }, - ChromiumBidi.Network.EventNames.ResponseCompleted - ); - } - #getResponseReceivedEvent(): Network.ResponseCompleted { assert(this.#request.info, 'RequestWillBeSentEvent is not set'); assert(this.#response.info, 'ResponseReceivedEvent is not set'); diff --git a/src/bidiMapper/domains/network/NetworkStorage.spec.ts b/src/bidiMapper/domains/network/NetworkStorage.spec.ts index e3f6848775..0d9e15627e 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.spec.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.spec.ts @@ -30,11 +30,13 @@ import {EventManager, EventManagerEvents} from '../session/EventManager.js'; import {NetworkStorage} from './NetworkStorage.js'; -// type Ka = keyof Protocol.Network.RequestWillBeSentEvent; -// type Kb = keyof Protocol.Network.RequestWillBeSentExtraInfoEvent; -// type Kc = keyof Protocol.Network.ResponseReceivedExtraInfoEvent; -// type Kd = keyof Protocol.Network.ResponseReceivedEvent; -// type Kz = Exclude<(Ka & Kb) | (Ka & Kd) | (Kb & Kc), 'headers' | 'timestamp'>; +function logger(...args: any[]) { + // eslint-disable-next-line no-constant-condition + if (false) { + // eslint-disable-next-line no-console + console.log(...args); + } +} // TODO: Extend with Redirect class MockCdpNetworkEvents { @@ -43,10 +45,10 @@ class MockCdpNetworkEvents { cdpClient: CdpClient; + url: string; requestId: string; fetchId: string; private loaderId: string; - private url: string; private frameId: string; private type: Protocol.Network.ResourceType; @@ -272,8 +274,17 @@ class MockCdpNetworkEvents { } class MockCdpClient extends EventEmitter { + #logger: (...args: any[]) => void; + sessionId = '23E8E97ED43449740710991CD32AEFA3'; - sendCommand() { + + constructor(logger: (...args: any[]) => void) { + super(); + this.#logger = logger; + } + + sendCommand(...args: any[]) { + this.#logger(...args); return Promise.resolve(); } @@ -283,8 +294,16 @@ class MockCdpClient extends EventEmitter { } class MockCdpTarget { - cdpClient = new MockCdpClient() as CdpClient; + cdpClient: CdpClient; + #logger: (...args: any[]) => void; + + constructor(logger: (...args: any[]) => void) { + this.#logger = logger; + this.cdpClient = new MockCdpClient(logger) as CdpClient; + } + enableFetchIfNeeded() { + this.#logger('Fetch.enabled called'); return Promise.resolve(); } } @@ -294,8 +313,10 @@ describe('NetworkStorage', () => { ChromiumBidi.Event['method'], ChromiumBidi.Event['params'] >(); + let eventManager!: EventManager; let networkStorage!: NetworkStorage; let cdpClient!: CdpClient; + let processingQueue!: ProcessingQueue; // TODO: Better way of getting Events async function getEvent(name: ChromiumBidi.Event['method']) { @@ -306,7 +327,7 @@ describe('NetworkStorage', () => { beforeEach(() => { processedEvents = new Map(); const browsingContextStorage = new BrowsingContextStorage(); - const cdpTarget = new MockCdpTarget() as unknown as CdpTarget; + const cdpTarget = new MockCdpTarget(logger) as unknown as CdpTarget; const browsingContext = { cdpTarget, id: MockCdpNetworkEvents.defaultFrameId, @@ -314,13 +335,14 @@ describe('NetworkStorage', () => { cdpClient = cdpTarget.cdpClient; // We need to add it the storage to emit properly browsingContextStorage.addContext(browsingContext); - const eventManager = new EventManager(browsingContextStorage); - const processingQueue = new ProcessingQueue( + eventManager = new EventManager(browsingContextStorage); + processingQueue = new ProcessingQueue( async ({message}) => { const cdpEvent = message as unknown as ChromiumBidi.Event; processedEvents.set(cdpEvent.method, cdpEvent.params); return await Promise.resolve(); - } + }, + logger ); // Subscribe to the `network` module globally eventManager.subscriptionManager.subscribe( @@ -366,13 +388,11 @@ describe('NetworkStorage', () => { }); it('should work interception', async () => { + const request = new MockCdpNetworkEvents(cdpClient); const interception = await networkStorage.addIntercept({ - urlPatterns: [ - {type: 'string', pattern: MockCdpNetworkEvents.defaultUrl}, - ], + urlPatterns: [{type: 'string', pattern: request.url}], phases: [Network.InterceptPhase.BeforeRequestSent], }); - const request = new MockCdpNetworkEvents(cdpClient); request.requestWillBeSent(); let event = await getEvent('network.beforeRequestSent'); @@ -387,13 +407,11 @@ describe('NetworkStorage', () => { }); it('should work interception pause first', async () => { + const request = new MockCdpNetworkEvents(cdpClient); const interception = await networkStorage.addIntercept({ - urlPatterns: [ - {type: 'string', pattern: MockCdpNetworkEvents.defaultUrl}, - ], + urlPatterns: [{type: 'string', pattern: request.url}], phases: [Network.InterceptPhase.BeforeRequestSent], }); - const request = new MockCdpNetworkEvents(cdpClient); request.requestPaused(); let event = await getEvent('network.beforeRequestSent'); @@ -461,13 +479,11 @@ describe('NetworkStorage', () => { expect(event).to.exist; }); it('should work interception', async () => { + const request = new MockCdpNetworkEvents(cdpClient); const interception = await networkStorage.addIntercept({ - urlPatterns: [ - {type: 'string', pattern: MockCdpNetworkEvents.defaultUrl}, - ], + urlPatterns: [{type: 'string', pattern: request.url}], phases: [Network.InterceptPhase.ResponseStarted], }); - const request = new MockCdpNetworkEvents(cdpClient); request.requestWillBeSent(); request.requestWillBeSentExtraInfo(); diff --git a/src/bidiMapper/domains/network/NetworkStorage.ts b/src/bidiMapper/domains/network/NetworkStorage.ts index cc6dfd95b3..60ad5f9915 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.ts @@ -296,10 +296,16 @@ export class NetworkStorage { cdpTarget: CdpTarget ) { // CDP quirk if the Network domain is not present this is undefined - this.#getOrCreateNetworkRequest( - event.networkId ?? '', - cdpTarget - ).onRequestPaused(event); + let request = this.getRequestByFetchId(event.requestId); + + if (!request) { + request = this.#getOrCreateNetworkRequest( + event.networkId ?? event.requestId, + cdpTarget + ); + } + + request.onRequestPaused(event); } #handleAuthInterception( @@ -307,7 +313,7 @@ export class NetworkStorage { cdpTarget: CdpTarget ) { // CDP quirk if the Network domain is not present this is undefined - const request = this.getRequestByFetchId(event.requestId ?? ''); + const request = this.getRequestByFetchId(event.requestId); if (!request) { // CDP quirk even both request/response may be continued // with this command diff --git a/tests/browsing_context/test_reload.py b/tests/browsing_context/test_reload.py index e0d5bcf99d..fba49c6fa3 100644 --- a/tests/browsing_context/test_reload.py +++ b/tests/browsing_context/test_reload.py @@ -195,10 +195,6 @@ async def test_browsingContext_reload_waitComplete(websocket, context_id, @pytest.mark.parametrize("ignoreCache", [True, False]) async def test_browsingContext_reload_ignoreCache(websocket, context_id, ignoreCache, cacheable_url): - if not ignoreCache: - pytest.xfail( - "TODO: https://github.com/GoogleChromeLabs/chromium-bidi/pull/1466/files#r1377517937 need to be fixed" - ) await subscribe(websocket, [ "network.beforeRequestSent", From 9425542d931b93e49cc56effe76c10d9a34c2b01 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Thu, 22 Feb 2024 13:42:05 +0100 Subject: [PATCH 02/23] chore: fix reload test --- .../domains/network/NetworkRequest.ts | 51 ++++++++----------- tests/browsing_context/test_reload.py | 19 +------ 2 files changed, 21 insertions(+), 49 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index 089e6f273b..24507bbdc5 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -175,9 +175,7 @@ export class NetworkRequest { ? requestInterceptionCompleted : requestExtraInfoCompleted) ) { - this.#emitEvent(() => { - return this.#getBeforeRequestEvent(); - }); + this.#emitEvent(this.#getBeforeRequestEvent()); } const responseExtraInfoCompleted = @@ -196,12 +194,10 @@ export class NetworkRequest { !responseInterceptionExpected; if ( - (responseInterceptionExpected && Boolean(this.#response.paused)) || - this.#response.info + this.#response.info || + (responseInterceptionExpected && Boolean(this.#response.paused)) ) { - this.#emitEvent(() => { - return this.#getResponseStartedEvent(); - }); + this.#emitEvent(this.#getResponseStartedEvent()); } if ( @@ -209,9 +205,7 @@ export class NetworkRequest { responseExtraInfoCompleted && responseInterceptionCompleted ) { - this.#emitEvent(() => { - return this.#getResponseReceivedEvent(); - }); + this.#emitEvent(this.#getResponseReceivedEvent()); } } @@ -246,14 +240,12 @@ export class NetworkRequest { } onLoadingFailedEvent(event: Protocol.Network.LoadingFailedEvent) { - this.#emitEvent(() => { - return { - method: ChromiumBidi.Network.EventNames.FetchError, - params: { - ...this.#getBaseEventParams(), - errorText: event.errorText, - }, - }; + this.#emitEvent({ + method: ChromiumBidi.Network.EventNames.FetchError, + params: { + ...this.#getBaseEventParams(), + errorText: event.errorText, + }, }); } @@ -299,16 +291,14 @@ export class NetworkRequest { void this.continueWithAuth(); } - this.#emitEvent((): Network.AuthRequired => { - return { - method: ChromiumBidi.Network.EventNames.AuthRequired, - params: { - ...this.#getBaseEventParams(Network.InterceptPhase.AuthRequired), - // TODO: Why is this on the Spec - // How are we suppose to know the response if we are blocked by Auth - response: {} as any, - }, - }; + this.#emitEvent({ + method: ChromiumBidi.Network.EventNames.AuthRequired, + params: { + ...this.#getBaseEventParams(Network.InterceptPhase.AuthRequired), + // TODO: Why is this on the Spec + // How are we suppose to know the response if we are blocked by Auth + response: {} as any, + }, }); } @@ -406,8 +396,7 @@ export class NetworkRequest { ); } - #emitEvent(getEventData: () => NetworkEvent) { - const event = getEventData(); + #emitEvent(event: NetworkEvent) { if (this.#isIgnoredEvent() || this.#emittedEvents[event.method]) { return; } diff --git a/tests/browsing_context/test_reload.py b/tests/browsing_context/test_reload.py index fba49c6fa3..5875297c50 100644 --- a/tests/browsing_context/test_reload.py +++ b/tests/browsing_context/test_reload.py @@ -197,7 +197,6 @@ async def test_browsingContext_reload_ignoreCache(websocket, context_id, ignoreCache, cacheable_url): await subscribe(websocket, [ - "network.beforeRequestSent", "network.responseCompleted", ]) @@ -213,21 +212,6 @@ async def test_browsingContext_reload_ignoreCache(websocket, context_id, } }) - before_request_sent_event = await read_JSON_message(websocket) - assert before_request_sent_event == { - 'type': 'event', - "method": "network.beforeRequestSent", - "params": { - "isBlocked": False, - "context": context_id, - "initiator": ANY_DICT, - "navigation": ANY_STR, - "redirectCount": 0, - "request": ANY_DICT, - "timestamp": ANY_TIMESTAMP, - }, - } - response_completed_event = await read_JSON_message(websocket) assert response_completed_event == { 'type': 'event', @@ -255,6 +239,5 @@ async def test_browsingContext_reload_ignoreCache(websocket, context_id, } assert response["result"]["navigation"] == response_completed_event[ - "params"]["navigation"] == before_request_sent_event["params"][ - "navigation"] + "params"]["navigation"] assert initial_navigation["navigation"] != response["result"]["navigation"] From b772030d66387bcc68067a0aeb4370b2723552f2 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Thu, 22 Feb 2024 19:52:13 +0100 Subject: [PATCH 03/23] chore: add logging --- src/bidiMapper/CommandProcessor.ts | 6 ++- .../domains/network/NetworkRequest.ts | 54 ++++++++++++------- .../domains/network/NetworkStorage.ts | 13 ++++- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/src/bidiMapper/CommandProcessor.ts b/src/bidiMapper/CommandProcessor.ts index 51c7b2e66c..6a2500aec8 100644 --- a/src/bidiMapper/CommandProcessor.ts +++ b/src/bidiMapper/CommandProcessor.ts @@ -91,7 +91,11 @@ export class CommandProcessor extends EventEmitter { this.#parser = parser; this.#logger = logger; - const networkStorage = new NetworkStorage(eventManager, browserCdpClient); + const networkStorage = new NetworkStorage( + eventManager, + browserCdpClient, + logger + ); const preloadScriptStorage = new PreloadScriptStorage(); // keep-sorted start block=yes diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index 24507bbdc5..12204eda05 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -29,6 +29,7 @@ import { type NetworkEvent, } from '../../../protocol/protocol.js'; import {assert} from '../../../utils/assert.js'; +import {LogType, type LoggerFn} from '../../../utils/log.js'; import type {CdpTarget} from '../context/CdpTarget.js'; import type {EventManager} from '../session/EventManager.js'; @@ -80,6 +81,7 @@ export class NetworkRequest { #eventManager: EventManager; #networkStorage: NetworkStorage; #cdpTarget: CdpTarget; + #logger?: LoggerFn; #emittedEvents: Record = { [ChromiumBidi.Network.EventNames.AuthRequired]: false, @@ -94,13 +96,15 @@ export class NetworkRequest { eventManager: EventManager, networkStorage: NetworkStorage, cdpTarget: CdpTarget, - redirectCount = 0 + redirectCount = 0, + logger?: LoggerFn ) { this.#id = id; this.#eventManager = eventManager; this.#networkStorage = networkStorage; this.#cdpTarget = cdpTarget; this.#redirectCount = redirectCount; + this.#logger = logger; } get id(): string { @@ -175,7 +179,7 @@ export class NetworkRequest { ? requestInterceptionCompleted : requestExtraInfoCompleted) ) { - this.#emitEvent(this.#getBeforeRequestEvent()); + this.#emitEvent(this.#getBeforeRequestEvent.bind(this)); } const responseExtraInfoCompleted = @@ -197,7 +201,7 @@ export class NetworkRequest { this.#response.info || (responseInterceptionExpected && Boolean(this.#response.paused)) ) { - this.#emitEvent(this.#getResponseStartedEvent()); + this.#emitEvent(this.#getResponseStartedEvent.bind(this)); } if ( @@ -205,7 +209,7 @@ export class NetworkRequest { responseExtraInfoCompleted && responseInterceptionCompleted ) { - this.#emitEvent(this.#getResponseReceivedEvent()); + this.#emitEvent(this.#getResponseReceivedEvent.bind(this)); } } @@ -240,12 +244,14 @@ export class NetworkRequest { } onLoadingFailedEvent(event: Protocol.Network.LoadingFailedEvent) { - this.#emitEvent({ - method: ChromiumBidi.Network.EventNames.FetchError, - params: { - ...this.#getBaseEventParams(), - errorText: event.errorText, - }, + this.#emitEvent(() => { + return { + method: ChromiumBidi.Network.EventNames.FetchError, + params: { + ...this.#getBaseEventParams(), + errorText: event.errorText, + }, + }; }); } @@ -291,14 +297,16 @@ export class NetworkRequest { void this.continueWithAuth(); } - this.#emitEvent({ - method: ChromiumBidi.Network.EventNames.AuthRequired, - params: { - ...this.#getBaseEventParams(Network.InterceptPhase.AuthRequired), - // TODO: Why is this on the Spec - // How are we suppose to know the response if we are blocked by Auth - response: {} as any, - }, + this.#emitEvent(() => { + return { + method: ChromiumBidi.Network.EventNames.AuthRequired, + params: { + ...this.#getBaseEventParams(Network.InterceptPhase.AuthRequired), + // TODO: Why is this on the Spec + // How are we suppose to know the response if we are blocked by Auth + response: {} as any, + }, + }; }); } @@ -396,7 +404,15 @@ export class NetworkRequest { ); } - #emitEvent(event: NetworkEvent) { + #emitEvent(getEvent: () => NetworkEvent) { + let event: NetworkEvent; + try { + event = getEvent(); + } catch (error) { + this.#logger?.(LogType.debugError, error); + throw error; + } + if (this.#isIgnoredEvent() || this.#emittedEvents[event.method]) { return; } diff --git a/src/bidiMapper/domains/network/NetworkStorage.ts b/src/bidiMapper/domains/network/NetworkStorage.ts index 60ad5f9915..a36aa89989 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.ts @@ -17,6 +17,7 @@ import type {Protocol} from 'devtools-protocol'; import {Network, NoSuchInterceptException} from '../../../protocol/protocol.js'; +import type {LoggerFn} from '../../../utils/log.js'; import {uuidv4} from '../../../utils/uuid.js'; import type {CdpClient} from '../../BidiMapper.js'; import type {CdpTarget} from '../context/CdpTarget.js'; @@ -33,6 +34,7 @@ interface NetworkInterception { /** Stores network and intercept maps. */ export class NetworkStorage { #eventManager: EventManager; + #logger?: LoggerFn; readonly #targets = new Set(); /** @@ -50,7 +52,11 @@ export class NetworkStorage { auth: false, }; - constructor(eventManager: EventManager, browserClient: CdpClient) { + constructor( + eventManager: EventManager, + browserClient: CdpClient, + logger?: LoggerFn + ) { this.#eventManager = eventManager; browserClient.on( @@ -59,6 +65,8 @@ export class NetworkStorage { this.disposeRequestMap(sessionId); } ); + + this.#logger = logger; } /** @@ -80,7 +88,8 @@ export class NetworkStorage { this.#eventManager, this, cdpTarget, - redirectCount + redirectCount, + this.#logger ); this.addRequest(request); From 41f96d191c6c011313292ee4c437d807436e1f6c Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 11:17:46 +0100 Subject: [PATCH 04/23] chore: fix auth event --- src/bidiMapper/domains/network/NetworkRequest.ts | 11 ++++++----- .../domains/network/NetworkStorage.spec.ts | 13 +++++++++++-- src/bidiMapper/domains/network/NetworkStorage.ts | 16 ++-------------- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index 12204eda05..468a89e680 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -170,8 +170,8 @@ export class NetworkRequest { ); const requestInterceptionCompleted = - (requestInterceptionExpected && Boolean(this.#request.paused)) || - !requestInterceptionExpected; + !requestInterceptionExpected || + (requestInterceptionExpected && Boolean(this.#request.paused)); if ( Boolean(this.#request.info) && @@ -194,8 +194,8 @@ export class NetworkRequest { ); const responseInterceptionCompleted = - (responseInterceptionExpected && Boolean(this.#response.paused)) || - !responseInterceptionExpected; + !responseInterceptionExpected || + (responseInterceptionExpected && Boolean(this.#response.paused)); if ( this.#response.info || @@ -291,7 +291,8 @@ export class NetworkRequest { } } - onAuthRequired(_event: Protocol.Fetch.AuthRequiredEvent) { + onAuthRequired(event: Protocol.Fetch.AuthRequiredEvent) { + this.#fetchId = event.requestId; this.#interceptPhase = Network.InterceptPhase.AuthRequired; if (!this.blocked) { void this.continueWithAuth(); diff --git a/src/bidiMapper/domains/network/NetworkStorage.spec.ts b/src/bidiMapper/domains/network/NetworkStorage.spec.ts index 0d9e15627e..9c4a913923 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.spec.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.spec.ts @@ -338,8 +338,9 @@ describe('NetworkStorage', () => { eventManager = new EventManager(browsingContextStorage); processingQueue = new ProcessingQueue( async ({message}) => { - const cdpEvent = message as unknown as ChromiumBidi.Event; - processedEvents.set(cdpEvent.method, cdpEvent.params); + if (message.type === 'event') { + processedEvents.set(message.method, message.params); + } return await Promise.resolve(); }, logger @@ -537,5 +538,13 @@ describe('NetworkStorage', () => { expect(event).to.exist; request.requestWillBeSent(); }); + + it('should work with only authRequired', async () => { + const request = new MockCdpNetworkEvents(cdpClient); + + request.authRequired(); + const event = await getEvent('network.authRequired'); + expect(event).to.exist; + }); }); }); diff --git a/src/bidiMapper/domains/network/NetworkStorage.ts b/src/bidiMapper/domains/network/NetworkStorage.ts index a36aa89989..48fac0fb02 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.ts @@ -322,21 +322,9 @@ export class NetworkStorage { cdpTarget: CdpTarget ) { // CDP quirk if the Network domain is not present this is undefined - const request = this.getRequestByFetchId(event.requestId); + let request = this.getRequestByFetchId(event.requestId); if (!request) { - // CDP quirk even both request/response may be continued - // with this command - void cdpTarget.cdpClient - .sendCommand('Fetch.continueWithAuth', { - requestId: event.requestId, - authChallengeResponse: { - response: 'Default', - }, - }) - .catch(() => { - // TODO: add logging - }); - return; + request = this.#getOrCreateNetworkRequest(event.requestId, cdpTarget); } request.onAuthRequired(event); From baa5ad56088934afc1969e4c2949200d06e31899 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 11:34:00 +0100 Subject: [PATCH 05/23] chore: 2 --- tests/tools/local_http_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tools/local_http_server.py b/tests/tools/local_http_server.py index 8b020adb70..90798f7428 100644 --- a/tests/tools/local_http_server.py +++ b/tests/tools/local_http_server.py @@ -89,7 +89,7 @@ def process_auth(request: Request): content_type="text/html") return Response( - '', 401, + 'HTTP Error 401 Unauthorized: Access is denied', 401, {"WWW-Authenticate": 'Basic realm="Access to staging site"'}) self.__http_server \ From 4c95c3c71d8830c567a5b115c4c5989fade194f8 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 14:34:44 +0100 Subject: [PATCH 06/23] chore: don't relay on phase --- .../domains/network/NetworkProcessor.ts | 31 +++++------ .../domains/network/NetworkRequest.ts | 54 +++++++++---------- .../domains/network/NetworkStorage.ts | 50 ++++++----------- tests/network/test_continue_request.py | 3 -- tests/network/test_continue_response.py | 6 +-- tests/network/test_continue_with_auth.py | 7 ++- tests/network/test_fail_request.py | 1 - tests/network/test_provide_response.py | 5 -- 8 files changed, 59 insertions(+), 98 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkProcessor.ts b/src/bidiMapper/domains/network/NetworkProcessor.ts index fbf6979f9b..064b9a62f4 100644 --- a/src/bidiMapper/domains/network/NetworkProcessor.ts +++ b/src/bidiMapper/domains/network/NetworkProcessor.ts @@ -67,10 +67,9 @@ export class NetworkProcessor { NetworkProcessor.parseUrlString(params.url); } - const request = this.#getBlockedRequestOrFail( - networkId, - Network.InterceptPhase.BeforeRequestSent - ); + const request = this.#getBlockedRequestOrFail(networkId, [ + Network.InterceptPhase.BeforeRequestSent, + ]); const {url, method, headers} = params; // TODO: Set / expand. @@ -90,10 +89,9 @@ export class NetworkProcessor { ): Promise { const networkId = params.request; const {statusCode, reasonPhrase, headers} = params; - const request = this.#getBlockedRequestOrFail( - networkId, - Network.InterceptPhase.ResponseStarted - ); + const request = this.#getBlockedRequestOrFail(networkId, [ + Network.InterceptPhase.ResponseStarted, + ]); const responseHeaders: Protocol.Fetch.HeaderEntry[] | undefined = cdpFetchHeadersFromBidiNetworkHeaders(headers); @@ -115,10 +113,9 @@ export class NetworkProcessor { params: Network.ContinueWithAuthParameters ): Promise { const networkId = params.request; - const request = this.#getBlockedRequestOrFail( - networkId, - Network.InterceptPhase.AuthRequired - ); + const request = this.#getBlockedRequestOrFail(networkId, [ + Network.InterceptPhase.AuthRequired, + ]); let username: string | undefined; let password: string | undefined; @@ -189,7 +186,11 @@ export class NetworkProcessor { // TODO: Set / expand. // ; Step 10. cookies // ; Step 11. credentials - const request = this.#getBlockedRequestOrFail(networkId, null); + const request = this.#getBlockedRequestOrFail(networkId, [ + Network.InterceptPhase.BeforeRequestSent, + Network.InterceptPhase.ResponseStarted, + Network.InterceptPhase.AuthRequired, + ]); await request.provideResponse({ responseCode: statusCode ?? request.statusCode, responsePhrase: reasonPhrase, @@ -220,7 +221,7 @@ export class NetworkProcessor { #getBlockedRequestOrFail( id: Network.Request, - phase: Network.InterceptPhase | null + phases: Network.InterceptPhase[] ): NetworkRequest { const request = this.#getRequestOrFail(id); if (!request.blocked) { @@ -228,7 +229,7 @@ export class NetworkProcessor { `No blocked request found for network id '${id}'` ); } - if (phase && request.phase !== phase) { + if (request.phase && !phases.includes(request.phase)) { throw new InvalidArgumentException( `Blocked request for network id '${id}' is in '${request.phase}' phase` ); diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index 468a89e680..57f224f3f9 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -144,8 +144,12 @@ export class NetworkRequest { return Boolean(this.#request.info); } + #isBlockedByInPhase(phase?: Network.InterceptPhase) { + return this.#networkStorage.requestBlockedBy(this, phase); + } + #isBlockedInPhase(phase?: Network.InterceptPhase) { - return this.#networkStorage.requestBlockedBy(this, phase).size > 0; + return this.#isBlockedByInPhase(phase).size > 0; } handleRedirect(event: Protocol.Network.RequestWillBeSentEvent): void { @@ -259,12 +263,11 @@ export class NetworkRequest { async failRequest(errorReason: Protocol.Network.ErrorReason) { assert(this.#fetchId, 'Network Interception not set-up.'); + this.#interceptPhase = undefined; await this.cdpClient.sendCommand('Fetch.failRequest', { requestId: this.#fetchId, errorReason, }); - - this.#interceptPhase = undefined; } onRequestPaused(event: Protocol.Fetch.RequestPausedEvent) { @@ -272,29 +275,26 @@ export class NetworkRequest { // CDP https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#event-requestPaused if (event.responseStatusCode || event.responseErrorReason) { - this.#interceptPhase = Network.InterceptPhase.ResponseStarted; this.#response.paused = event; - - this.#emitEventsIfReady(); - if (!this.blocked) { + this.#interceptPhase = Network.InterceptPhase.ResponseStarted; + if (!this.#isBlockedInPhase(Network.InterceptPhase.ResponseStarted)) { void this.continueResponse(); - } else if (!this.#response.info) { - this.#emitEventsIfReady(); } } else { this.#request.paused = event; this.#interceptPhase = Network.InterceptPhase.BeforeRequestSent; - this.#emitEventsIfReady(); - if (!this.blocked) { + if (!this.#isBlockedInPhase(Network.InterceptPhase.BeforeRequestSent)) { void this.continueRequest(); } } + + this.#emitEventsIfReady(); } onAuthRequired(event: Protocol.Fetch.AuthRequiredEvent) { this.#fetchId = event.requestId; this.#interceptPhase = Network.InterceptPhase.AuthRequired; - if (!this.blocked) { + if (!this.#isBlockedInPhase(Network.InterceptPhase.AuthRequired)) { void this.continueWithAuth(); } @@ -318,8 +318,8 @@ export class NetworkRequest { headers?: Protocol.Fetch.HeaderEntry[] ) { assert(this.#fetchId, 'Network Interception not set-up.'); - // TODO: Expand. + this.#interceptPhase = undefined; await this.cdpClient.sendCommand('Fetch.continueRequest', { requestId: this.#fetchId, url, @@ -329,8 +329,6 @@ export class NetworkRequest { // postData:, // interceptResponse:, }); - - this.#interceptPhase = undefined; } /** @see https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#method-continueResponse */ @@ -341,14 +339,13 @@ export class NetworkRequest { }: Omit = {}) { assert(this.#fetchId, 'Network Interception not set-up.'); + this.#interceptPhase = undefined; await this.cdpClient.sendCommand('Fetch.continueResponse', { requestId: this.#fetchId, responseCode, responsePhrase, responseHeaders, }); - - this.#interceptPhase = undefined; } /** @see https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#method-continueWithAuth */ @@ -359,12 +356,11 @@ export class NetworkRequest { ) { assert(this.#fetchId, 'Network Interception not set-up.'); + this.#interceptPhase = undefined; await this.cdpClient.sendCommand('Fetch.continueWithAuth', { requestId: this.#fetchId, authChallengeResponse, }); - - this.#interceptPhase = undefined; } /** @see https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#method-provideResponse */ @@ -376,6 +372,7 @@ export class NetworkRequest { }: Omit) { assert(this.#fetchId, 'Network Interception not set-up.'); + this.#interceptPhase = undefined; await this.cdpClient.sendCommand('Fetch.fulfillRequest', { requestId: this.#fetchId, responseCode, @@ -383,8 +380,6 @@ export class NetworkRequest { responseHeaders, ...(body ? {body: btoa(body)} : {}), // TODO: Double-check if btoa usage is correct. }); - - this.#interceptPhase = undefined; } dispose() { @@ -434,15 +429,14 @@ export class NetworkRequest { > = { isBlocked: false, }; - if (phase && phase === this.#interceptPhase) { - const blockedBy = this.#networkStorage.requestBlockedBy(this, phase); - interceptProps.isBlocked = blockedBy.size > 0; - if (interceptProps.isBlocked) { - interceptProps.intercepts = [...blockedBy] as [ - Network.Intercept, - ...Network.Intercept[], - ]; - } + + const blockedBy = this.#isBlockedByInPhase(phase); + interceptProps.isBlocked = blockedBy.size > 0; + if (interceptProps.isBlocked) { + interceptProps.intercepts = [...blockedBy] as [ + Network.Intercept, + ...Network.Intercept[], + ]; } return { diff --git a/src/bidiMapper/domains/network/NetworkStorage.ts b/src/bidiMapper/domains/network/NetworkStorage.ts index 48fac0fb02..8ff3d8e7a7 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.ts @@ -17,7 +17,7 @@ import type {Protocol} from 'devtools-protocol'; import {Network, NoSuchInterceptException} from '../../../protocol/protocol.js'; -import type {LoggerFn} from '../../../utils/log.js'; +import {LogType, type LoggerFn} from '../../../utils/log.js'; import {uuidv4} from '../../../utils/uuid.js'; import type {CdpClient} from '../../BidiMapper.js'; import type {CdpTarget} from '../context/CdpTarget.js'; @@ -116,8 +116,6 @@ export class NetworkStorage { cdpTarget, request.redirectCount + 1 ).onRequestWillBeSentEvent(params); - } else if (request) { - request.onRequestWillBeSentEvent(params); } else { this.#getOrCreateNetworkRequest( params.requestId, @@ -174,13 +172,25 @@ export class NetworkStorage { [ 'Fetch.requestPaused', (event: Protocol.Fetch.RequestPausedEvent) => { - this.#handleNetworkInterception(event, cdpTarget); + this.#getOrCreateNetworkRequest( + // CDP quirk if the Network domain is not present this is undefined + event.networkId ?? event.requestId, + cdpTarget + ).onRequestPaused(event); }, ], [ 'Fetch.authRequired', (event: Protocol.Fetch.AuthRequiredEvent) => { - this.#handleAuthInterception(event, cdpTarget); + let request = this.getRequestByFetchId(event.requestId); + if (!request) { + request = this.#getOrCreateNetworkRequest( + event.requestId, + cdpTarget + ); + } + + request.onAuthRequired(event); }, ], ] as const; @@ -300,36 +310,6 @@ export class NetworkStorage { } } - #handleNetworkInterception( - event: Protocol.Fetch.RequestPausedEvent, - cdpTarget: CdpTarget - ) { - // CDP quirk if the Network domain is not present this is undefined - let request = this.getRequestByFetchId(event.requestId); - - if (!request) { - request = this.#getOrCreateNetworkRequest( - event.networkId ?? event.requestId, - cdpTarget - ); - } - - request.onRequestPaused(event); - } - - #handleAuthInterception( - event: Protocol.Fetch.AuthRequiredEvent, - cdpTarget: CdpTarget - ) { - // CDP quirk if the Network domain is not present this is undefined - let request = this.getRequestByFetchId(event.requestId); - if (!request) { - request = this.#getOrCreateNetworkRequest(event.requestId, cdpTarget); - } - - request.onAuthRequired(event); - } - /** * Adds the given entry to the intercept map. * URL patterns are assumed to be parsed. diff --git a/tests/network/test_continue_request.py b/tests/network/test_continue_request.py index ba5b63ba28..c0bf23d7b0 100644 --- a/tests/network/test_continue_request.py +++ b/tests/network/test_continue_request.py @@ -38,7 +38,6 @@ async def test_continue_request_non_existent_request(websocket): }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_continue_request_invalid_phase_response_started( websocket, context_id, example_url): @@ -64,7 +63,6 @@ async def test_continue_request_invalid_phase_response_started( }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_continue_request_invalid_phase_auth_required( websocket, context_id, base_url, auth_required_url): @@ -92,7 +90,6 @@ async def test_continue_request_invalid_phase_auth_required( }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_continue_request_invalid_url(websocket, context_id, example_url): diff --git a/tests/network/test_continue_response.py b/tests/network/test_continue_response.py index 1aa2a80b71..f8e8087480 100644 --- a/tests/network/test_continue_response.py +++ b/tests/network/test_continue_response.py @@ -40,7 +40,6 @@ async def test_continue_response_non_existent_request(websocket): }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_continue_response_invalid_phase(websocket, context_id, example_url): @@ -74,7 +73,6 @@ async def test_continue_response_invalid_phase(websocket, context_id, }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_continue_response_invalid_status_code(websocket, context_id, example_url): @@ -100,7 +98,6 @@ async def test_continue_response_invalid_status_code(websocket, context_id, }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_continue_response_invalid_reason_phrase(websocket, context_id, example_url): @@ -125,7 +122,6 @@ async def test_continue_response_invalid_reason_phrase(websocket, context_id, }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_continue_response_invalid_headers(websocket, context_id, example_url): @@ -209,7 +205,7 @@ async def test_continue_response_non_blocked_request(websocket, context_id, "statusCode": 401, }, ], - ids=["headers-only", "statusCode-only"]) + ids=["headers-only", "statusCode-only"]) @pytest.mark.asyncio @pytest.mark.skip(reason="TODO: Fix this test, as it's racy") async def test_continue_response_must_specify_both_status_and_headers( diff --git a/tests/network/test_continue_with_auth.py b/tests/network/test_continue_with_auth.py index 6150ea18ef..a35b7f36aa 100644 --- a/tests/network/test_continue_with_auth.py +++ b/tests/network/test_continue_with_auth.py @@ -39,7 +39,6 @@ async def test_continue_with_auth_non_existent_request(websocket): }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio @pytest.mark.parametrize("phase", ["beforeRequestSent", "responseStarted"]) async def test_continue_with_auth_invalid_phase(websocket, context_id, @@ -112,17 +111,17 @@ async def test_continue_with_auth_non_blocked_request( }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio @pytest.mark.parametrize("credentials", [{}, { "type": "notapassword", "username": "user", "password": "pass" }], - ids=["empty", "invalid type value"]) + ids=["empty", "invalid type value"]) +@pytest.mark.parametrize('execution_number', range(1)) async def test_continue_with_auth_invalid_credentials(websocket, context_id, auth_required_url, - credentials, base_url): + credentials, base_url, execution_number): await goto_url(websocket, context_id, base_url) network_id = await create_blocked_request(websocket, diff --git a/tests/network/test_fail_request.py b/tests/network/test_fail_request.py index 0554740088..d6c84f14ee 100644 --- a/tests/network/test_fail_request.py +++ b/tests/network/test_fail_request.py @@ -163,7 +163,6 @@ async def test_fail_request_twice(websocket, context_id, example_url): }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_fail_request_with_auth_required_phase(websocket, context_id, auth_required_url, diff --git a/tests/network/test_provide_response.py b/tests/network/test_provide_response.py index 1bee5aff93..2576fc8ad7 100644 --- a/tests/network/test_provide_response.py +++ b/tests/network/test_provide_response.py @@ -40,7 +40,6 @@ async def test_provide_response_non_existent_request(websocket): }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_provide_response_beforeRequestSent_is_valid_phase( websocket, context_id, example_url): @@ -60,7 +59,6 @@ async def test_provide_response_beforeRequestSent_is_valid_phase( }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_provide_response_invalid_status_code(websocket, context_id, example_url): @@ -86,7 +84,6 @@ async def test_provide_response_invalid_status_code(websocket, context_id, }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_provide_response_invalid_reason_phrase(websocket, context_id, example_url): @@ -111,7 +108,6 @@ async def test_provide_response_invalid_reason_phrase(websocket, context_id, }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_provide_response_invalid_headers(websocket, context_id, example_url): @@ -136,7 +132,6 @@ async def test_provide_response_invalid_headers(websocket, context_id, }) -@pytest.mark.skip(reason="TODO: #1883") @pytest.mark.asyncio async def test_provide_response_invalid_body(websocket, context_id, example_url): From cb2e70a7dd78fe3863f03f47a11f1dd1b9b2db7c Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 14:43:23 +0100 Subject: [PATCH 07/23] chore: fix style --- src/bidiMapper/domains/network/NetworkStorage.ts | 2 +- tests/network/test_continue_response.py | 2 +- tests/network/test_continue_with_auth.py | 13 ++++++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkStorage.ts b/src/bidiMapper/domains/network/NetworkStorage.ts index 8ff3d8e7a7..4c2937b8c7 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.ts @@ -17,7 +17,7 @@ import type {Protocol} from 'devtools-protocol'; import {Network, NoSuchInterceptException} from '../../../protocol/protocol.js'; -import {LogType, type LoggerFn} from '../../../utils/log.js'; +import type {LoggerFn} from '../../../utils/log.js'; import {uuidv4} from '../../../utils/uuid.js'; import type {CdpClient} from '../../BidiMapper.js'; import type {CdpTarget} from '../context/CdpTarget.js'; diff --git a/tests/network/test_continue_response.py b/tests/network/test_continue_response.py index f8e8087480..54a51d1b63 100644 --- a/tests/network/test_continue_response.py +++ b/tests/network/test_continue_response.py @@ -205,7 +205,7 @@ async def test_continue_response_non_blocked_request(websocket, context_id, "statusCode": 401, }, ], - ids=["headers-only", "statusCode-only"]) + ids=["headers-only", "statusCode-only"]) @pytest.mark.asyncio @pytest.mark.skip(reason="TODO: Fix this test, as it's racy") async def test_continue_response_must_specify_both_status_and_headers( diff --git a/tests/network/test_continue_with_auth.py b/tests/network/test_continue_with_auth.py index a35b7f36aa..951b8874be 100644 --- a/tests/network/test_continue_with_auth.py +++ b/tests/network/test_continue_with_auth.py @@ -117,11 +117,14 @@ async def test_continue_with_auth_non_blocked_request( "username": "user", "password": "pass" }], - ids=["empty", "invalid type value"]) -@pytest.mark.parametrize('execution_number', range(1)) -async def test_continue_with_auth_invalid_credentials(websocket, context_id, - auth_required_url, - credentials, base_url, execution_number): + ids=["empty", "invalid type value"]) +async def test_continue_with_auth_invalid_credentials( + websocket, + context_id, + auth_required_url, + credentials, + base_url, +): await goto_url(websocket, context_id, base_url) network_id = await create_blocked_request(websocket, From 104de6aaa9715bb99b9b6b4b254cc867d90f083a Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 15:09:33 +0100 Subject: [PATCH 08/23] chore: mark test correctly --- tests/network/test_continue_request.py | 19 +++++++++++-------- tests/network/test_continue_response.py | 4 ++-- tests/network/test_continue_with_auth.py | 4 ++-- tests/network/test_fail_request.py | 2 +- tests/network/test_provide_response.py | 2 +- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/tests/network/test_continue_request.py b/tests/network/test_continue_request.py index c0bf23d7b0..2b7326e720 100644 --- a/tests/network/test_continue_request.py +++ b/tests/network/test_continue_request.py @@ -320,6 +320,7 @@ async def test_continue_request_twice(websocket, context_id, example_url): @pytest.mark.asyncio +@pytest.mark.skip(reason="TODO: #1890") async def test_continue_request_remove_intercept_inflight_request( websocket, context_id, example_url): @@ -385,17 +386,19 @@ async def test_continue_request_remove_intercept_inflight_request( }) assert result == {} + network_id = event_response["params"]["request"]["request"] + # TODO: Clarify the behavior of of removing intercept # while there are inflight requests. - # await execute_command( - # websocket, { - # "method": "network.continueRequest", - # "params": { - # "request": network_id, - # "url": example_url, - # }, - # }) + await execute_command( + websocket, { + "method": "network.continueRequest", + "params": { + "request": network_id, + "url": example_url, + }, + }) event_response = await wait_for_event(websocket, "network.responseCompleted") diff --git a/tests/network/test_continue_response.py b/tests/network/test_continue_response.py index 54a51d1b63..6a0c7982c4 100644 --- a/tests/network/test_continue_response.py +++ b/tests/network/test_continue_response.py @@ -205,7 +205,7 @@ async def test_continue_response_non_blocked_request(websocket, context_id, "statusCode": 401, }, ], - ids=["headers-only", "statusCode-only"]) + ids=["headers-only", "statusCode-only"]) @pytest.mark.asyncio @pytest.mark.skip(reason="TODO: Fix this test, as it's racy") async def test_continue_response_must_specify_both_status_and_headers( @@ -443,7 +443,7 @@ async def test_continue_response_twice(websocket, context_id, example_url): @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Clarify how removing the last intercept works") +@pytest.mark.skip(reason="TODO: #1890") async def test_continue_response_remove_intercept_inflight_request( websocket, context_id, example_url): diff --git a/tests/network/test_continue_with_auth.py b/tests/network/test_continue_with_auth.py index 951b8874be..f106cbd70d 100644 --- a/tests/network/test_continue_with_auth.py +++ b/tests/network/test_continue_with_auth.py @@ -117,7 +117,7 @@ async def test_continue_with_auth_non_blocked_request( "username": "user", "password": "pass" }], - ids=["empty", "invalid type value"]) + ids=["empty", "invalid type value"]) async def test_continue_with_auth_invalid_credentials( websocket, context_id, @@ -330,7 +330,7 @@ async def test_continue_with_auth_twice(websocket, context_id, example_url): @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Fix this test") +@pytest.mark.skip(reason="TODO: #1890") async def test_continue_with_auth_remove_intercept_inflight_request( websocket, context_id, example_url, auth_required_url): await subscribe(websocket, diff --git a/tests/network/test_fail_request.py b/tests/network/test_fail_request.py index d6c84f14ee..5d10c0d80c 100644 --- a/tests/network/test_fail_request.py +++ b/tests/network/test_fail_request.py @@ -594,7 +594,7 @@ async def test_fail_request_multiple_contexts(websocket, context_id, @pytest.mark.asyncio @pytest.mark.skip( - reason='TODO: Clarify the behavior after last intercept is removed') + reason='TODO: #1890') async def test_fail_request_remove_intercept_inflight_request( websocket, context_id, example_url): diff --git a/tests/network/test_provide_response.py b/tests/network/test_provide_response.py index 2576fc8ad7..4f480729f6 100644 --- a/tests/network/test_provide_response.py +++ b/tests/network/test_provide_response.py @@ -396,7 +396,7 @@ async def test_provide_response_twice(websocket, context_id, example_url): @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Fix this test") +@pytest.mark.skip(reason="TODO: #1890") async def test_provide_response_remove_intercept_inflight_request( websocket, context_id, example_url): await subscribe(websocket, From 27a29e3628126dfcfeffd8c1976cbe9785b14a8d Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 15:11:45 +0100 Subject: [PATCH 09/23] chore: enable test --- tests/network/test_continue_with_auth.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/network/test_continue_with_auth.py b/tests/network/test_continue_with_auth.py index f106cbd70d..34bbf33bc1 100644 --- a/tests/network/test_continue_with_auth.py +++ b/tests/network/test_continue_with_auth.py @@ -43,8 +43,6 @@ async def test_continue_with_auth_non_existent_request(websocket): @pytest.mark.parametrize("phase", ["beforeRequestSent", "responseStarted"]) async def test_continue_with_auth_invalid_phase(websocket, context_id, example_url, phase): - if phase == "responseStarted": - pytest.xfail(reason="TODO: Fix test for responseStarted phase") network_id = await create_blocked_request(websocket, context_id, url=example_url, From 751ec4fef72b9890a05ce338e3258e3508a622ef Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 17:28:02 +0100 Subject: [PATCH 10/23] chore: enable tests --- .../domains/network/NetworkRequest.ts | 10 +-- tests/network/test_continue_response.py | 34 ++++------ tests/network/test_continue_with_auth.py | 64 +++++++------------ tests/network/test_fail_request.py | 4 +- tests/network/test_provide_response.py | 27 ++++---- 5 files changed, 54 insertions(+), 85 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index 57f224f3f9..0caa6fc496 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -152,7 +152,7 @@ export class NetworkRequest { return this.#isBlockedByInPhase(phase).size > 0; } - handleRedirect(event: Protocol.Network.RequestWillBeSentEvent): void { + handleRedirect(event: Protocol.Network.RequestWillBeSentEvent) { this.#response.hasExtraInfo = event.redirectHasExtraInfo; this.#response.info = event.redirectResponse!; this.#emitEventsIfReady(true); @@ -197,10 +197,6 @@ export class NetworkRequest { Network.InterceptPhase.ResponseStarted ); - const responseInterceptionCompleted = - !responseInterceptionExpected || - (responseInterceptionExpected && Boolean(this.#response.paused)); - if ( this.#response.info || (responseInterceptionExpected && Boolean(this.#response.paused)) @@ -208,6 +204,10 @@ export class NetworkRequest { this.#emitEvent(this.#getResponseStartedEvent.bind(this)); } + const responseInterceptionCompleted = + !responseInterceptionExpected || + (responseInterceptionExpected && Boolean(this.#response.paused)); + if ( Boolean(this.#response.info) && responseExtraInfoCompleted && diff --git a/tests/network/test_continue_response.py b/tests/network/test_continue_response.py index 6a0c7982c4..f718a2859a 100644 --- a/tests/network/test_continue_response.py +++ b/tests/network/test_continue_response.py @@ -205,13 +205,12 @@ async def test_continue_response_non_blocked_request(websocket, context_id, "statusCode": 401, }, ], - ids=["headers-only", "statusCode-only"]) + ids=["headers-only", "statusCode-only"]) @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Fix this test, as it's racy") async def test_continue_response_must_specify_both_status_and_headers( websocket, context_id, example_url, continueResponseParams): await subscribe(websocket, - ["network.beforeRequestSent", "network.responseCompleted"], + ["network.responseStarted", "network.responseCompleted"], [context_id]) await execute_command( @@ -236,8 +235,7 @@ async def test_continue_response_must_specify_both_status_and_headers( } }) - event_response = await wait_for_event(websocket, - "network.beforeRequestSent") + event_response = await wait_for_event(websocket, "network.responseStarted") network_id = event_response["params"]["request"]["request"] # TODO(https://github.com/w3c/webdriver-bidi/issues/572): Follow up with spec. @@ -258,11 +256,12 @@ async def test_continue_response_must_specify_both_status_and_headers( @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Fix this test") +@pytest.mark.skip(reason="CDP does not update the response correctly") async def test_continue_response_completes(websocket, context_id, example_url): - await subscribe(websocket, - ["network.beforeRequestSent", "network.responseCompleted"], - [context_id]) + await subscribe(websocket, [ + "network.beforeRequestSent", "network.responseStarted", + "network.responseCompleted" + ], [context_id]) await execute_command( websocket, { @@ -291,9 +290,6 @@ async def test_continue_response_completes(websocket, context_id, example_url): "method": "network.responseStarted", "params": { "context": context_id, - "initiator": { - "type": "other", - }, "intercepts": ANY_LIST, "isBlocked": True, "navigation": ANY_STR, @@ -308,6 +304,7 @@ async def test_continue_response_completes(websocket, context_id, example_url): "bodySize": 0, "timings": ANY_DICT, }, + "response": ANY_DICT, "timestamp": ANY_TIMESTAMP, }, "type": "event", @@ -360,10 +357,9 @@ async def test_continue_response_completes(websocket, context_id, example_url): @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Fix this test") async def test_continue_response_twice(websocket, context_id, example_url): await subscribe(websocket, - ["network.beforeRequestSent", "network.responseCompleted"], + ["network.responseStarted", "network.responseCompleted"], [context_id]) await execute_command( @@ -388,15 +384,12 @@ async def test_continue_response_twice(websocket, context_id, example_url): } }) - event_response = await wait_for_event(websocket, - "network.beforeRequestSent") + event_response = await wait_for_event(websocket, "network.responseStarted") assert event_response == { - "method": "network.beforeRequestSent", + "method": "network.responseStarted", "params": { "context": context_id, - "initiator": { - "type": "other", - }, + "intercepts": ANY_LIST, "isBlocked": True, "navigation": ANY_STR, "redirectCount": 0, @@ -410,6 +403,7 @@ async def test_continue_response_twice(websocket, context_id, example_url): "bodySize": 0, "timings": ANY_DICT, }, + "response": ANY_DICT, "timestamp": ANY_TIMESTAMP, }, "type": "event", diff --git a/tests/network/test_continue_with_auth.py b/tests/network/test_continue_with_auth.py index 34bbf33bc1..e6f39745bc 100644 --- a/tests/network/test_continue_with_auth.py +++ b/tests/network/test_continue_with_auth.py @@ -14,9 +14,8 @@ # limitations under the License. import pytest from anys import ANY_DICT, ANY_LIST, ANY_NUMBER, ANY_STR -from test_helpers import (ANY_TIMESTAMP, ANY_UUID, AnyExtending, - execute_command, goto_url, send_JSON_command, - subscribe, wait_for_event) +from test_helpers import (ANY_TIMESTAMP, ANY_UUID, execute_command, goto_url, + send_JSON_command, subscribe, wait_for_event) from . import create_blocked_request @@ -115,7 +114,7 @@ async def test_continue_with_auth_non_blocked_request( "username": "user", "password": "pass" }], - ids=["empty", "invalid type value"]) + ids=["empty", "invalid type value"]) async def test_continue_with_auth_invalid_credentials( websocket, context_id, @@ -147,13 +146,11 @@ async def test_continue_with_auth_invalid_credentials( @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Fix this test") async def test_continue_with_auth_completes(websocket, context_id, auth_required_url): - await subscribe(websocket, [ - "network.beforeRequestSent", "network.authRequired", - "network.responseCompleted" - ], [context_id]) + await subscribe(websocket, + ["network.authRequired", "network.responseCompleted"], + [context_id]) await execute_command( websocket, { @@ -177,15 +174,12 @@ async def test_continue_with_auth_completes(websocket, context_id, } }) - event_response = await wait_for_event(websocket, - "network.beforeRequestSent") + event_response = await wait_for_event(websocket, "network.authRequired") assert event_response == { - "method": "network.beforeRequestSent", + "method": "network.authRequired", "params": { "context": context_id, - "initiator": { - "type": "other", - }, + "intercepts": ANY_LIST, "isBlocked": True, "navigation": ANY_STR, "redirectCount": 0, @@ -199,6 +193,7 @@ async def test_continue_with_auth_completes(websocket, context_id, "bodySize": 0, "timings": ANY_DICT, }, + "response": ANY_DICT, "timestamp": ANY_TIMESTAMP, }, "type": "event", @@ -224,18 +219,7 @@ async def test_continue_with_auth_completes(websocket, context_id, "navigation": ANY_STR, "redirectCount": 0, "request": ANY_DICT, - "response": AnyExtending({ - "headers": [{ - "name": "header1", - "value": { - "type": "string", - "value": "value1", - }, - }, ], - "url": auth_required_url, - "status": 501, - "statusText": "Remember to drink water", - }), + "response": ANY_DICT, "timestamp": ANY_TIMESTAMP, }, "type": "event", @@ -243,20 +227,20 @@ async def test_continue_with_auth_completes(websocket, context_id, @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Fix this test") -async def test_continue_with_auth_twice(websocket, context_id, example_url): +async def test_continue_with_auth_twice(websocket, context_id, + auth_required_url): await subscribe(websocket, - ["network.beforeRequestSent", "network.responseCompleted"], + ["network.authRequired", "network.responseCompleted"], [context_id]) await execute_command( websocket, { "method": "network.addIntercept", "params": { - "phases": ["responseStarted"], + "phases": ["authRequired"], "urlPatterns": [{ "type": "string", - "pattern": example_url, + "pattern": auth_required_url, }, ], }, }) @@ -265,27 +249,24 @@ async def test_continue_with_auth_twice(websocket, context_id, example_url): websocket, { "method": "browsingContext.navigate", "params": { - "url": example_url, + "url": auth_required_url, "context": context_id, "wait": "complete", } }) - event_response = await wait_for_event(websocket, - "network.beforeRequestSent") + event_response = await wait_for_event(websocket, "network.authRequired") assert event_response == { - "method": "network.beforeRequestSent", + "method": "network.authRequired", "params": { "context": context_id, - "initiator": { - "type": "other", - }, - "isBlocked": False, + "intercepts": ANY_LIST, + "isBlocked": True, "navigation": ANY_STR, "redirectCount": 0, "request": { "request": ANY_STR, - "url": example_url, + "url": auth_required_url, "method": "GET", "headers": ANY_LIST, "cookies": [], @@ -293,6 +274,7 @@ async def test_continue_with_auth_twice(websocket, context_id, example_url): "bodySize": 0, "timings": ANY_DICT, }, + "response": ANY_DICT, "timestamp": ANY_TIMESTAMP, }, "type": "event", diff --git a/tests/network/test_fail_request.py b/tests/network/test_fail_request.py index 5d10c0d80c..a4f8886148 100644 --- a/tests/network/test_fail_request.py +++ b/tests/network/test_fail_request.py @@ -282,7 +282,6 @@ async def test_fail_request_completes(websocket, context_id, example_url): @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Fix this test") async def test_fail_request_completes_new_request_still_blocks( websocket, context_id, example_url): await subscribe(websocket, ["network.beforeRequestSent"], [context_id]) @@ -593,8 +592,7 @@ async def test_fail_request_multiple_contexts(websocket, context_id, @pytest.mark.asyncio -@pytest.mark.skip( - reason='TODO: #1890') +@pytest.mark.skip(reason='TODO: #1890') async def test_fail_request_remove_intercept_inflight_request( websocket, context_id, example_url): diff --git a/tests/network/test_provide_response.py b/tests/network/test_provide_response.py index 4f480729f6..119fe35044 100644 --- a/tests/network/test_provide_response.py +++ b/tests/network/test_provide_response.py @@ -204,10 +204,10 @@ async def test_provide_response_non_blocked_request(websocket, context_id, @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Fix this test") +@pytest.mark.skip(reason="CDP does not update the response correctly") async def test_provide_response_completes(websocket, context_id, example_url): await subscribe(websocket, - ["network.beforeRequestSent", "network.responseCompleted"], + ["network.responseStarted", "network.responseCompleted"], [context_id]) await execute_command( @@ -232,15 +232,12 @@ async def test_provide_response_completes(websocket, context_id, example_url): } }) - event_response = await wait_for_event(websocket, - "network.beforeRequestSent") + event_response = await wait_for_event(websocket, "network.responseStarted") assert event_response == { - "method": "network.beforeRequestSent", + "method": "network.responseStarted", "params": { "context": context_id, - "initiator": { - "type": "other", - }, + "intercepts": ANY_LIST, "isBlocked": True, "navigation": ANY_STR, "redirectCount": 0, @@ -254,6 +251,7 @@ async def test_provide_response_completes(websocket, context_id, example_url): "bodySize": 0, "timings": ANY_DICT, }, + "response": ANY_DICT, "timestamp": ANY_TIMESTAMP, }, "type": "event", @@ -311,10 +309,9 @@ async def test_provide_response_completes(websocket, context_id, example_url): @pytest.mark.asyncio -@pytest.mark.skip(reason="TODO: Fix this test") async def test_provide_response_twice(websocket, context_id, example_url): await subscribe(websocket, - ["network.beforeRequestSent", "network.responseCompleted"], + ["network.responseStarted", "network.responseCompleted"], [context_id]) await execute_command( @@ -339,15 +336,12 @@ async def test_provide_response_twice(websocket, context_id, example_url): } }) - event_response = await wait_for_event(websocket, - "network.beforeRequestSent") + event_response = await wait_for_event(websocket, "network.responseStarted") assert event_response == { - "method": "network.beforeRequestSent", + "method": "network.responseStarted", "params": { "context": context_id, - "initiator": { - "type": "other", - }, + "intercepts": ANY_LIST, "isBlocked": True, "navigation": ANY_STR, "redirectCount": 0, @@ -361,6 +355,7 @@ async def test_provide_response_twice(websocket, context_id, example_url): "bodySize": 0, "timings": ANY_DICT, }, + "response": ANY_DICT, "timestamp": ANY_TIMESTAMP, }, "type": "event", From 2cbfd4f48bd3568f63ef11474be2e873acef47c7 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 20:07:54 +0100 Subject: [PATCH 11/23] chore: disable url_patterns test for now --- .../network/add_intercept/url_patterns.py.ini | 117 +----------------- .../network/add_intercept/url_patterns.py.ini | 117 +----------------- .../network/add_intercept/url_patterns.py.ini | 117 +----------------- .../network/add_intercept/url_patterns.py.ini | 117 +----------------- 4 files changed, 4 insertions(+), 464 deletions(-) diff --git a/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini b/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini index 794953c915..9b33871a6f 100644 --- a/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini +++ b/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini @@ -1,117 +1,2 @@ [url_patterns.py] - [test_pattern_patterns_matching[patterns10-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns11-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns12-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns13-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns14-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns15-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns16-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns17-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns18-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns19-https://{wpt_host_upper}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns20-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns21-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns22-https://{wpt_host}\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns23-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns24-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns25-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns26-https://{wpt_host}/?\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns27-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern0-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern1-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern2-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern3-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern4-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern5-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern6-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern7-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern8-https://{wpt_host}/path/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern9-https://{wpt_host}/other/path\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern10-https://{wpt_host}/path/continued\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern11-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern12-https://{wpt_host}/?other\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[HTTPS://{wpt_host}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host_upper}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://user:password@{wpt_host}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}:443/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}:443/-https://{wpt_host}:443/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}/#ref-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}/#ref1-https://{wpt_host}/#ref2\]] - expected: FAIL + expected: TIMEOUT diff --git a/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini b/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini index 794953c915..9b33871a6f 100644 --- a/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini +++ b/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini @@ -1,117 +1,2 @@ [url_patterns.py] - [test_pattern_patterns_matching[patterns10-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns11-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns12-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns13-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns14-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns15-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns16-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns17-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns18-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns19-https://{wpt_host_upper}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns20-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns21-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns22-https://{wpt_host}\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns23-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns24-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns25-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns26-https://{wpt_host}/?\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns27-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern0-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern1-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern2-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern3-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern4-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern5-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern6-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern7-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern8-https://{wpt_host}/path/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern9-https://{wpt_host}/other/path\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern10-https://{wpt_host}/path/continued\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern11-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern12-https://{wpt_host}/?other\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[HTTPS://{wpt_host}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host_upper}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://user:password@{wpt_host}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}:443/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}:443/-https://{wpt_host}:443/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}/#ref-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}/#ref1-https://{wpt_host}/#ref2\]] - expected: FAIL + expected: TIMEOUT diff --git a/wpt-metadata/mapper/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini b/wpt-metadata/mapper/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini index 794953c915..9b33871a6f 100644 --- a/wpt-metadata/mapper/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini +++ b/wpt-metadata/mapper/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini @@ -1,117 +1,2 @@ [url_patterns.py] - [test_pattern_patterns_matching[patterns10-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns11-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns12-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns13-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns14-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns15-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns16-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns17-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns18-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns19-https://{wpt_host_upper}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns20-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns21-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns22-https://{wpt_host}\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns23-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns24-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns25-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns26-https://{wpt_host}/?\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns27-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern0-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern1-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern2-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern3-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern4-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern5-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern6-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern7-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern8-https://{wpt_host}/path/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern9-https://{wpt_host}/other/path\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern10-https://{wpt_host}/path/continued\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern11-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern12-https://{wpt_host}/?other\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[HTTPS://{wpt_host}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host_upper}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://user:password@{wpt_host}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}:443/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}:443/-https://{wpt_host}:443/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}/#ref-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}/#ref1-https://{wpt_host}/#ref2\]] - expected: FAIL + expected: TIMEOUT diff --git a/wpt-metadata/mapper/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini b/wpt-metadata/mapper/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini index 794953c915..9b33871a6f 100644 --- a/wpt-metadata/mapper/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini +++ b/wpt-metadata/mapper/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini @@ -1,117 +1,2 @@ [url_patterns.py] - [test_pattern_patterns_matching[patterns10-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns11-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns12-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns13-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns14-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns15-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns16-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns17-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns18-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns19-https://{wpt_host_upper}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns20-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns21-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns22-https://{wpt_host}\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns23-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns24-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns25-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns26-https://{wpt_host}/?\]] - expected: FAIL - - [test_pattern_patterns_matching[patterns27-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern0-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern1-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern2-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern3-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern4-https://{wpt_host}:1234/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern5-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern6-https://{wpt_host}/path\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern7-https://{wpt_host}/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern8-https://{wpt_host}/path/\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern9-https://{wpt_host}/other/path\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern10-https://{wpt_host}/path/continued\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern11-https://{wpt_host}/?search\]] - expected: FAIL - - [test_pattern_patterns_not_matching[pattern12-https://{wpt_host}/?other\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[HTTPS://{wpt_host}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host_upper}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://user:password@{wpt_host}/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}:443/-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}:443/-https://{wpt_host}:443/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}/#ref-https://{wpt_host}/\]] - expected: FAIL - - [test_string_patterns_matching[https://{wpt_host}/#ref1-https://{wpt_host}/#ref2\]] - expected: FAIL + expected: TIMEOUT From 7f398b207c9beb4918b1768759abc3ecfe753b2d Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 20:08:24 +0100 Subject: [PATCH 12/23] chore: better matching --- .../domains/network/NetworkUtils.spec.ts | 78 +++++++++++-------- .../domains/network/NetworkUtils.ts | 28 +------ 2 files changed, 47 insertions(+), 59 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkUtils.spec.ts b/src/bidiMapper/domains/network/NetworkUtils.spec.ts index 0d9d977b92..735a6442a5 100644 --- a/src/bidiMapper/domains/network/NetworkUtils.spec.ts +++ b/src/bidiMapper/domains/network/NetworkUtils.spec.ts @@ -232,30 +232,6 @@ describe('NetworkUtils', () => { }); }); - describe('cdpFromSpecUrlPattern', () => { - it('string type', () => { - expect( - networkUtils.cdpFromSpecUrlPattern({ - type: 'string', - pattern: 'https://example.com', - } satisfies Network.UrlPattern) - ).to.equal('https://example.com'); - }); - - it('pattern type', () => { - expect( - networkUtils.cdpFromSpecUrlPattern({ - type: 'pattern', - protocol: 'https', - hostname: 'example.com', - port: '80', - pathname: '/foo', - search: 'bar=baz', - } satisfies Network.UrlPattern) - ).to.equal('https://example.com:80/foo?bar=baz'); - }); - }); - describe('buildUrlPatternString', () => { describe('protocol', () => { it('empty', () => { @@ -412,16 +388,50 @@ describe('NetworkUtils', () => { }); describe('matchUrlPattern', () => { - it('string type', () => { - expect( - networkUtils.matchUrlPattern( - { - type: 'string', - pattern: 'https://example.com', - } satisfies Network.UrlPattern, - 'https://example.com' - ) - ).to.be.true; + describe('string type', () => { + it('positive match', () => { + expect( + networkUtils.matchUrlPattern( + { + type: 'string', + pattern: 'https://example.com', + } satisfies Network.UrlPattern, + 'https://example.com' + ) + ).to.be.true; + }); + it('negative match', () => { + const test = [ + ['https://example.com/', 'https://some.other.host/'], + ['https://example.com:1234/', 'https://example.com:5678/'], + ['https://example.com/', 'https://example.com:5678/'], + ['https://example.com/path', 'https://example.com/other/path'], + ['https://example.com/path', 'https://example.com/path/continued'], + ['https://example.com/pathcase', 'https://example.com/PATHCASE'], + [ + 'https://example.com/?searchcase', + 'https://example.com/?SEARCHCASE', + ], + ['https://example.com/?key', 'https://example.com/?otherkey'], + ['https://example.com/?key', 'https://example.com/?key=value'], + ['https://example.com/?a=b&c=d', 'https://example.com/?c=d&a=b'], + // TODO: Consider removing ? + // ['https://example.com/??', 'https://example.com/?'], + ] as const; + + for (const [url, expected] of test) { + expect( + networkUtils.matchUrlPattern( + { + type: 'string', + pattern: url, + } satisfies Network.UrlPattern, + expected + ), + `${url} should match ${expected}` + ).to.be.false; + } + }); }); describe('pattern type', () => { diff --git a/src/bidiMapper/domains/network/NetworkUtils.ts b/src/bidiMapper/domains/network/NetworkUtils.ts index eddcf0cdcb..922bb6b8b5 100644 --- a/src/bidiMapper/domains/network/NetworkUtils.ts +++ b/src/bidiMapper/domains/network/NetworkUtils.ts @@ -306,29 +306,7 @@ export function matchUrlPattern( urlPattern: Network.UrlPattern, url: string | undefined ): boolean { - switch (urlPattern.type) { - case 'string': - return urlPattern.pattern === url; - case 'pattern': { - return ( - new URLPattern({ - protocol: urlPattern.protocol, - hostname: urlPattern.hostname, - port: urlPattern.port, - pathname: urlPattern.pathname, - search: urlPattern.search, - }).exec(url) !== null - ); - } - } -} - -/** Converts a URL pattern from the spec to a CDP URL pattern. */ -export function cdpFromSpecUrlPattern(urlPattern: Network.UrlPattern): string { - switch (urlPattern.type) { - case 'string': - return urlPattern.pattern; - case 'pattern': - return buildUrlPatternString(urlPattern); - } + return new URLPattern( + urlPattern.type === 'string' ? urlPattern.pattern : urlPattern + ).test(url); } From 501fedf33232e01f56c8f489a710e3d9ca3647f7 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 20:13:09 +0100 Subject: [PATCH 13/23] chore: remove comment --- tests/network/test_continue_request.py | 3 --- tests/network/test_continue_response.py | 5 +---- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/network/test_continue_request.py b/tests/network/test_continue_request.py index 2b7326e720..f293a07e5e 100644 --- a/tests/network/test_continue_request.py +++ b/tests/network/test_continue_request.py @@ -388,9 +388,6 @@ async def test_continue_request_remove_intercept_inflight_request( network_id = event_response["params"]["request"]["request"] - # TODO: Clarify the behavior of of removing intercept - # while there are inflight requests. - await execute_command( websocket, { "method": "network.continueRequest", diff --git a/tests/network/test_continue_response.py b/tests/network/test_continue_response.py index f718a2859a..f0ec39e06a 100644 --- a/tests/network/test_continue_response.py +++ b/tests/network/test_continue_response.py @@ -205,7 +205,7 @@ async def test_continue_response_non_blocked_request(websocket, context_id, "statusCode": 401, }, ], - ids=["headers-only", "statusCode-only"]) + ids=["headers-only", "statusCode-only"]) @pytest.mark.asyncio async def test_continue_response_must_specify_both_status_and_headers( websocket, context_id, example_url, continueResponseParams): @@ -493,9 +493,6 @@ async def test_continue_response_remove_intercept_inflight_request( "type": "event", } - # TODO: Clarify the behavior of of removing intercept - # while there are inflight requests. - result = await execute_command( websocket, { "method": "network.removeIntercept", From 45afef5eb44df898c318769fedd824d31411d277 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 20:42:01 +0100 Subject: [PATCH 14/23] chore: simplify implementation --- .../domains/network/NetworkProcessor.ts | 10 +++--- .../domains/network/NetworkRequest.ts | 33 +++++++++---------- .../domains/network/NetworkStorage.ts | 17 +++------- 3 files changed, 25 insertions(+), 35 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkProcessor.ts b/src/bidiMapper/domains/network/NetworkProcessor.ts index 064b9a62f4..f1a360fdd7 100644 --- a/src/bidiMapper/domains/network/NetworkProcessor.ts +++ b/src/bidiMapper/domains/network/NetworkProcessor.ts @@ -150,12 +150,12 @@ export class NetworkProcessor { request: networkId, }: Network.FailRequestParameters): Promise { const request = this.#getRequestOrFail(networkId); - if (request.phase === Network.InterceptPhase.AuthRequired) { + if (request.blockedIn === Network.InterceptPhase.AuthRequired) { throw new InvalidArgumentException( `Request '${networkId}' in 'authRequired' phase cannot be failed` ); } - if (!request.blocked) { + if (!request.blockedIn) { throw new NoSuchRequestException( `No blocked request found for network id '${networkId}'` ); @@ -224,14 +224,14 @@ export class NetworkProcessor { phases: Network.InterceptPhase[] ): NetworkRequest { const request = this.#getRequestOrFail(id); - if (!request.blocked) { + if (!request.blockedIn) { throw new NoSuchRequestException( `No blocked request found for network id '${id}'` ); } - if (request.phase && !phases.includes(request.phase)) { + if (request.blockedIn && !phases.includes(request.blockedIn)) { throw new InvalidArgumentException( - `Blocked request for network id '${id}' is in '${request.phase}' phase` + `Blocked request for network id '${id}' is in '${request.blockedIn}' phase` ); } diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index 0caa6fc496..93cf35e7e3 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -115,7 +115,10 @@ export class NetworkRequest { return this.#fetchId; } - get phase(): Network.InterceptPhase | undefined { + /** + * When block returns the phase for it + */ + get blockedIn(): Network.InterceptPhase | undefined { return this.#interceptPhase; } @@ -132,10 +135,6 @@ export class NetworkRequest { return this.#redirectCount; } - get blocked() { - return this.#isBlockedInPhase(this.#interceptPhase); - } - get cdpClient() { return this.#cdpTarget.cdpClient; } @@ -144,11 +143,11 @@ export class NetworkRequest { return Boolean(this.#request.info); } - #isBlockedByInPhase(phase?: Network.InterceptPhase) { + #isBlockedByInPhase(phase: Network.InterceptPhase) { return this.#networkStorage.requestBlockedBy(this, phase); } - #isBlockedInPhase(phase?: Network.InterceptPhase) { + #isBlockedInPhase(phase: Network.InterceptPhase) { return this.#isBlockedByInPhase(phase).size > 0; } @@ -382,10 +381,6 @@ export class NetworkRequest { }); } - dispose() { - // TODO: ??? - } - get #context() { return this.#request.info?.frameId ?? null; } @@ -430,13 +425,15 @@ export class NetworkRequest { isBlocked: false, }; - const blockedBy = this.#isBlockedByInPhase(phase); - interceptProps.isBlocked = blockedBy.size > 0; - if (interceptProps.isBlocked) { - interceptProps.intercepts = [...blockedBy] as [ - Network.Intercept, - ...Network.Intercept[], - ]; + if (phase) { + const blockedBy = this.#isBlockedByInPhase(phase); + interceptProps.isBlocked = blockedBy.size > 0; + if (interceptProps.isBlocked) { + interceptProps.intercepts = [...blockedBy] as [ + Network.Intercept, + ...Network.Intercept[], + ]; + } } return { diff --git a/src/bidiMapper/domains/network/NetworkStorage.ts b/src/bidiMapper/domains/network/NetworkStorage.ts index 4c2937b8c7..1ee0607b3d 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.ts @@ -300,13 +300,10 @@ export class NetworkStorage { } disposeRequestMap(sessionId: string) { - const requests = [...this.#requests.values()].filter((request) => { - return request.cdpClient.sessionId === sessionId; - }); - - for (const request of requests) { - request.dispose(); - this.#requests.delete(request.id); + for (const request of this.#requests.values()) { + if (request.cdpClient.sessionId === sessionId) { + this.#requests.delete(request.id); + } } } @@ -359,10 +356,6 @@ export class NetworkStorage { } deleteRequest(id: Network.Request) { - const request = this.#requests.get(id); - if (request) { - request.dispose(); - this.#requests.delete(id); - } + this.#requests.delete(id); } } From 6cd85b6013e79bc8b092dcf42cb05901020ff1d6 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 20:44:41 +0100 Subject: [PATCH 15/23] chore: fix format --- tests/network/test_continue_response.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/network/test_continue_response.py b/tests/network/test_continue_response.py index f0ec39e06a..c70a91164d 100644 --- a/tests/network/test_continue_response.py +++ b/tests/network/test_continue_response.py @@ -205,7 +205,7 @@ async def test_continue_response_non_blocked_request(websocket, context_id, "statusCode": 401, }, ], - ids=["headers-only", "statusCode-only"]) + ids=["headers-only", "statusCode-only"]) @pytest.mark.asyncio async def test_continue_response_must_specify_both_status_and_headers( websocket, context_id, example_url, continueResponseParams): From 676a7603cb7182e7007932baf3dced306ef9710d Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Fri, 23 Feb 2024 20:53:06 +0100 Subject: [PATCH 16/23] chore: fix --- src/bidiMapper/domains/network/NetworkStorage.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkStorage.ts b/src/bidiMapper/domains/network/NetworkStorage.ts index 1ee0607b3d..986be7e41a 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.ts @@ -272,9 +272,9 @@ export class NetworkStorage { requestBlockedBy( request: NetworkRequest, - phase?: Network.InterceptPhase + phase: Network.InterceptPhase ): Set { - if (request.url === undefined || phase === undefined) { + if (request.url === undefined) { return new Set(); } From 11ee67f66bc75064f9931c26f4a73335d304763a Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Mon, 26 Feb 2024 16:33:54 +0100 Subject: [PATCH 17/23] chore: fixes --- .../domains/network/NetworkProcessor.ts | 13 ++- .../domains/network/NetworkRequest.ts | 29 +++-- .../domains/network/NetworkStorage.spec.ts | 29 +++++ .../domains/network/NetworkStorage.ts | 2 +- .../network/add_intercept/url_patterns.py.ini | 105 +++++++++++++++++- .../network/add_intercept/url_patterns.py.ini | 105 +++++++++++++++++- .../network/add_intercept/url_patterns.py.ini | 105 +++++++++++++++++- 7 files changed, 370 insertions(+), 18 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkProcessor.ts b/src/bidiMapper/domains/network/NetworkProcessor.ts index f1a360fdd7..6dd2d79b96 100644 --- a/src/bidiMapper/domains/network/NetworkProcessor.ts +++ b/src/bidiMapper/domains/network/NetworkProcessor.ts @@ -150,12 +150,12 @@ export class NetworkProcessor { request: networkId, }: Network.FailRequestParameters): Promise { const request = this.#getRequestOrFail(networkId); - if (request.blockedIn === Network.InterceptPhase.AuthRequired) { + if (request.currentInterceptPhase === Network.InterceptPhase.AuthRequired) { throw new InvalidArgumentException( `Request '${networkId}' in 'authRequired' phase cannot be failed` ); } - if (!request.blockedIn) { + if (!request.currentInterceptPhase) { throw new NoSuchRequestException( `No blocked request found for network id '${networkId}'` ); @@ -224,14 +224,17 @@ export class NetworkProcessor { phases: Network.InterceptPhase[] ): NetworkRequest { const request = this.#getRequestOrFail(id); - if (!request.blockedIn) { + if (!request.currentInterceptPhase) { throw new NoSuchRequestException( `No blocked request found for network id '${id}'` ); } - if (request.blockedIn && !phases.includes(request.blockedIn)) { + if ( + request.currentInterceptPhase && + !phases.includes(request.currentInterceptPhase) + ) { throw new InvalidArgumentException( - `Blocked request for network id '${id}' is in '${request.blockedIn}' phase` + `Blocked request for network id '${id}' is in '${request.currentInterceptPhase}' phase` ); } diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index 93cf35e7e3..52dde0bc8a 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -116,9 +116,9 @@ export class NetworkRequest { } /** - * When block returns the phase for it + * When blocked returns the phase for it */ - get blockedIn(): Network.InterceptPhase | undefined { + get currentInterceptPhase(): Network.InterceptPhase | undefined { return this.#interceptPhase; } @@ -143,24 +143,32 @@ export class NetworkRequest { return Boolean(this.#request.info); } - #isBlockedByInPhase(phase: Network.InterceptPhase) { - return this.#networkStorage.requestBlockedBy(this, phase); + #interceptsInPhase(phase: Network.InterceptPhase) { + return this.#networkStorage.getInterceptsForPhase(this, phase); } #isBlockedInPhase(phase: Network.InterceptPhase) { - return this.#isBlockedByInPhase(phase).size > 0; + return this.#interceptsInPhase(phase).size > 0; } handleRedirect(event: Protocol.Network.RequestWillBeSentEvent) { this.#response.hasExtraInfo = event.redirectHasExtraInfo; this.#response.info = event.redirectResponse!; - this.#emitEventsIfReady(true); + this.#emitEventsIfReady({ + wasRedirected: true, + }); } - #emitEventsIfReady(wasRedirected = false) { + #emitEventsIfReady( + options: { + wasRedirected?: boolean; + hasFailed?: boolean; + } = {} + ) { const requestExtraInfoCompleted = // Flush redirects - wasRedirected || + options.wasRedirected || + options.hasFailed || Boolean(this.#request.extraInfo) || // Requests from cache don't have extra info this.#servedFromCache || @@ -247,6 +255,9 @@ export class NetworkRequest { } onLoadingFailedEvent(event: Protocol.Network.LoadingFailedEvent) { + this.#emitEventsIfReady({ + hasFailed: true, + }); this.#emitEvent(() => { return { method: ChromiumBidi.Network.EventNames.FetchError, @@ -426,7 +437,7 @@ export class NetworkRequest { }; if (phase) { - const blockedBy = this.#isBlockedByInPhase(phase); + const blockedBy = this.#interceptsInPhase(phase); interceptProps.isBlocked = blockedBy.size > 0; if (interceptProps.isBlocked) { interceptProps.intercepts = [...blockedBy] as [ diff --git a/src/bidiMapper/domains/network/NetworkStorage.spec.ts b/src/bidiMapper/domains/network/NetworkStorage.spec.ts index 9c4a913923..ddffe68e60 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.spec.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.spec.ts @@ -271,6 +271,16 @@ class MockCdpNetworkEvents { }, }); } + + loadingFailed() { + this.cdpClient.emit('Network.loadingFailed', { + requestId: this.requestId, + timestamp: 279179.745291, + type: 'Fetch', + errorText: 'net::ERR_NAME_NOT_RESOLVED', + canceled: false, + }); + } } class MockCdpClient extends EventEmitter { @@ -444,6 +454,25 @@ describe('NetworkStorage', () => { isBlocked: false, }); }); + + it.only('should work with non blocking interception and fail response', async () => { + const request = new MockCdpNetworkEvents(cdpClient); + await networkStorage.addIntercept({ + urlPatterns: [{type: 'string', pattern: 'http://not.correct.com'}], + phases: [Network.InterceptPhase.BeforeRequestSent], + }); + + request.requestWillBeSent(); + request.requestPaused(); + let event = await getEvent('network.beforeRequestSent'); + expect(event).to.not.exist; + + request.loadingFailed(); + event = await getEvent('network.beforeRequestSent'); + expect(event).to.deep.include({ + isBlocked: false, + }); + }); }); describe('network.responseStarted', () => { diff --git a/src/bidiMapper/domains/network/NetworkStorage.ts b/src/bidiMapper/domains/network/NetworkStorage.ts index 986be7e41a..1124d251f2 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.ts @@ -270,7 +270,7 @@ export class NetworkStorage { } } - requestBlockedBy( + getInterceptsForPhase( request: NetworkRequest, phase: Network.InterceptPhase ): Set { diff --git a/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini b/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini index 9b33871a6f..15f07bae6a 100644 --- a/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini +++ b/wpt-metadata/chromedriver/headful/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini @@ -1,2 +1,105 @@ [url_patterns.py] - expected: TIMEOUT + [test_pattern_patterns_matching[patterns10-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns11-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns12-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns13-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns14-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns15-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns16-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns17-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns18-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns19-https://{wpt_host_upper}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns20-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns21-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns22-https://{wpt_host}\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns23-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns24-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns25-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns26-https://{wpt_host}/?\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns27-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern0-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern1-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern2-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern3-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern4-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern5-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern6-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern7-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern8-https://{wpt_host}/path/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern9-https://{wpt_host}/other/path\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern10-https://{wpt_host}/path/continued\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern11-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern12-https://{wpt_host}/?other\]] + expected: FAIL + + [test_string_patterns_not_matching[https://{wpt_host}/??-https://{wpt_host}/?\]] + expected: FAIL + + [test_string_patterns_matching[https://user:password@{wpt_host}/-https://{wpt_host}/\]] + expected: FAIL + + [test_string_patterns_matching[https://{wpt_host}/#ref-https://{wpt_host}/\]] + expected: FAIL + + [test_string_patterns_matching[https://{wpt_host}/#ref1-https://{wpt_host}/#ref2\]] + expected: FAIL diff --git a/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini b/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini index 9b33871a6f..8168d8d621 100644 --- a/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini +++ b/wpt-metadata/chromedriver/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini @@ -1,2 +1,105 @@ [url_patterns.py] - expected: TIMEOUT + [test_pattern_patterns_matching[patterns10-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns11-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns12-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns13-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns14-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns15-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns16-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns17-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns18-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns19-https://{wpt_host_upper}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns20-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns21-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns22-https://{wpt_host}\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns23-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns24-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns25-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns26-https://{wpt_host}/?\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns27-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern0-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern1-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern2-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern3-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern4-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern5-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern6-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern7-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern8-https://{wpt_host}/path/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern9-https://{wpt_host}/other/path\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern10-https://{wpt_host}/path/continued\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern11-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern12-https://{wpt_host}/?other\]] + expected: FAIL + + [test_string_patterns_matching[https://user:password@{wpt_host}/-https://{wpt_host}/\]] + expected: FAIL + + [test_string_patterns_matching[https://{wpt_host}/#ref-https://{wpt_host}/\]] + expected: FAIL + + [test_string_patterns_matching[https://{wpt_host}/#ref1-https://{wpt_host}/#ref2\]] + expected: FAIL + + [test_string_patterns_not_matching[https://{wpt_host}/??-https://{wpt_host}/?\]] + expected: FAIL diff --git a/wpt-metadata/mapper/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini b/wpt-metadata/mapper/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini index 9b33871a6f..8168d8d621 100644 --- a/wpt-metadata/mapper/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini +++ b/wpt-metadata/mapper/headless/webdriver/tests/bidi/network/add_intercept/url_patterns.py.ini @@ -1,2 +1,105 @@ [url_patterns.py] - expected: TIMEOUT + [test_pattern_patterns_matching[patterns10-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns11-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns12-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns13-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns14-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns15-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns16-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns17-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns18-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns19-https://{wpt_host_upper}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns20-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns21-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns22-https://{wpt_host}\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns23-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns24-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns25-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns26-https://{wpt_host}/?\]] + expected: FAIL + + [test_pattern_patterns_matching[patterns27-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern0-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern1-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern2-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern3-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern4-https://{wpt_host}:1234/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern5-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern6-https://{wpt_host}/path\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern7-https://{wpt_host}/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern8-https://{wpt_host}/path/\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern9-https://{wpt_host}/other/path\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern10-https://{wpt_host}/path/continued\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern11-https://{wpt_host}/?search\]] + expected: FAIL + + [test_pattern_patterns_not_matching[pattern12-https://{wpt_host}/?other\]] + expected: FAIL + + [test_string_patterns_matching[https://user:password@{wpt_host}/-https://{wpt_host}/\]] + expected: FAIL + + [test_string_patterns_matching[https://{wpt_host}/#ref-https://{wpt_host}/\]] + expected: FAIL + + [test_string_patterns_matching[https://{wpt_host}/#ref1-https://{wpt_host}/#ref2\]] + expected: FAIL + + [test_string_patterns_not_matching[https://{wpt_host}/??-https://{wpt_host}/?\]] + expected: FAIL From 807cddf402a87c95dacd2a590c33588502fd0ec9 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Mon, 26 Feb 2024 16:39:19 +0100 Subject: [PATCH 18/23] chore: remove test only --- src/bidiMapper/domains/network/NetworkStorage.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bidiMapper/domains/network/NetworkStorage.spec.ts b/src/bidiMapper/domains/network/NetworkStorage.spec.ts index ddffe68e60..7e231df7ca 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.spec.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.spec.ts @@ -455,7 +455,7 @@ describe('NetworkStorage', () => { }); }); - it.only('should work with non blocking interception and fail response', async () => { + it('should work with non blocking interception and fail response', async () => { const request = new MockCdpNetworkEvents(cdpClient); await networkStorage.addIntercept({ urlPatterns: [{type: 'string', pattern: 'http://not.correct.com'}], From cdf05650a02f3ad8465d2d6cdc674a582322aca0 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Tue, 27 Feb 2024 15:24:03 +0100 Subject: [PATCH 19/23] chore: small change --- src/bidiMapper/domains/network/NetworkRequest.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index 52dde0bc8a..c2f97a14bd 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -286,14 +286,16 @@ export class NetworkRequest { // CDP https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#event-requestPaused if (event.responseStatusCode || event.responseErrorReason) { this.#response.paused = event; - this.#interceptPhase = Network.InterceptPhase.ResponseStarted; - if (!this.#isBlockedInPhase(Network.InterceptPhase.ResponseStarted)) { + if (this.#isBlockedInPhase(Network.InterceptPhase.ResponseStarted)) { + this.#interceptPhase = Network.InterceptPhase.ResponseStarted; + } else { void this.continueResponse(); } } else { this.#request.paused = event; - this.#interceptPhase = Network.InterceptPhase.BeforeRequestSent; - if (!this.#isBlockedInPhase(Network.InterceptPhase.BeforeRequestSent)) { + if (this.#isBlockedInPhase(Network.InterceptPhase.BeforeRequestSent)) { + this.#interceptPhase = Network.InterceptPhase.BeforeRequestSent; + } else { void this.continueRequest(); } } @@ -303,8 +305,9 @@ export class NetworkRequest { onAuthRequired(event: Protocol.Fetch.AuthRequiredEvent) { this.#fetchId = event.requestId; - this.#interceptPhase = Network.InterceptPhase.AuthRequired; - if (!this.#isBlockedInPhase(Network.InterceptPhase.AuthRequired)) { + if (this.#isBlockedInPhase(Network.InterceptPhase.AuthRequired)) { + this.#interceptPhase = Network.InterceptPhase.AuthRequired; + } else { void this.continueWithAuth(); } From 35f19fd6a32d78b2f2a9c4db95d50bde2d12fe26 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Tue, 27 Feb 2024 16:46:58 +0100 Subject: [PATCH 20/23] fix: handle case of Auth before RequestInfo --- .../network/NetworkModuleMocks.spec.ts | 347 ++++++++++++++++++ .../domains/network/NetworkRequest.ts | 13 +- .../domains/network/NetworkStorage.spec.ts | 292 +-------------- 3 files changed, 366 insertions(+), 286 deletions(-) create mode 100644 src/bidiMapper/domains/network/NetworkModuleMocks.spec.ts diff --git a/src/bidiMapper/domains/network/NetworkModuleMocks.spec.ts b/src/bidiMapper/domains/network/NetworkModuleMocks.spec.ts new file mode 100644 index 0000000000..cf65272d84 --- /dev/null +++ b/src/bidiMapper/domains/network/NetworkModuleMocks.spec.ts @@ -0,0 +1,347 @@ +/* + * Copyright 2024 Google LLC. + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +import type {Protocol} from 'devtools-protocol'; + +import { + CloseError, + type CdpClient, + type CdpEvents, +} from '../../../cdp/CdpClient.js'; +import {EventEmitter} from '../../../utils/EventEmitter.js'; + +// TODO: Extend with Redirect +export class MockCdpNetworkEvents { + static defaultFrameId = '099A5216AF03AAFEC988F214B024DF08'; + static defaultUrl = 'http://www.google.com'; + + cdpClient: CdpClient; + + url: string; + requestId: string; + fetchId: string; + private loaderId: string; + private frameId: string; + private type: Protocol.Network.ResourceType; + + constructor( + cdpClient: CdpClient, + { + requestId, + fetchId, + loaderId, + url, + frameId, + type, + }: Partial<{ + requestId: string; + fetchId: string; + loaderId: string; + url: string; + frameId: string; + type: Protocol.Network.ResourceType; + }> = {} + ) { + this.cdpClient = cdpClient; + + this.requestId = requestId ?? '7151.2'; + this.fetchId = fetchId ?? 'interception-job-1.0'; + this.loaderId = loaderId ?? '7760711DEFCFA23132D98ABA6B4E175C'; + this.url = url ?? MockCdpNetworkEvents.defaultUrl; + this.frameId = frameId ?? MockCdpNetworkEvents.defaultFrameId; + this.type = type ?? 'XHR'; + } + + requestWillBeSent() { + this.cdpClient.emit('Network.requestWillBeSent', { + requestId: this.requestId, + loaderId: this.loaderId, + documentURL: this.url, + request: { + url: this.url, + method: 'GET', + headers: { + 'sec-ch-ua': '"Not-A.Brand";v="99", "Chromium";v="124"', + Referer: 'http://localhost:49243/', + 'sec-ch-ua-mobile': '?0', + 'User-Agent': + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36', + 'sec-ch-ua-platform': '"macOS"', + }, + mixedContentType: 'none', + initialPriority: 'High', + referrerPolicy: 'strict-origin-when-cross-origin', + isSameSite: true, + }, + timestamp: 2111.55635, + wallTime: 1637315638.473634, + initiator: { + type: 'script', + stack: { + callFrames: [ + { + functionName: '', + scriptId: '5', + url: '', + lineNumber: 0, + columnNumber: 0, + }, + ], + }, + }, + redirectHasExtraInfo: false, + type: this.type, + frameId: this.frameId, + hasUserGesture: false, + }); + } + + requestWillBeSentExtraInfo() { + this.cdpClient.emit('Network.requestWillBeSentExtraInfo', { + requestId: this.requestId, + associatedCookies: [], + headers: { + Accept: '*/*', + 'Accept-Encoding': 'gzip, deflate, br, zstd', + 'Accept-Language': 'en-US,en;q=0.9', + Connection: 'keep-alive', + Host: 'localhost:49243', + Referer: 'http://localhost:49243/', + 'Sec-Fetch-Dest': 'empty', + 'Sec-Fetch-Mode': 'cors', + 'Sec-Fetch-Site': 'same-origin', + 'User-Agent': + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36', + 'sec-ch-ua': '"Not-A.Brand";v="99", "Chromium";v="124"', + 'sec-ch-ua-mobile': '?0', + 'sec-ch-ua-platform': '"macOS"', + }, + connectTiming: {requestTime: 2111.557593}, + clientSecurityState: { + initiatorIsSecureContext: true, + initiatorIPAddressSpace: 'Local', + privateNetworkRequestPolicy: 'PreflightWarn', + }, + siteHasCookieInOtherPartition: false, + }); + } + + requestServedFromCache() { + this.cdpClient.emit('Network.requestServedFromCache', { + requestId: this.requestId, + }); + } + + responseReceivedExtraInfo() { + this.cdpClient.emit('Network.responseReceivedExtraInfo', { + requestId: this.requestId, + blockedCookies: [], + headers: { + 'Cache-Control': 'no-cache, no-store', + 'Content-Type': 'text/html; charset=utf-8', + Date: 'Fri, 19 Nov 2021 09:53:58 GMT', + Connection: 'keep-alive', + 'Keep-Alive': 'timeout=5', + 'Content-Length': '0', + }, + resourceIPAddressSpace: 'Local', + statusCode: 200, + headersText: + 'HTTP/1.1 200 OK\r\nCache-Control: no-cache, no-store\r\nContent-Type: text/html; charset=utf-8\r\nDate: Fri, 19 Nov 2021 09:53:58 GMT\r\nConnection: keep-alive\r\nKeep-Alive: timeout=5\r\nContent-Length: 0\r\n\r\n', + }); + } + + responseReceived(hasExtraInfo = false) { + this.cdpClient.emit('Network.responseReceived', { + requestId: this.requestId, + loaderId: this.loaderId, + timestamp: 2111.563565, + type: this.type, + response: { + url: this.url, + status: 200, + statusText: 'OK', + headers: { + 'Cache-Control': 'no-cache, no-store', + 'Content-Type': 'text/html; charset=utf-8', + Date: 'Fri, 19 Nov 2021 09:53:58 GMT', + Connection: 'keep-alive', + 'Keep-Alive': 'timeout=5', + 'Content-Length': '0', + }, + // TODO: set to a correct one + charset: '', + mimeType: 'text/html', + connectionReused: true, + connectionId: 322, + remoteIPAddress: '[::1]', + remotePort: 8907, + fromDiskCache: false, + fromServiceWorker: false, + fromPrefetchCache: false, + encodedDataLength: 197, + timing: { + receiveHeadersStart: 0, + requestTime: 2111.561759, + proxyStart: -1, + proxyEnd: -1, + dnsStart: -1, + dnsEnd: -1, + connectStart: -1, + connectEnd: -1, + sslStart: -1, + sslEnd: -1, + workerStart: -1, + workerReady: -1, + workerFetchStart: -1, + workerRespondWithSettled: -1, + sendStart: 0.148, + sendEnd: 0.19, + pushStart: 0, + pushEnd: 0, + receiveHeadersEnd: 0.925, + }, + responseTime: 1.637315638479928e12, + protocol: 'http/1.1', + securityState: 'secure', + }, + hasExtraInfo, + frameId: this.frameId, + }); + } + + requestPaused() { + this.cdpClient.emit('Fetch.requestPaused', { + requestId: this.fetchId, + request: { + url: this.url, + method: 'GET', + headers: { + Accept: '*/*', + Referer: 'http://localhost:49243/', + 'User-Agent': + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36', + 'sec-ch-ua': '"Not-A.Brand";v="99", "Chromium";v="124"', + 'sec-ch-ua-mobile': '?0', + 'sec-ch-ua-platform': '"macOS"', + }, + initialPriority: 'High', + referrerPolicy: 'strict-origin-when-cross-origin', + }, + frameId: this.frameId, + resourceType: this.type, + networkId: this.requestId, + }); + } + + responsePaused() { + this.cdpClient.emit('Fetch.requestPaused', { + requestId: this.fetchId, + request: { + url: this.url, + method: 'GET', + headers: {}, + initialPriority: 'VeryHigh', + referrerPolicy: 'strict-origin-when-cross-origin', + }, + // TODO: fill for response correctly + responseStatusCode: 200, + responseStatusText: '', + responseHeaders: [], + frameId: this.frameId, + resourceType: this.type, + networkId: this.requestId, + }); + } + + authRequired() { + this.cdpClient.emit('Fetch.authRequired', { + requestId: this.fetchId, + request: { + url: this.url, + method: 'GET', + headers: { + Accept: '*/*', + Referer: 'http://localhost:49243/', + 'User-Agent': + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36', + 'sec-ch-ua': '"Not-A.Brand";v="99", "Chromium";v="124"', + 'sec-ch-ua-mobile': '?0', + 'sec-ch-ua-platform': '"macOS"', + }, + initialPriority: 'High', + referrerPolicy: 'strict-origin-when-cross-origin', + }, + frameId: this.frameId, + resourceType: this.type, + authChallenge: { + source: 'Server', + origin: new URL(this.url).origin, + scheme: 'basic', + realm: 'Access to staging site', + }, + }); + } + + loadingFailed() { + this.cdpClient.emit('Network.loadingFailed', { + requestId: this.requestId, + timestamp: 279179.745291, + type: 'Fetch', + errorText: 'net::ERR_NAME_NOT_RESOLVED', + canceled: false, + }); + } +} + +export class MockCdpClient extends EventEmitter { + #logger: (...args: any[]) => void; + + sessionId = '23E8E97ED43449740710991CD32AEFA3'; + + constructor(logger: (...args: any[]) => void) { + super(); + this.#logger = logger; + } + + sendCommand(...args: any[]) { + this.#logger(...args); + + return new Promise((resolve) => { + setTimeout(resolve, 100); + }); + } + + isCloseError(error: unknown) { + return error instanceof CloseError; + } +} + +export class MockCdpTarget { + cdpClient: CdpClient; + #logger: (...args: any[]) => void; + + constructor(logger: (...args: any[]) => void) { + this.#logger = logger; + this.cdpClient = new MockCdpClient(logger) as CdpClient; + } + + enableFetchIfNeeded() { + this.#logger('Fetch.enabled called'); + return Promise.resolve(); + } +} diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index c2f97a14bd..22c8e967cf 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -69,6 +69,7 @@ export class NetworkRequest { info?: Protocol.Network.RequestWillBeSentEvent; extraInfo?: Protocol.Network.RequestWillBeSentExtraInfoEvent; paused?: Protocol.Fetch.RequestPausedEvent; + auth?: Protocol.Fetch.AuthRequiredEvent; } = {}; #response: { @@ -127,7 +128,8 @@ export class NetworkRequest { this.#response.info?.url ?? this.#request.info?.request.url ?? this.#response.paused?.request.url ?? - this.#request.paused?.request.url + this.#request.paused?.request.url ?? + this.#request.auth?.request.url ); } @@ -305,6 +307,7 @@ export class NetworkRequest { onAuthRequired(event: Protocol.Fetch.AuthRequiredEvent) { this.#fetchId = event.requestId; + this.#request.auth = event; if (this.#isBlockedInPhase(Network.InterceptPhase.AuthRequired)) { this.#interceptPhase = Network.InterceptPhase.AuthRequired; } else { @@ -396,7 +399,13 @@ export class NetworkRequest { } get #context() { - return this.#request.info?.frameId ?? null; + return ( + this.#response.paused?.frameId ?? + this.#request.info?.frameId ?? + this.#request.paused?.frameId ?? + this.#request.auth?.frameId ?? + null + ); } /** Returns the HTTP status code associated with this request if any. */ diff --git a/src/bidiMapper/domains/network/NetworkStorage.spec.ts b/src/bidiMapper/domains/network/NetworkStorage.spec.ts index 7e231df7ca..2dc3184d88 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.spec.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.spec.ts @@ -16,11 +16,9 @@ * */ import {expect} from 'chai'; -import type {Protocol} from 'devtools-protocol'; -import type {CdpClient, CdpEvents} from '../../../cdp/CdpClient.js'; +import type {CdpClient} from '../../../cdp/CdpClient.js'; import {ChromiumBidi, Network} from '../../../protocol/protocol.js'; -import {EventEmitter} from '../../../utils/EventEmitter.js'; import {ProcessingQueue} from '../../../utils/ProcessingQueue.js'; import type {OutgoingMessage} from '../../OutgoingMessage.js'; import type {BrowsingContextImpl} from '../context/BrowsingContextImpl.js'; @@ -28,6 +26,10 @@ import {BrowsingContextStorage} from '../context/BrowsingContextStorage.js'; import type {CdpTarget} from '../context/CdpTarget.js'; import {EventManager, EventManagerEvents} from '../session/EventManager.js'; +import { + MockCdpNetworkEvents, + MockCdpTarget, +} from './NetworkModuleMocks.spec.js'; import {NetworkStorage} from './NetworkStorage.js'; function logger(...args: any[]) { @@ -38,286 +40,6 @@ function logger(...args: any[]) { } } -// TODO: Extend with Redirect -class MockCdpNetworkEvents { - static defaultFrameId = '099A5216AF03AAFEC988F214B024DF08'; - static defaultUrl = 'http://www.google.com'; - - cdpClient: CdpClient; - - url: string; - requestId: string; - fetchId: string; - private loaderId: string; - private frameId: string; - private type: Protocol.Network.ResourceType; - - constructor( - cdpClient: CdpClient, - { - requestId, - fetchId, - loaderId, - url, - frameId, - type, - }: Partial<{ - requestId: string; - fetchId: string; - loaderId: string; - url: string; - frameId: string; - type: Protocol.Network.ResourceType; - }> = {} - ) { - this.cdpClient = cdpClient; - - this.requestId = requestId ?? '7760711DEFCFA23132D98ABA6B4E175C'; - this.fetchId = fetchId ?? 'interception-job-1.0'; - this.loaderId = loaderId ?? '7760711DEFCFA23132D98ABA6B4E175C'; - this.url = url ?? MockCdpNetworkEvents.defaultUrl; - this.frameId = frameId ?? MockCdpNetworkEvents.defaultFrameId; - this.type = type ?? 'Document'; - } - - requestWillBeSent() { - this.cdpClient.emit('Network.requestWillBeSent', { - requestId: this.requestId, - loaderId: this.loaderId, - documentURL: this.url, - request: { - url: this.url, - method: 'GET', - headers: { - 'Upgrade-Insecure-Requests': '1', - 'User-Agent': - 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/97.0.4691.0 Safari/537.36', - }, - mixedContentType: 'none', - initialPriority: 'VeryHigh', - referrerPolicy: 'strict-origin-when-cross-origin', - isSameSite: true, - }, - timestamp: 2111.55635, - wallTime: 1637315638.473634, - initiator: {type: 'other'}, - redirectHasExtraInfo: false, - type: this.type, - frameId: this.frameId, - hasUserGesture: false, - }); - } - - requestWillBeSentExtraInfo() { - this.cdpClient.emit('Network.requestWillBeSentExtraInfo', { - requestId: this.requestId, - associatedCookies: [], - headers: { - Host: 'localhost:8907', - Connection: 'keep-alive', - 'Upgrade-Insecure-Requests': '1', - 'User-Agent': - 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/97.0.4691.0 Safari/537.36', - Accept: - 'text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9', - 'Sec-Fetch-Site': 'none', - 'Sec-Fetch-Mode': 'navigate', - 'Sec-Fetch-User': '?1', - 'Sec-Fetch-Dest': 'document', - 'Accept-Encoding': 'gzip, deflate, br', - }, - connectTiming: {requestTime: 2111.557593}, - }); - } - - requestServedFromCache() { - this.cdpClient.emit('Network.requestServedFromCache', { - requestId: this.requestId, - }); - } - - responseReceivedExtraInfo() { - this.cdpClient.emit('Network.responseReceivedExtraInfo', { - requestId: this.requestId, - blockedCookies: [], - headers: { - 'Cache-Control': 'no-cache, no-store', - 'Content-Type': 'text/html; charset=utf-8', - Date: 'Fri, 19 Nov 2021 09:53:58 GMT', - Connection: 'keep-alive', - 'Keep-Alive': 'timeout=5', - 'Content-Length': '0', - }, - resourceIPAddressSpace: 'Local', - statusCode: 200, - headersText: - 'HTTP/1.1 200 OK\r\nCache-Control: no-cache, no-store\r\nContent-Type: text/html; charset=utf-8\r\nDate: Fri, 19 Nov 2021 09:53:58 GMT\r\nConnection: keep-alive\r\nKeep-Alive: timeout=5\r\nContent-Length: 0\r\n\r\n', - }); - } - - responseReceived(hasExtraInfo = false) { - this.cdpClient.emit('Network.responseReceived', { - requestId: this.requestId, - loaderId: this.loaderId, - timestamp: 2111.563565, - type: this.type, - response: { - url: this.url, - status: 200, - statusText: 'OK', - headers: { - 'Cache-Control': 'no-cache, no-store', - 'Content-Type': 'text/html; charset=utf-8', - Date: 'Fri, 19 Nov 2021 09:53:58 GMT', - Connection: 'keep-alive', - 'Keep-Alive': 'timeout=5', - 'Content-Length': '0', - }, - // TODO: set to a correct one - charset: '', - mimeType: 'text/html', - connectionReused: true, - connectionId: 322, - remoteIPAddress: '[::1]', - remotePort: 8907, - fromDiskCache: false, - fromServiceWorker: false, - fromPrefetchCache: false, - encodedDataLength: 197, - timing: { - receiveHeadersStart: 0, - requestTime: 2111.561759, - proxyStart: -1, - proxyEnd: -1, - dnsStart: -1, - dnsEnd: -1, - connectStart: -1, - connectEnd: -1, - sslStart: -1, - sslEnd: -1, - workerStart: -1, - workerReady: -1, - workerFetchStart: -1, - workerRespondWithSettled: -1, - sendStart: 0.148, - sendEnd: 0.19, - pushStart: 0, - pushEnd: 0, - receiveHeadersEnd: 0.925, - }, - responseTime: 1.637315638479928e12, - protocol: 'http/1.1', - securityState: 'secure', - }, - hasExtraInfo, - frameId: this.frameId, - }); - } - - requestPaused() { - this.cdpClient.emit('Fetch.requestPaused', { - requestId: this.fetchId, - request: { - url: this.url, - method: 'GET', - headers: {}, - initialPriority: 'VeryHigh', - referrerPolicy: 'strict-origin-when-cross-origin', - }, - frameId: this.frameId, - resourceType: this.type, - networkId: this.requestId, - }); - } - - responsePaused() { - this.cdpClient.emit('Fetch.requestPaused', { - requestId: this.fetchId, - request: { - url: this.url, - method: 'GET', - headers: {}, - initialPriority: 'VeryHigh', - referrerPolicy: 'strict-origin-when-cross-origin', - }, - // TODO: fill for response correctly - responseStatusCode: 200, - responseStatusText: '', - responseHeaders: [], - frameId: this.frameId, - resourceType: this.type, - networkId: this.requestId, - }); - } - - authRequired() { - this.cdpClient.emit('Fetch.authRequired', { - requestId: this.fetchId, - request: { - url: this.url, - method: 'GET', - headers: {}, - initialPriority: 'VeryHigh', - referrerPolicy: 'strict-origin-when-cross-origin', - }, - frameId: this.frameId, - resourceType: this.type, - // TODO: fill for auth challenge correctly - authChallenge: { - source: 'Server', - origin: new URL(this.url).origin, - scheme: 'Basic', - realm: 'Access to staging site', - }, - }); - } - - loadingFailed() { - this.cdpClient.emit('Network.loadingFailed', { - requestId: this.requestId, - timestamp: 279179.745291, - type: 'Fetch', - errorText: 'net::ERR_NAME_NOT_RESOLVED', - canceled: false, - }); - } -} - -class MockCdpClient extends EventEmitter { - #logger: (...args: any[]) => void; - - sessionId = '23E8E97ED43449740710991CD32AEFA3'; - - constructor(logger: (...args: any[]) => void) { - super(); - this.#logger = logger; - } - - sendCommand(...args: any[]) { - this.#logger(...args); - return Promise.resolve(); - } - - isCloseError() { - return false; - } -} - -class MockCdpTarget { - cdpClient: CdpClient; - #logger: (...args: any[]) => void; - - constructor(logger: (...args: any[]) => void) { - this.#logger = logger; - this.cdpClient = new MockCdpClient(logger) as CdpClient; - } - - enableFetchIfNeeded() { - this.#logger('Fetch.enabled called'); - return Promise.resolve(); - } -} - describe('NetworkStorage', () => { let processedEvents = new Map< ChromiumBidi.Event['method'], @@ -358,7 +80,9 @@ describe('NetworkStorage', () => { // Subscribe to the `network` module globally eventManager.subscriptionManager.subscribe( ChromiumBidi.BiDiModule.Network, - null, + // Verify that the Request send the message + // To the correct context + MockCdpNetworkEvents.defaultFrameId, null ); eventManager.on(EventManagerEvents.Event, ({message, event}) => { From 882dba273a9565d50a697e7c41d436b1c9eddb37 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Tue, 27 Feb 2024 18:51:53 +0100 Subject: [PATCH 21/23] chore: fix issue with not setting ids correctly --- .../domains/network/NetworkRequest.ts | 32 ++++++++++++------- .../domains/network/NetworkStorage.spec.ts | 12 +++++-- .../domains/network/NetworkStorage.ts | 2 +- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index 22c8e967cf..f71dc6bde9 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -42,7 +42,7 @@ import { /** Abstracts one individual network request. */ export class NetworkRequest { - static #unknown = 'UNKNOWN'; + static unknownParameter = 'UNKNOWN'; /** * Each network request has an associated request id, which is a string @@ -123,13 +123,24 @@ export class NetworkRequest { return this.#interceptPhase; } - get url(): string | undefined { + get url(): string { return ( this.#response.info?.url ?? - this.#request.info?.request.url ?? this.#response.paused?.request.url ?? + this.#request.auth?.request.url ?? + this.#request.info?.request.url ?? this.#request.paused?.request.url ?? - this.#request.auth?.request.url + NetworkRequest.unknownParameter + ); + } + + get method(): string { + return ( + this.#request.info?.request.method ?? + this.#request.paused?.request.method ?? + this.#request.auth?.request.method ?? + this.#response.paused?.request.method ?? + NetworkRequest.unknownParameter ); } @@ -495,9 +506,9 @@ export class NetworkRequest { ); return { - request: this.#request.info?.requestId ?? NetworkRequest.#unknown, - url: this.#request.info?.request.url ?? NetworkRequest.#unknown, - method: this.#request.info?.request.method ?? NetworkRequest.#unknown, + request: this.#id, + url: this.url, + method: this.method, headers, cookies, headersSize: computeHeadersSize(headers), @@ -567,10 +578,7 @@ export class NetworkRequest { params: { ...this.#getBaseEventParams(Network.InterceptPhase.ResponseStarted), response: { - url: - this.#response.info?.url ?? - this.#response.paused?.request.url ?? - NetworkRequest.#unknown, + url: this.url, protocol: this.#response.info?.protocol ?? '', status: this.statusCode, statusText: @@ -616,7 +624,7 @@ export class NetworkRequest { params: { ...this.#getBaseEventParams(), response: { - url: this.#response.info.url ?? NetworkRequest.#unknown, + url: this.url, protocol: this.#response.info.protocol ?? '', status: this.statusCode, statusText: this.#response.info.statusText, diff --git a/src/bidiMapper/domains/network/NetworkStorage.spec.ts b/src/bidiMapper/domains/network/NetworkStorage.spec.ts index 2dc3184d88..ebba3d0b22 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.spec.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.spec.ts @@ -288,8 +288,10 @@ describe('NetworkStorage', () => { request.requestWillBeSentExtraInfo(); request.authRequired(); const event = await getEvent('network.authRequired'); - expect(event).to.exist; - request.requestWillBeSent(); + expect(event).to.deep.nested.include({ + 'request.request': request.requestId, + 'request.method': 'GET', + }); }); it('should work with only authRequired', async () => { @@ -297,7 +299,11 @@ describe('NetworkStorage', () => { request.authRequired(); const event = await getEvent('network.authRequired'); - expect(event).to.exist; + event; + expect(event).to.deep.nested.include({ + 'request.request': request.fetchId, + 'request.method': 'GET', + }); }); }); }); diff --git a/src/bidiMapper/domains/network/NetworkStorage.ts b/src/bidiMapper/domains/network/NetworkStorage.ts index 1124d251f2..bdbe06aa03 100644 --- a/src/bidiMapper/domains/network/NetworkStorage.ts +++ b/src/bidiMapper/domains/network/NetworkStorage.ts @@ -274,7 +274,7 @@ export class NetworkStorage { request: NetworkRequest, phase: Network.InterceptPhase ): Set { - if (request.url === undefined) { + if (request.url === NetworkRequest.unknownParameter) { return new Set(); } From ec0c06ddc10c95f1af1d4cd008ed3fa72f246871 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Tue, 27 Feb 2024 20:03:44 +0100 Subject: [PATCH 22/23] test: deflake test to use fetch instead of navigation --- tests/network/test_fail_request.py | 85 +++++------------------------- 1 file changed, 12 insertions(+), 73 deletions(-) diff --git a/tests/network/test_fail_request.py b/tests/network/test_fail_request.py index a4f8886148..bb05b68301 100644 --- a/tests/network/test_fail_request.py +++ b/tests/network/test_fail_request.py @@ -283,66 +283,16 @@ async def test_fail_request_completes(websocket, context_id, example_url): @pytest.mark.asyncio async def test_fail_request_completes_new_request_still_blocks( - websocket, context_id, example_url): - await subscribe(websocket, ["network.beforeRequestSent"], [context_id]) - - result = await execute_command( - websocket, { - "method": "network.addIntercept", - "params": { - "phases": ["beforeRequestSent"], - "urlPatterns": [{ - "type": "string", - "pattern": example_url, - }, ], - }, - }) - - assert result == { - "intercept": ANY_UUID, - } - intercept_id = result["intercept"] + websocket, context_id, example_url, base_url): - await send_JSON_command( - websocket, { - "method": "browsingContext.navigate", - "params": { - "url": example_url, - "context": context_id, - "wait": "complete", - } - }) + await goto_url(websocket, context_id, base_url) - event_response1 = await wait_for_event(websocket, - "network.beforeRequestSent") - assert event_response1 == { - "method": "network.beforeRequestSent", - "params": { - "context": context_id, - "initiator": { - "type": "other", - }, - "intercepts": [intercept_id], - "isBlocked": True, - "navigation": ANY_STR, - "redirectCount": 0, - "request": { - "request": ANY_STR, - "url": example_url, - "method": "GET", - "headers": ANY_LIST, - "cookies": [], - "headersSize": ANY_NUMBER, - "bodySize": 0, - "timings": ANY_DICT, - }, - "timestamp": ANY_TIMESTAMP, - }, - "type": "event", - } - network_id_1 = event_response1["params"]["request"]["request"] + await subscribe(websocket, ["network.beforeRequestSent", "network.fetchError"], [context_id]) - await subscribe(websocket, ["network.fetchError"]) + network_id_1 = await create_blocked_request(websocket, + context_id, + url=example_url, + phase="beforeRequestSent") result = await execute_command(websocket, { "method": "network.failRequest", @@ -361,7 +311,7 @@ async def test_fail_request_completes_new_request_still_blocks( "context": context_id, "errorText": "net::ERR_FAILED", "isBlocked": False, - "navigation": ANY_STR, + "navigation": None, "redirectCount": 0, "request": AnyExtending( { @@ -375,16 +325,7 @@ async def test_fail_request_completes_new_request_still_blocks( "type": "event", } - # Perform the same navigation again. - await send_JSON_command( - websocket, { - "method": "browsingContext.navigate", - "params": { - "url": example_url, - "context": context_id, - "wait": "complete", - } - }) + await create_request_via_fetch(websocket, context_id, example_url) event_response2 = await wait_for_event(websocket, "network.beforeRequestSent") @@ -393,11 +334,11 @@ async def test_fail_request_completes_new_request_still_blocks( "params": { "context": context_id, "initiator": { - "type": "other", + "type": "script", }, - "intercepts": [intercept_id], + "intercepts": ANY_LIST, "isBlocked": True, - "navigation": ANY_STR, + "navigation": None, "redirectCount": 0, "request": { "request": ANY_STR, @@ -415,8 +356,6 @@ async def test_fail_request_completes_new_request_still_blocks( } network_id_2 = event_response2["params"]["request"]["request"] - assert event_response1 != event_response2 - # The second request should have a different ID. assert network_id_1 != network_id_2 From ec4085ec5a84c583f17e12e314e7fc9dcc2b6f7c Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov Date: Tue, 27 Feb 2024 20:17:53 +0100 Subject: [PATCH 23/23] chore: deflake more tests --- tests/browsing_context/test_reload.py | 25 ++++++------------------- tests/network/test_fail_request.py | 4 +++- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/tests/browsing_context/test_reload.py b/tests/browsing_context/test_reload.py index 5875297c50..5e110cc9fe 100644 --- a/tests/browsing_context/test_reload.py +++ b/tests/browsing_context/test_reload.py @@ -15,7 +15,8 @@ import pytest from anys import ANY_DICT, ANY_STR from test_helpers import (ANY_TIMESTAMP, AnyExtending, goto_url, - read_JSON_message, send_JSON_command, subscribe) + read_JSON_message, send_JSON_command, subscribe, + wait_for_event) @pytest.mark.asyncio @@ -200,9 +201,9 @@ async def test_browsingContext_reload_ignoreCache(websocket, context_id, "network.responseCompleted", ]) - initial_navigation = await goto_url(websocket, context_id, cacheable_url) + await goto_url(websocket, context_id, cacheable_url) - id = await send_JSON_command( + await send_JSON_command( websocket, { "method": "browsingContext.reload", "params": { @@ -212,7 +213,8 @@ async def test_browsingContext_reload_ignoreCache(websocket, context_id, } }) - response_completed_event = await read_JSON_message(websocket) + response_completed_event = await wait_for_event( + websocket, "network.responseCompleted") assert response_completed_event == { 'type': 'event', "method": "network.responseCompleted", @@ -226,18 +228,3 @@ async def test_browsingContext_reload_ignoreCache(websocket, context_id, "timestamp": ANY_TIMESTAMP, }, } - - # Assert command done. - response = await read_JSON_message(websocket) - assert response == { - "id": id, - "type": "success", - "result": { - "navigation": ANY_STR, - "url": cacheable_url, - }, - } - - assert response["result"]["navigation"] == response_completed_event[ - "params"]["navigation"] - assert initial_navigation["navigation"] != response["result"]["navigation"] diff --git a/tests/network/test_fail_request.py b/tests/network/test_fail_request.py index bb05b68301..c79540eb8f 100644 --- a/tests/network/test_fail_request.py +++ b/tests/network/test_fail_request.py @@ -287,7 +287,9 @@ async def test_fail_request_completes_new_request_still_blocks( await goto_url(websocket, context_id, base_url) - await subscribe(websocket, ["network.beforeRequestSent", "network.fetchError"], [context_id]) + await subscribe(websocket, + ["network.beforeRequestSent", "network.fetchError"], + [context_id]) network_id_1 = await create_blocked_request(websocket, context_id,