[py] use get_timeout() for urllib pool manager timeouts in remote connection, prevents passing the default socket object directly (#10563)

symonk · AutomatedTester · web-flow · commit 5b840664a6a5 · 2022-04-29T11:52:36.000+01:00
* pass the underlying `timeout` to urls pool manager

* invoke the `get_timeout()` class method for `urllib` timeouts

* fix class state leaking across tests for url lib timeouts; add unit test for default

Co-authored-by: David Burns &lt;david.burns@theautomatedtester.co.uk&gt;
diff --git a/py/selenium/webdriver/remote/remote_connection.py b/py/selenium/webdriver/remote/remote_connection.py
@@ -143,7 +143,7 @@ def _seperate_http_proxy_auth(self):
 
     def _get_connection_manager(self):
         pool_manager_init_args = {
-            'timeout': self._timeout
+            'timeout': self.get_timeout()
         }
         if self._ca_certs:
             pool_manager_init_args['cert_reqs'] = 'CERT_REQUIRED'
diff --git a/py/test/unit/selenium/webdriver/remote/remote_connection_tests.py b/py/test/unit/selenium/webdriver/remote/remote_connection_tests.py
@@ -15,13 +15,10 @@
 # specific language governing permissions and limitations
 # under the License.
 
-
-import urllib3
-import pytest
-
-
 from urllib import parse
 
+import pytest
+import urllib3
 
 from selenium import __version__
 from selenium.webdriver.remote.remote_connection import (
@@ -96,15 +93,21 @@ def test_get_connection_manager_without_proxy(mock_proxy_settings_missing):
     assert type(conn) == urllib3.PoolManager
 
 
-def test_get_connection_manager_for_certs_and_timeout():
+def test_get_connection_manager_for_certs_and_timeout(monkeypatch):
+    monkeypatch.setattr(RemoteConnection, "get_timeout", lambda _: 10)  # Class state; leaks into subsequent tests.
     remote_connection = RemoteConnection('http://remote', keep_alive=False)
-    remote_connection.set_timeout(10)
     conn = remote_connection._get_connection_manager()
     assert conn.connection_pool_kw['timeout'] == 10
     assert conn.connection_pool_kw['cert_reqs'] == 'CERT_REQUIRED'
     assert 'certifi/cacert.pem' in conn.connection_pool_kw['ca_certs']
 
 
+def test_default_socket_timeout_is_correct():
+    remote_connection = RemoteConnection("http://remote", keep_alive=True)
+    conn = remote_connection._get_connection_manager()
+    assert conn.connection_pool_kw['timeout'] is None
+
+
 def test_get_connection_manager_with_proxy(mock_proxy_settings):
     remote_connection = RemoteConnection('http://remote', keep_alive=False)
     conn = remote_connection._get_connection_manager()

Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,7 @@ def _seperate_http_proxy_auth(self):`
`143`	`143`
`144`	`144`	`def _get_connection_manager(self):`
`145`	`145`	`pool_manager_init_args = {`
`146`		`- 'timeout': self._timeout`
	`146`	`+ 'timeout': self.get_timeout()`
`147`	`147`	`}`
`148`	`148`	`if self._ca_certs:`
`149`	`149`	`pool_manager_init_args['cert_reqs'] = 'CERT_REQUIRED'`