diff --git a/py/selenium/webdriver/remote/client_config.py b/py/selenium/webdriver/remote/client_config.py index bec52531bd43e..d0eaccac31e2c 100644 --- a/py/selenium/webdriver/remote/client_config.py +++ b/py/selenium/webdriver/remote/client_config.py @@ -134,14 +134,16 @@ def __init__( self.keep_alive = keep_alive self.proxy = proxy self.ignore_certificates = ignore_certificates - self.init_args_for_pool_manager = init_args_for_pool_manager or {} + # Copy caller-owned dicts so two ClientConfigs (or the caller's own dict) can't be + # aliased into one another and mutated in place. + self.init_args_for_pool_manager = dict(init_args_for_pool_manager or {}) self.timeout = socket.getdefaulttimeout() if timeout is None else timeout self.username = username self.password = password self.auth_type = auth_type self.token = token self.user_agent = user_agent - self.extra_headers = extra_headers + self.extra_headers = dict(extra_headers) if extra_headers is not None else None self.websocket_timeout = websocket_timeout self.websocket_interval = websocket_interval self.websocket_max_message_size = websocket_max_message_size diff --git a/py/selenium/webdriver/remote/remote_connection.py b/py/selenium/webdriver/remote/remote_connection.py index 38f9493d1bf1a..1a863dc4efc2c 100644 --- a/py/selenium/webdriver/remote/remote_connection.py +++ b/py/selenium/webdriver/remote/remote_connection.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. +import functools import logging import string import sys @@ -138,6 +139,23 @@ } +class _classorinstancemethod: + """Bind the wrapped function to the instance when accessed on one, else to the class. + + Lets get_remote_connection_headers keep its classmethod call surface + (RemoteConnection.get_remote_connection_headers(...) and subclass ``super()`` overrides + such as Appium's) while, on an instance, resolving that connection's own + user_agent/extra_headers instead of the process-wide class defaults. + """ + + def __init__(self, func): + self.func = func + functools.update_wrapper(self, func) + + def __get__(self, obj, cls=None): + return functools.partial(self.func, obj if obj is not None else cls) + + class RemoteConnection: """A connection with the Remote WebDriver server. @@ -239,10 +257,15 @@ def set_certificate_bundle_path(cls, path): ) cls._client_config.ca_certs = path - @classmethod + @_classorinstancemethod def get_remote_connection_headers(cls, parsed_url, keep_alive=False): """Get headers for remote request. + ``cls`` is the connection instance when called on one (so ``user_agent`` and + ``extra_headers`` come from that connection's client_config) and the class when + called on the class, preserving ``RemoteConnection.get_remote_connection_headers`` + and subclass ``super()`` overrides. + Args: parsed_url: The parsed url keep_alive: Is this a keep-alive connection (default: False) @@ -331,8 +354,13 @@ def __init__( RemoteConnection._timeout = self._client_config.timeout RemoteConnection._ca_certs = self._client_config.ca_certs RemoteConnection._client_config = self._client_config - RemoteConnection.extra_headers = self._client_config.extra_headers or RemoteConnection.extra_headers - RemoteConnection.user_agent = self._client_config.user_agent or RemoteConnection.user_agent + # Resolve this connection's headers once into instance state instead of mutating the + # shared class attributes (which leaked one connection's headers, auth included, onto + # every other one). A client_config value replaces the class-level default, matching + # trunk's "or" semantics; get_remote_connection_headers reads these per-instance via + # the _classorinstancemethod descriptor. + self.user_agent = self._client_config.user_agent or type(self).user_agent + self.extra_headers = dict(self._client_config.extra_headers or type(self).extra_headers or {}) or None if remote_server_addr: warnings.warn( diff --git a/py/test/unit/selenium/webdriver/remote/client_config_tests.py b/py/test/unit/selenium/webdriver/remote/client_config_tests.py index dcc291de1ff4b..e745f6d8cb0e5 100644 --- a/py/test/unit/selenium/webdriver/remote/client_config_tests.py +++ b/py/test/unit/selenium/webdriver/remote/client_config_tests.py @@ -46,6 +46,23 @@ def system_config(remote_server_addr="http://localhost:4444"): return ClientConfig(remote_server_addr=remote_server_addr, proxy=Proxy(raw={"proxyType": ProxyType.SYSTEM})) +def test_extra_headers_are_copied_from_the_callers_dict(): + """The caller's dict must not be aliased into the config, and two configs must not share one.""" + caller = {"X-Api-Key": "secret"} + cfg = ClientConfig(remote_server_addr="http://localhost:4444", extra_headers=caller) + assert cfg.extra_headers == caller + assert cfg.extra_headers is not caller + caller["X-Api-Key"] = "changed" + assert cfg.extra_headers["X-Api-Key"] == "secret" + + +def test_init_args_for_pool_manager_are_copied_from_the_callers_dict(): + caller = {"maxsize": 10} + cfg = ClientConfig(remote_server_addr="http://localhost:4444", init_args_for_pool_manager=caller) + assert cfg.init_args_for_pool_manager == caller + assert cfg.init_args_for_pool_manager is not caller + + def test_websocket_max_message_size_defaults_to_none(config): assert config.websocket_max_message_size is None diff --git a/py/test/unit/selenium/webdriver/remote/remote_connection_tests.py b/py/test/unit/selenium/webdriver/remote/remote_connection_tests.py index 697e8ad98aeda..fd39f55c63b76 100644 --- a/py/test/unit/selenium/webdriver/remote/remote_connection_tests.py +++ b/py/test/unit/selenium/webdriver/remote/remote_connection_tests.py @@ -404,15 +404,37 @@ def mock_no_proxy_settings(monkeypatch): monkeypatch.setenv("NO_PROXY", "65.253.214.253,localhost,127.0.0.1,*zyz.xx,::1,127.0.0.0/8") -@patch("selenium.webdriver.remote.remote_connection.RemoteConnection.get_remote_connection_headers") -def test_override_user_agent_in_headers(mock_get_remote_connection_headers, remote_connection): - RemoteConnection.user_agent = "custom-agent/1.0 (python 3.13)" +class _FakeResponse: + status = 200 + data = b'{"value": {}}' + headers = {"Content-Type": "application/json"} - mock_get_remote_connection_headers.return_value = { - "Accept": "application/json", - "Content-Type": "application/json;charset=UTF-8", - "User-Agent": "custom-agent/1.0 (python 3.13)", - } + def close(self): + pass + + +def _capture_sent_headers(remote_connection): + """Stub out the pool manager and return the list that records each request's headers. + + Only the keep-alive path reuses ``self._conn``; a ``keep_alive=False`` connection builds + a fresh pool manager per request and would reach the network, so guard against that here. + """ + assert getattr(remote_connection, "_conn", None) is not None, ( + "_capture_sent_headers only intercepts keep_alive=True connections (self._conn)" + ) + sent = [] + + class _Conn: + def request(self, method, url, body=None, headers=None, timeout=None): + sent.append(dict(headers)) + return _FakeResponse() + + remote_connection._conn = _Conn() + return sent + + +def test_override_user_agent_in_headers(monkeypatch): + monkeypatch.setattr(RemoteConnection, "user_agent", "custom-agent/1.0 (python 3.13)") headers = RemoteConnection.get_remote_connection_headers(parse.urlparse("http://remote")) @@ -421,26 +443,20 @@ def test_override_user_agent_in_headers(mock_get_remote_connection_headers, remo assert headers.get("Content-Type") == "application/json;charset=UTF-8" -@patch("selenium.webdriver.remote.remote_connection.RemoteConnection.get_remote_connection_headers") -def test_override_user_agent_via_client_config(mock_get_remote_connection_headers): +def test_override_user_agent_via_client_config(): client_config = ClientConfig( remote_server_addr="http://localhost:4444", user_agent="custom-agent/1.0 (python 3.13)", extra_headers={"Content-Type": "application/xml;charset=UTF-8"}, ) remote_connection = RemoteConnection(client_config=client_config) + sent = _capture_sent_headers(remote_connection) - mock_get_remote_connection_headers.return_value = { - "Accept": "application/json", - "Content-Type": "application/xml;charset=UTF-8", - "User-Agent": "custom-agent/1.0 (python 3.13)", - } - - headers = remote_connection.get_remote_connection_headers(parse.urlparse("http://localhost:4444")) + remote_connection.execute("newSession", {}) - assert headers.get("User-Agent") == "custom-agent/1.0 (python 3.13)" - assert headers.get("Accept") == "application/json" - assert headers.get("Content-Type") == "application/xml;charset=UTF-8" + assert sent[0]["User-Agent"] == "custom-agent/1.0 (python 3.13)" + assert sent[0]["Accept"] == "application/json" + assert sent[0]["Content-Type"] == "application/xml;charset=UTF-8" @patch("selenium.webdriver.remote.remote_connection.RemoteConnection._request") @@ -455,8 +471,10 @@ def test_register_extra_headers(mock_request, remote_connection): assert headers["Foo"] == "bar" -@patch("selenium.webdriver.remote.remote_connection.RemoteConnection._request") -def test_register_extra_headers_via_client_config(mock_request): +def test_register_extra_headers_via_client_config(): + # client_config extra_headers are applied per-connection in _request (not copied onto + # the class), so assert they reach the actual request rather than the class-level + # get_remote_connection_headers classmethod. client_config = ClientConfig( remote_server_addr="http://localhost:4444", extra_headers={ @@ -465,14 +483,12 @@ def test_register_extra_headers_via_client_config(mock_request): }, ) remote_connection = RemoteConnection(client_config=client_config) + sent = _capture_sent_headers(remote_connection) - mock_request.return_value = {"status": 200, "value": "OK"} remote_connection.execute("newSession", {}) - mock_request.assert_called_once_with("POST", "http://localhost:4444/session", body="{}") - headers = remote_connection.get_remote_connection_headers(parse.urlparse("http://localhost:4444"), False) - assert headers["Authorization"] == "AWS4-HMAC-SHA256" - assert headers["Credential"] == "abc/20200618/us-east-1/execute-api/aws4_request" + assert sent[0]["Authorization"] == "AWS4-HMAC-SHA256" + assert sent[0]["Credential"] == "abc/20200618/us-east-1/execute-api/aws4_request" def test_backwards_compatibility_with_appium_connection(): @@ -674,3 +690,116 @@ def test_get_remote_connection_selects_browser_specific_handler( ignore_local_proxy=True, ) assert type(conn) is expected_handler + + +def test_client_config_headers_do_not_leak_across_connections(monkeypatch): + """Per-connection user_agent/extra_headers must not be shared across RemoteConnection instances.""" + # Reset any class-level pollution left by other tests so the default is deterministic. + monkeypatch.setattr(RemoteConnection, "extra_headers", None) + monkeypatch.setattr(RemoteConnection, "user_agent", "selenium-default-ua") + + conn_a = RemoteConnection( + client_config=ClientConfig( + remote_server_addr="http://localhost:4444", + user_agent="ua-a", + extra_headers={"X-Tenant": "A", "Authorization": "Bearer SECRET-A"}, + ) + ) + # A plain connection created AFTER conn_a (worst case for the old class-attribute leak). + conn_b = RemoteConnection(client_config=ClientConfig(remote_server_addr="http://localhost:4444")) + + sent_a = _capture_sent_headers(conn_a) + sent_b = _capture_sent_headers(conn_b) + + conn_a._request("GET", "http://localhost:4444/status") + conn_b._request("GET", "http://localhost:4444/status") + + # conn_a keeps its own identity. + assert sent_a[0]["User-Agent"] == "ua-a" + assert sent_a[0]["X-Tenant"] == "A" + assert sent_a[0]["Authorization"] == "Bearer SECRET-A" + + # conn_b must not inherit conn_a's headers, token, or user agent. + assert "X-Tenant" not in sent_b[0] + assert "Authorization" not in sent_b[0] + assert sent_b[0]["User-Agent"] == "selenium-default-ua" + + +def test_class_level_extra_headers_are_a_default_not_a_floor(monkeypatch): + """A process-wide class-level header applies only when a connection sets none of its own. + + A connection that provides its own extra_headers gets exactly those (client_config replaces + the class-level value), so a global header - auth included - is never forced onto it. + """ + monkeypatch.setattr(RemoteConnection, "extra_headers", {"Authorization": "Bearer GLOBAL"}) + monkeypatch.setattr(RemoteConnection, "user_agent", "selenium-default-ua") + + # Sets its own extra_headers -> the class-level Authorization must not reach the wire. + own = RemoteConnection( + client_config=ClientConfig(remote_server_addr="http://localhost:4444", extra_headers={"X-Tenant": "A"}) + ) + sent_own = _capture_sent_headers(own) + own._request("GET", "http://localhost:4444/status") + assert sent_own[0]["X-Tenant"] == "A" + assert "Authorization" not in sent_own[0] + + # Sets no extra_headers -> the class-level default still applies. + default = RemoteConnection(client_config=ClientConfig(remote_server_addr="http://localhost:4444")) + sent_default = _capture_sent_headers(default) + default._request("GET", "http://localhost:4444/status") + assert sent_default[0]["Authorization"] == "Bearer GLOBAL" + + +def test_get_remote_connection_headers_on_instance_matches_the_wire(): + """The public classmethod, called on a connection, reports what that connection sends.""" + cfg = ClientConfig( + remote_server_addr="http://localhost:4444", + user_agent="my-ua/1", + extra_headers={"X-Tenant": "A"}, + ) + conn = RemoteConnection(client_config=cfg) + + reported = conn.get_remote_connection_headers(parse.urlparse(cfg.remote_server_addr)) + sent = _capture_sent_headers(conn) + conn._request("GET", "http://localhost:4444/status") + + assert reported["User-Agent"] == sent[0]["User-Agent"] == "my-ua/1" + assert reported["X-Tenant"] == sent[0]["X-Tenant"] == "A" + + +def test_subclass_class_level_headers_survive_client_config(monkeypatch): + """Subclass class-level headers survive client_config. + + A subclass whose get_remote_connection_headers override reads its own class attributes + (as appium-python-client's AppiumConnection does for issue #14694) keeps them regardless of + client_config, because its zero-arg ``super()`` stays class-bound. + """ + monkeypatch.setattr(RemoteConnection, "user_agent", "selenium-default-ua") + + class _DownstreamConnection(RemoteConnection): + user_agent = f"downstream/1.0 ({RemoteConnection.user_agent})" + extra_headers = {"X-Downstream": "on"} + + @classmethod + def get_remote_connection_headers(cls, parsed_url, keep_alive=True): + return {**super().get_remote_connection_headers(parsed_url, keep_alive=keep_alive), **cls.extra_headers} + + # No client_config user_agent/extra_headers: the subclass identity is used. + c1 = _DownstreamConnection(client_config=ClientConfig(remote_server_addr="http://localhost:4444")) + s1 = _capture_sent_headers(c1) + c1._request("GET", "http://localhost:4444/status") + assert s1[0]["User-Agent"] == _DownstreamConnection.user_agent + assert s1[0]["X-Downstream"] == "on" + + # client_config sets its own user_agent/extra_headers: the subclass override still wins. + c2 = _DownstreamConnection( + client_config=ClientConfig( + remote_server_addr="http://localhost:4444", + user_agent="my-app/1.0", + extra_headers={"X-Downstream": "off"}, + ) + ) + s2 = _capture_sent_headers(c2) + c2._request("GET", "http://localhost:4444/status") + assert s2[0]["User-Agent"] == _DownstreamConnection.user_agent + assert s2[0]["X-Downstream"] == "on"