From 9aa5b4797bc4c944c277c5000618fbdd971cd0f9 Mon Sep 17 00:00:00 2001 From: Thiago Perrotta Date: Mon, 9 Oct 2023 15:36:48 +0200 Subject: [PATCH] feat: unblock event queue when network events are blocked Furthermore, update some E2E tests so that they do not rely on CDP events. Bug #644 Co-authored-by: Maksim Sadym --- .../domains/network/NetworkRequest.ts | 35 ++- tests/network/test_add_intercept.py | 99 +++++- tests/network/test_fail_request.py | 288 +++++++++++------- tests/network/test_remove_intercept.py | 142 ++++++++- 4 files changed, 434 insertions(+), 130 deletions(-) diff --git a/src/bidiMapper/domains/network/NetworkRequest.ts b/src/bidiMapper/domains/network/NetworkRequest.ts index 47bdd282a9..3b7c80ebb6 100644 --- a/src/bidiMapper/domains/network/NetworkRequest.ts +++ b/src/bidiMapper/domains/network/NetworkRequest.ts @@ -53,8 +53,12 @@ export class NetworkRequest { */ readonly requestId: Network.Request; - /** Indicates whether the request is blocked by a network intercept. */ - #isBlocked = false; + // TODO: Handle auth required? + /** + * Indicates the network intercept phase, if the request is currently blocked. + * Undefined necessarily implies that the request is not blocked. + */ + #interceptPhase: Network.InterceptPhase | undefined = undefined; #servedFromCache = false; @@ -126,7 +130,8 @@ export class NetworkRequest { this.#servedFromCache || // Sometimes there is no extra info and the response // is the only place we can find out - Boolean(this.#response.info && !this.#response.info.hasExtraInfo); + Boolean(this.#response.info && !this.#response.info.hasExtraInfo) || + this.#interceptPhase === Network.InterceptPhase.BeforeRequestSent; if (this.#request.info && requestExtraInfoCompleted) { this.#beforeRequestSentDeferred.resolve({ @@ -140,7 +145,8 @@ export class NetworkRequest { // Response from cache don't have extra info this.#servedFromCache || // Don't expect extra info if the flag is false - Boolean(this.#response.info && !this.#response.info.hasExtraInfo); + Boolean(this.#response.info && !this.#response.info.hasExtraInfo) || + this.#interceptPhase === Network.InterceptPhase.ResponseStarted; if (this.#response.info && responseExtraInfoCompleted) { this.#responseCompletedDeferred.resolve({ @@ -264,31 +270,32 @@ export class NetworkRequest { }, }); - this.#isBlocked = true; + this.#interceptPhase = phase; + this.#emitEventsIfReady(); } /** @see https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#method-failRequest */ async failRequest( - fetchRequestId: Protocol.Fetch.RequestId, + networkId: Network.Request, errorReason: Protocol.Network.ErrorReason ) { await this.#cdpTarget.cdpClient.sendCommand('Fetch.failRequest', { - requestId: fetchRequestId, + requestId: networkId, errorReason, }); - this.#isBlocked = false; + this.#interceptPhase = undefined; } /** @see https://chromedevtools.github.io/devtools-protocol/tot/Fetch/#method-continueRequest */ async continueRequest( - fetchRequestId: Protocol.Fetch.RequestId, + networkId: Protocol.Fetch.RequestId, url?: string, method?: string ) { // TODO: Expand. await this.#cdpTarget.cdpClient.sendCommand('Fetch.continueRequest', { - requestId: fetchRequestId, + requestId: networkId, url, method, // TODO: Set? @@ -297,7 +304,7 @@ export class NetworkRequest { // interceptResponse:, }); - this.#isBlocked = false; + this.#interceptPhase = undefined; } dispose() { @@ -313,9 +320,9 @@ export class NetworkRequest { return this.#request.info?.frameId ?? null; } - #getBaseEventParams(): Network.BaseParameters { + #getBaseEventParams(phase?: Network.InterceptPhase): Network.BaseParameters { return { - isBlocked: this.#isBlocked, + isBlocked: phase !== undefined && phase === this.#interceptPhase, context: this.#context, navigation: this.#getNavigationId(), redirectCount: this.#redirectCount, @@ -407,7 +414,7 @@ export class NetworkRequest { return { method: ChromiumBidi.Network.EventNames.BeforeRequestSent, params: { - ...this.#getBaseEventParams(), + ...this.#getBaseEventParams(Network.InterceptPhase.BeforeRequestSent), initiator: { type: NetworkRequest.#getInitiatorType( this.#request.info.initiator.type diff --git a/tests/network/test_add_intercept.py b/tests/network/test_add_intercept.py index b29a8c9a48..10d3524198 100644 --- a/tests/network/test_add_intercept.py +++ b/tests/network/test_add_intercept.py @@ -13,9 +13,10 @@ # See the License for the specific language governing permissions and # limitations under the License. import pytest -from anys import ANY_DICT, ANY_STR -from test_helpers import (ANY_UUID, AnyExtending, execute_command, - send_JSON_command, subscribe, wait_for_event) +from anys import ANY_DICT, ANY_LIST, ANY_STR +from test_helpers import (ANY_TIMESTAMP, ANY_UUID, AnyExtending, + execute_command, send_JSON_command, subscribe, + wait_for_event) @pytest.mark.asyncio @@ -289,8 +290,8 @@ async def test_add_intercept_type_pattern_port_empty_invalid(websocket): "pattern", "string and pattern", ]) -async def test_add_intercept_blocks(websocket, context_id, url_patterns, - example_url): +async def test_add_intercept_blocks_use_cdp_events(websocket, context_id, + url_patterns, example_url): await subscribe(websocket, ["cdp.Fetch.requestPaused"]) result = await execute_command( @@ -334,3 +335,91 @@ async def test_add_intercept_blocks(websocket, context_id, url_patterns, }, "type": "event", } + + +@pytest.mark.asyncio +@pytest.mark.parametrize("url_patterns", [ + [ + { + "type": "string", + "pattern": "https://www.example.com/", + }, + ], + [ + { + "type": "pattern", + "protocol": "https", + "hostname": "www.example.com", + "pathname": "/", + }, + ], + [ + { + "type": "string", + "pattern": "https://www.example.com/", + }, + { + "type": "pattern", + "protocol": "https", + "hostname": "www.example.com", + "pathname": "/", + }, + ], +], + ids=[ + "string", + "pattern", + "string and pattern", + ]) +async def test_add_intercept_blocks_use_bidi_events(websocket, context_id, + url_patterns, example_url): + await subscribe(websocket, ["network.beforeRequestSent"]) + + result = await execute_command( + websocket, { + "method": "network.addIntercept", + "params": { + "phases": ["beforeRequestSent"], + "urlPatterns": url_patterns, + }, + }) + + assert result == { + "intercept": ANY_UUID, + } + + await send_JSON_command( + websocket, { + "method": "browsingContext.navigate", + "params": { + "url": example_url, + "context": context_id, + } + }) + + event_response = await wait_for_event(websocket, + "network.beforeRequestSent") + assert event_response == { + "method": "network.beforeRequestSent", + "params": { + "isBlocked": True, + "initiator": { + "type": "other", + }, + "context": context_id, + "navigation": ANY_STR, + "redirectCount": 0, + "request": { + "request": ANY_STR, + "url": example_url, + "method": "GET", + "headers": ANY_LIST, + "cookies": [], + "headersSize": -1, + "bodySize": 0, + "timings": ANY_DICT + }, + "timestamp": ANY_TIMESTAMP, + }, + "type": "event", + } diff --git a/tests/network/test_fail_request.py b/tests/network/test_fail_request.py index 4bc5220f79..5754aa2ae6 100644 --- a/tests/network/test_fail_request.py +++ b/tests/network/test_fail_request.py @@ -14,8 +14,9 @@ # limitations under the License. import pytest from anys import ANY_DICT, ANY_LIST, ANY_NUMBER, ANY_STR -from test_helpers import (ANY_UUID, AnyExtending, execute_command, - send_JSON_command, subscribe, wait_for_event) +from test_helpers import (ANY_TIMESTAMP, ANY_UUID, AnyExtending, + execute_command, send_JSON_command, subscribe, + wait_for_event) @pytest.mark.asyncio @@ -85,7 +86,7 @@ async def test_fail_request_non_blocked_request(websocket, context_id, @pytest.mark.asyncio async def test_fail_request_twice(websocket, context_id, example_url): - await subscribe(websocket, ["cdp.Fetch.requestPaused"]) + await subscribe(websocket, ["network.beforeRequestSent"], [context_id]) result = await execute_command( websocket, { @@ -112,26 +113,33 @@ async def test_fail_request_twice(websocket, context_id, example_url): } }) - event_response = await wait_for_event(websocket, "cdp.Fetch.requestPaused") + event_response = await wait_for_event(websocket, + "network.beforeRequestSent") assert event_response == { - "method": "cdp.Fetch.requestPaused", + "method": "network.beforeRequestSent", "params": { - "event": "Fetch.requestPaused", - "params": { - "frameId": context_id, - "networkId": ANY_STR, - "request": AnyExtending({ - "headers": ANY_DICT, - "url": example_url, - }), - "requestId": ANY_STR, - "resourceType": "Document", + "context": context_id, + "initiator": { + "type": "other", }, - "session": ANY_STR, + "isBlocked": True, + "navigation": ANY_STR, + "redirectCount": 0, + "request": { + "request": ANY_STR, + "url": example_url, + "method": "GET", + "headers": ANY_LIST, + "cookies": [], + "headersSize": -1, + "bodySize": 0, + "timings": ANY_DICT, + }, + "timestamp": ANY_TIMESTAMP, }, "type": "event", } - network_id = event_response["params"]["params"]["networkId"] + network_id = event_response["params"]["request"]["request"] result = await execute_command(websocket, { "method": "network.failRequest", @@ -174,6 +182,7 @@ async def test_fail_request_twice(websocket, context_id, example_url): async def test_fail_request_with_auth_required_phase( websocket, context_id, phases, exception_and_response_expected, auth_required_url): + await subscribe(websocket, ["network.beforeRequestSent"], [context_id]) await subscribe(websocket, ["cdp.Fetch.requestPaused"]) result = await execute_command( @@ -201,8 +210,40 @@ async def test_fail_request_with_auth_required_phase( } }) - event_response = await wait_for_event(websocket, "cdp.Fetch.requestPaused") - assert event_response == { + network_event_response = await wait_for_event(websocket, + "network.beforeRequestSent") + assert network_event_response == AnyExtending({ + "method": "network.beforeRequestSent", + "params": { + "context": context_id, + "initiator": { + "type": "other", + }, + "isBlocked": not exception_and_response_expected, + "navigation": ANY_STR, + "redirectCount": 0, + "request": { + "request": ANY_STR, + "url": auth_required_url, + "method": "GET", + "headers": ANY_LIST, + "cookies": [], + "headersSize": -1, + "bodySize": 0, + "timings": ANY_DICT, + }, + "timestamp": ANY_TIMESTAMP, + }, + "type": "event", + }) + network_id_from_bidi_event = network_event_response["params"]["request"][ + "request"] + + # TODO: Replace with "network.authRequired" BiDi event when it is implemented. + # If we remove this "wait_for_event" block the test fails. + cdp_event_response = await wait_for_event(websocket, + "cdp.Fetch.requestPaused") + assert cdp_event_response == { "method": "cdp.Fetch.requestPaused", "params": { "event": "Fetch.requestPaused", @@ -224,27 +265,30 @@ async def test_fail_request_with_auth_required_phase( }, "type": "event", } - network_id = event_response["params"]["params"]["networkId"] + network_id_from_cdp_event = cdp_event_response["params"]["params"][ + "networkId"] + + assert network_id_from_bidi_event == network_id_from_cdp_event if exception_and_response_expected: with pytest.raises( Exception, match=str({ "error": "invalid argument", - "message": f"Blocked request for network id '{network_id}' is in 'AuthRequired' phase" + "message": f"Blocked request for network id '{network_id_from_bidi_event}' is in 'AuthRequired' phase" })): await execute_command( websocket, { "method": "network.failRequest", "params": { - "request": network_id + "request": network_id_from_bidi_event }, }) @pytest.mark.asyncio async def test_fail_request_completes(websocket, context_id, example_url): - await subscribe(websocket, ["cdp.Fetch.requestPaused"]) + await subscribe(websocket, ["network.beforeRequestSent"], [context_id]) result = await execute_command( websocket, { @@ -271,26 +315,33 @@ async def test_fail_request_completes(websocket, context_id, example_url): } }) - event_response = await wait_for_event(websocket, "cdp.Fetch.requestPaused") - assert event_response == { - "method": "cdp.Fetch.requestPaused", + event_response = await wait_for_event(websocket, + "network.beforeRequestSent") + assert event_response == AnyExtending({ + "method": "network.beforeRequestSent", "params": { - "event": "Fetch.requestPaused", - "params": { - "frameId": context_id, - "networkId": ANY_STR, - "request": AnyExtending({ - "headers": ANY_DICT, - "url": example_url, - }), - "requestId": ANY_STR, - "resourceType": "Document", + "context": context_id, + "initiator": { + "type": "other", }, - "session": ANY_STR, + "isBlocked": True, + "navigation": ANY_STR, + "redirectCount": 0, + "request": { + "request": ANY_STR, + "url": example_url, + "method": "GET", + "headers": ANY_LIST, + "cookies": [], + "headersSize": -1, + "bodySize": 0, + "timings": ANY_DICT, + }, + "timestamp": ANY_TIMESTAMP, }, "type": "event", - } - network_id = event_response["params"]["params"]["networkId"] + }) + network_id = event_response["params"]["request"]["request"] await subscribe(websocket, ["cdp.Network.loadingFailed"]) @@ -324,7 +375,7 @@ 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, ["cdp.Fetch.requestPaused"]) + await subscribe(websocket, ["network.beforeRequestSent"], [context_id]) result = await execute_command( websocket, { @@ -352,26 +403,32 @@ async def test_fail_request_completes_new_request_still_blocks( }) event_response1 = await wait_for_event(websocket, - "cdp.Fetch.requestPaused") - assert event_response1 == { - "method": "cdp.Fetch.requestPaused", + "network.beforeRequestSent") + assert event_response1 == AnyExtending({ + "method": "network.beforeRequestSent", "params": { - "event": "Fetch.requestPaused", - "params": { - "frameId": context_id, - "networkId": ANY_STR, - "request": AnyExtending({ - "headers": ANY_DICT, - "url": example_url, - }), - "requestId": ANY_STR, - "resourceType": "Document", + "context": context_id, + "initiator": { + "type": "other", }, - "session": ANY_STR, + "isBlocked": True, + "navigation": ANY_STR, + "redirectCount": 0, + "request": { + "request": ANY_STR, + "url": example_url, + "method": "GET", + "headers": ANY_LIST, + "cookies": [], + "headersSize": -1, + "bodySize": 0, + "timings": ANY_DICT, + }, + "timestamp": ANY_TIMESTAMP, }, "type": "event", - } - network_id_1 = event_response1["params"]["params"]["networkId"] + }) + network_id_1 = event_response1["params"]["request"]["request"] await subscribe(websocket, ["cdp.Network.loadingFailed"]) @@ -412,26 +469,32 @@ async def test_fail_request_completes_new_request_still_blocks( }) event_response2 = await wait_for_event(websocket, - "cdp.Fetch.requestPaused") - assert event_response2 == { - "method": "cdp.Fetch.requestPaused", + "network.beforeRequestSent") + assert event_response2 == AnyExtending({ + "method": "network.beforeRequestSent", "params": { - "event": "Fetch.requestPaused", - "params": { - "frameId": context_id, - "networkId": ANY_STR, - "request": AnyExtending({ - "headers": ANY_DICT, - "url": example_url, - }), - "requestId": ANY_STR, - "resourceType": "Document", + "context": context_id, + "initiator": { + "type": "other", }, - "session": ANY_STR, + "isBlocked": True, + "navigation": ANY_STR, + "redirectCount": 0, + "request": { + "request": ANY_STR, + "url": example_url, + "method": "GET", + "headers": ANY_LIST, + "cookies": [], + "headersSize": -1, + "bodySize": 0, + "timings": ANY_DICT, + }, + "timestamp": ANY_TIMESTAMP, }, "type": "event", - } - network_id_2 = event_response2["params"]["params"]["networkId"] + }) + network_id_2 = event_response2["params"]["request"]["request"] assert event_response1 != event_response2 @@ -442,7 +505,7 @@ async def test_fail_request_completes_new_request_still_blocks( @pytest.mark.asyncio async def test_fail_request_multiple_contexts(websocket, context_id, another_context_id, example_url): - await subscribe(websocket, ["cdp.Fetch.requestPaused"]) + await subscribe(websocket, ["network.beforeRequestSent"]) result = await execute_command( websocket, { @@ -471,26 +534,32 @@ async def test_fail_request_multiple_contexts(websocket, context_id, }) event_response1 = await wait_for_event(websocket, - "cdp.Fetch.requestPaused") - assert event_response1 == { - "method": "cdp.Fetch.requestPaused", + "network.beforeRequestSent") + assert event_response1 == AnyExtending({ + "method": "network.beforeRequestSent", "params": { - "event": "Fetch.requestPaused", - "params": { - "frameId": context_id, - "networkId": ANY_STR, - "request": AnyExtending({ - "headers": ANY_DICT, - "url": example_url, - }), - "requestId": ANY_STR, - "resourceType": "Document", + "context": context_id, + "initiator": { + "type": "other", }, - "session": ANY_STR, + "isBlocked": True, + "navigation": ANY_STR, + "redirectCount": 0, + "request": { + "request": ANY_STR, + "url": example_url, + "method": "GET", + "headers": ANY_LIST, + "cookies": [], + "headersSize": -1, + "bodySize": 0, + "timings": ANY_DICT, + }, + "timestamp": ANY_TIMESTAMP, }, "type": "event", - } - network_id_1 = event_response1["params"]["params"]["networkId"] + }) + network_id_1 = event_response1["params"]["request"]["request"] # Navigation in second context. await send_JSON_command( @@ -503,26 +572,32 @@ async def test_fail_request_multiple_contexts(websocket, context_id, }) event_response2 = await wait_for_event(websocket, - "cdp.Fetch.requestPaused") - assert event_response2 == { - "method": "cdp.Fetch.requestPaused", + "network.beforeRequestSent") + assert event_response2 == AnyExtending({ + "method": "network.beforeRequestSent", "params": { - "event": "Fetch.requestPaused", - "params": { - "frameId": another_context_id, - "networkId": ANY_STR, - "request": AnyExtending({ - "headers": ANY_DICT, - "url": example_url, - }), - "requestId": ANY_STR, - "resourceType": "Document", + "context": another_context_id, + "initiator": { + "type": "other", }, - "session": ANY_STR, + "isBlocked": True, + "navigation": ANY_STR, + "redirectCount": 0, + "request": { + "request": ANY_STR, + "url": example_url, + "method": "GET", + "headers": ANY_LIST, + "cookies": [], + "headersSize": -1, + "bodySize": 0, + "timings": ANY_DICT, + }, + "timestamp": ANY_TIMESTAMP, }, "type": "event", - } - network_id_2 = event_response2["params"]["params"]["networkId"] + }) + network_id_2 = event_response2["params"]["request"]["request"] assert network_id_1 != network_id_2 @@ -581,6 +656,3 @@ async def test_fail_request_multiple_contexts(websocket, context_id, }, "type": "event", } - - -# TODO: assertion with isBlocked: true diff --git a/tests/network/test_remove_intercept.py b/tests/network/test_remove_intercept.py index 7e38d9c444..b52c4a53e6 100644 --- a/tests/network/test_remove_intercept.py +++ b/tests/network/test_remove_intercept.py @@ -111,9 +111,8 @@ async def test_remove_intercept_twice(websocket): "string and pattern", ]) @pytest.mark.asyncio -async def test_remove_intercept_unblocks(websocket, context_id, - another_context_id, url_patterns, - example_url): +async def test_remove_intercept_unblocks_use_cdp_events( + websocket, context_id, another_context_id, url_patterns, example_url): await subscribe(websocket, ["cdp.Fetch.requestPaused"]) await subscribe(websocket, ["network"], [another_context_id]) @@ -205,3 +204,140 @@ async def test_remove_intercept_unblocks(websocket, context_id, "timestamp": ANY_TIMESTAMP } } + + +@pytest.mark.asyncio +@pytest.mark.parametrize("url_patterns", [ + [ + { + "type": "string", + "pattern": "https://www.example.com/", + }, + ], + [ + { + "type": "pattern", + "protocol": "https", + "hostname": "www.example.com", + "pathname": "/", + }, + ], + [ + { + "type": "string", + "pattern": "https://www.example.com/", + }, + { + "type": "pattern", + "protocol": "https", + "hostname": "www.example.com", + "pathname": "/", + }, + ], +], + ids=[ + "string", + "pattern", + "string and pattern", + ]) +@pytest.mark.asyncio +async def test_remove_intercept_unblocks_use_bidi_events( + websocket, context_id, another_context_id, url_patterns, example_url): + await subscribe(websocket, ["network.beforeRequestSent"], [context_id]) + await subscribe(websocket, ["network"], [another_context_id]) + + result = await execute_command( + websocket, { + "method": "network.addIntercept", + "params": { + "phases": ["beforeRequestSent"], + "urlPatterns": url_patterns, + }, + }) + + assert result == { + "intercept": ANY_UUID, + } + intercept_id = result["intercept"] + + await send_JSON_command( + websocket, { + "method": "browsingContext.navigate", + "params": { + "url": example_url, + "context": context_id, + } + }) + + event_response = await wait_for_event(websocket, + "network.beforeRequestSent") + assert event_response == { + "method": "network.beforeRequestSent", + "params": { + "context": context_id, + "initiator": { + "type": "other", + }, + "isBlocked": True, + "navigation": ANY_STR, + "redirectCount": 0, + "request": { + "request": ANY_STR, + "url": example_url, + "method": "GET", + "headers": ANY_LIST, + "cookies": [], + "headersSize": -1, + "bodySize": 0, + "timings": ANY_DICT + }, + "timestamp": ANY_TIMESTAMP, + }, + "type": "event", + } + + result = await execute_command( + websocket, { + "method": "network.removeIntercept", + "params": { + "intercept": intercept_id, + }, + }) + assert result == {} + + # Try again, with the intercept removed, in another context. + await send_JSON_command( + websocket, { + "method": "browsingContext.navigate", + "params": { + "url": example_url, + "wait": "complete", + "context": another_context_id, + } + }) + + # Network events should complete. + event_response = await wait_for_event(websocket, + "network.responseCompleted") + assert event_response == { + 'type': 'event', + "method": "network.responseCompleted", + "params": { + "isBlocked": False, + "context": another_context_id, + "navigation": ANY_STR, + "redirectCount": 0, + "request": { + "request": ANY_STR, + "url": example_url, + "method": "GET", + "headers": ANY_LIST, + "cookies": [], + "headersSize": -1, + "bodySize": 0, + "timings": ANY_DICT + }, + "response": ANY_DICT, + "timestamp": ANY_TIMESTAMP, + } + }