Skip to content

Commit

Permalink
fix: the capability to ignore cert errors should be browser-wide (#2369)
Browse files Browse the repository at this point in the history
Addressing #2357
* Fix the root cause.
* Add e2e.

Prerequisite:
#2368
  • Loading branch information
sadym-chromium authored Jul 2, 2024
1 parent be5e61b commit 6db665b
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 17 deletions.
5 changes: 4 additions & 1 deletion src/bidiMapper/BidiServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ export class BidiServer extends EventEmitter<BidiServerEvent> {
this.#realmStorage,
networkStorage,
this.#preloadScriptStorage,
options?.acceptInsecureCerts ?? false,
defaultUserContextId,
options?.unhandledPromptBehavior,
logger
Expand Down Expand Up @@ -154,6 +153,10 @@ export class BidiServer extends EventEmitter<BidiServerEvent> {
const [{browserContextIds}, {targetInfos}] = await Promise.all([
browserCdpClient.sendCommand('Target.getBrowserContexts'),
browserCdpClient.sendCommand('Target.getTargets'),
// This is required to ignore certificate errors when service worker is fetched.
browserCdpClient.sendCommand('Security.setIgnoreCertificateErrors', {
ignore: options?.acceptInsecureCerts ?? false,
}),
]);
let defaultUserContextId = 'default';
for (const info of targetInfos) {
Expand Down
9 changes: 0 additions & 9 deletions src/bidiMapper/modules/cdp/CdpTarget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export class CdpTarget {

readonly #unblocked = new Deferred<Result<void>>();
readonly #unhandledPromptBehavior?: Session.UserPromptHandler;
readonly #acceptInsecureCerts: boolean;
readonly #logger: LoggerFn | undefined;

#networkDomainEnabled = false;
Expand All @@ -65,7 +64,6 @@ export class CdpTarget {
preloadScriptStorage: PreloadScriptStorage,
browsingContextStorage: BrowsingContextStorage,
networkStorage: NetworkStorage,
acceptInsecureCerts: boolean,
unhandledPromptBehavior?: Session.UserPromptHandler,
logger?: LoggerFn
): CdpTarget {
Expand All @@ -78,7 +76,6 @@ export class CdpTarget {
preloadScriptStorage,
browsingContextStorage,
networkStorage,
acceptInsecureCerts,
unhandledPromptBehavior,
logger
);
Expand All @@ -103,7 +100,6 @@ export class CdpTarget {
preloadScriptStorage: PreloadScriptStorage,
browsingContextStorage: BrowsingContextStorage,
networkStorage: NetworkStorage,
acceptInsecureCerts: boolean,
unhandledPromptBehavior?: Session.UserPromptHandler,
logger?: LoggerFn
) {
Expand All @@ -115,7 +111,6 @@ export class CdpTarget {
this.#preloadScriptStorage = preloadScriptStorage;
this.#networkStorage = networkStorage;
this.#browsingContextStorage = browsingContextStorage;
this.#acceptInsecureCerts = acceptInsecureCerts;
this.#unhandledPromptBehavior = unhandledPromptBehavior;
this.#logger = logger;
}
Expand Down Expand Up @@ -167,10 +162,6 @@ export class CdpTarget {
this.#cdpClient.sendCommand('Page.setLifecycleEventsEnabled', {
enabled: true,
}),
// Set ignore certificate errors for each target.
this.#cdpClient.sendCommand('Security.setIgnoreCertificateErrors', {
ignore: this.#acceptInsecureCerts,
}),
this.toggleNetworkIfNeeded(),
this.#cdpClient.sendCommand('Target.setAutoAttach', {
autoAttach: true,
Expand Down
4 changes: 0 additions & 4 deletions src/bidiMapper/modules/cdp/CdpTargetManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export class CdpTargetManager {

readonly #browsingContextStorage: BrowsingContextStorage;
readonly #networkStorage: NetworkStorage;
readonly #acceptInsecureCerts: boolean;
readonly #preloadScriptStorage: PreloadScriptStorage;
readonly #realmStorage: RealmStorage;

Expand All @@ -65,12 +64,10 @@ export class CdpTargetManager {
realmStorage: RealmStorage,
networkStorage: NetworkStorage,
preloadScriptStorage: PreloadScriptStorage,
acceptInsecureCerts: boolean,
defaultUserContextId: Browser.UserContext,
unhandledPromptBehavior?: Session.UserPromptHandler,
logger?: LoggerFn
) {
this.#acceptInsecureCerts = acceptInsecureCerts;
this.#cdpConnection = cdpConnection;
this.#browserCdpClient = browserCdpClient;
this.#selfTargetId = selfTargetId;
Expand Down Expand Up @@ -256,7 +253,6 @@ export class CdpTargetManager {
this.#preloadScriptStorage,
this.#browsingContextStorage,
this.#networkStorage,
this.#acceptInsecureCerts,
this.#unhandledPromptBehavior,
this.#logger
);
Expand Down
Empty file.
62 changes: 62 additions & 0 deletions tests/service_worker/test_navigate.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# 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 pytest
from anys import ANY_STR
from test_helpers import execute_command


@pytest.mark.asyncio
@pytest.mark.parametrize('capabilities', [{
'acceptInsecureCerts': True
}],
indirect=True)
async def test_serviceWorker_acceptInsecureCertsCapability_respected(
websocket, context_id, url_bad_ssl, local_server_bad_ssl):
service_worker_script = local_server_bad_ssl.url_200(
content='', content_type='text/javascript')
service_worker_page = local_server_bad_ssl.url_200(content=f"""<script>
window.registrationPromise = navigator.serviceWorker.register('{service_worker_script}');
</script>""")

await execute_command(
websocket, {
'method': 'browsingContext.navigate',
'params': {
'url': service_worker_page,
'wait': 'complete',
'context': context_id
}
})

resp = await execute_command(
websocket, {
'method': 'script.evaluate',
'params': {
'expression': 'window.registrationPromise.then(r=>r.unregister())',
'awaitPromise': True,
'target': {
'context': context_id
}
}
})
assert resp == {
'realm': ANY_STR,
'result': {
'type': 'boolean',
'value': True,
},
'type': 'success',
}
16 changes: 13 additions & 3 deletions tests/tools/local_http_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,18 @@ class LocalHttpServer:
__path_basic_auth = "/401"
__path_hang_forever = "/hang_forever"
__path_cacheable = "/cacheable"
# __path_sw_page_bad_ssl = "/sw_bad_ssl.html"
# __path_empty_script = "/empty.js"

__protocol: Literal['http', 'https']

content_200: str = 'default 200 page'
content_200_page: str = 'default 200 page'

# def __content_sw_page_bad_ssl(self) -> str:
# return f"""<script>
# window.registrationPromise = navigator.serviceWorker.register('{self.url_empty_script(protocol='https')}');
# </script>"""

def clear(self):
self.__http_server.clear()
Expand Down Expand Up @@ -175,16 +183,18 @@ def url_base(self) -> str:
"""
return self.__http_server.url_for(self.__path_base)

def url_200(self, content=None) -> str:
def url_200(self, content=None, content_type="text/html") -> str:
"""Returns the url for the 200 page with the `default_200_page_content`.
"""
if content is not None:
path = f"{self.__path_200}/{str(uuid.uuid4())}"
if content_type == "text/html":
content = self.__html_doc(content)
self.__http_server \
.expect_request(path) \
.respond_with_data(
self.__html_doc(content),
headers={"Content-Type": "text/html"})
content,
headers={"Content-Type": content_type})

return self.__http_server.url_for(path)

Expand Down

0 comments on commit 6db665b

Please sign in to comment.