diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 894fc073..51db8b05 100644 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -43,6 +43,7 @@ import os import re import shutil # used for attachment download +import socket # used to configure TCP keepalive import ssl import stat # used for attachment upload import sys @@ -68,7 +69,14 @@ # to be exposed as part of the API. from xmlrpc.client import Error, ProtocolError, ResponseError # noqa -from .lib.httplib2 import Http, ProxyInfo, socks +from .lib.httplib2 import ( + DEFAULT_MAX_REDIRECTS, + Http, + HTTPConnectionWithTimeout, + HTTPSConnectionWithTimeout, + ProxyInfo, + socks, +) from .lib.sgtimezone import SgTimezone LOG = logging.getLogger("shotgun_api3") @@ -123,6 +131,146 @@ class BaseEntity(TypedDict, total=False): type: str +# ---------------------------------------------------------------------------- +# Connection keepalive + +# Enable OS-level TCP keepalive so the kernel can notice a peer that has gone +# away silently -- a NAT, firewall or load balancer dropping an idle session +# without sending FIN or RST -- instead of leaving a dead socket in httplib2's +# connection cache. The values below aim to detect such a drop within roughly a +# minute of idling. +# +# Keepalive is best effort only: the probe timers are not adjustable on every +# platform, and probes do not run while data is still unacknowledged. It +# complements rather than replaces _Config.max_connection_idle_secs. +KEEPALIVE_IDLE_SECS = 30 +KEEPALIVE_INTERVAL_SECS = 10 +KEEPALIVE_PROBE_COUNT = 3 + + +def _set_socket_keepalive(sock) -> None: + """ + Best-effort enabling of TCP keepalive on an already connected socket. + + :param sock: Connected socket, or SSL-wrapped socket, to configure. + """ + try: + sock.setsockopt(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1) + except OSError: + # Nothing further to tune if the socket rejects keepalive outright. + LOG.debug("Unable to enable TCP keepalive on socket.", exc_info=True) + return + + # Windows exposes the timers through an ioctl rather than socket options. + # Read via getattr so this branch stays reachable in tests on any platform. + keepalive_vals = getattr(socket, "SIO_KEEPALIVE_VALS", None) + if keepalive_vals is not None and hasattr(sock, "ioctl"): + try: + sock.ioctl( + keepalive_vals, + (1, KEEPALIVE_IDLE_SECS * 1000, KEEPALIVE_INTERVAL_SECS * 1000), + ) + except OSError: + LOG.debug("Unable to tune TCP keepalive timers.", exc_info=True) + return + + # TCP_KEEPIDLE is the idle timer on Linux, TCP_KEEPALIVE on macOS; only one + # of them exists on most platforms, and neither exists on some. + for option_name, value in ( + ("TCP_KEEPIDLE", KEEPALIVE_IDLE_SECS), + ("TCP_KEEPALIVE", KEEPALIVE_IDLE_SECS), + ("TCP_KEEPINTVL", KEEPALIVE_INTERVAL_SECS), + ("TCP_KEEPCNT", KEEPALIVE_PROBE_COUNT), + ): + option = getattr(socket, option_name, None) + if option is None: + continue + try: + sock.setsockopt(socket.IPPROTO_TCP, option, value) + except OSError: + LOG.debug("Unable to set %s on socket." % option_name, exc_info=True) + + +class _KeepaliveConnectionMixin(http.client.HTTPConnection): + """ + Mixin that enables TCP keepalive once the connection is established. + + Must be listed before the httplib2 connection class so that this + ``connect()`` runs and delegates to the real one. ``self.sock`` is the + SSL-wrapped socket for HTTPS, which delegates ``setsockopt`` to the socket + underneath. + + Derives from ``http.client.HTTPConnection``, the common base of both + httplib2 connection classes, so that ``super().connect()`` resolves for type + checkers. It is never instantiated on its own. + """ + + def connect(self) -> None: + super().connect() + _set_socket_keepalive(self.sock) + + +class KeepaliveHTTPConnection(_KeepaliveConnectionMixin, HTTPConnectionWithTimeout): + """ + httplib2 HTTP connection that enables TCP keepalive once connected. + + Passed to ``httplib2.Http.request()`` as its ``connection_type`` so that the + bundled httplib2 does not need to be modified. + """ + + +class KeepaliveHTTPSConnection(_KeepaliveConnectionMixin, HTTPSConnectionWithTimeout): + """ + httplib2 HTTPS connection that enables TCP keepalive once connected. + + Passed to ``httplib2.Http.request()`` as its ``connection_type`` so that the + bundled httplib2 does not need to be modified. + """ + + +KEEPALIVE_CONNECTION_TYPES = { + "http": KeepaliveHTTPConnection, + "https": KeepaliveHTTPSConnection, +} + + +class KeepaliveHttp(Http): + """ + httplib2 ``Http`` that routes every request through a keepalive-enabled + connection class. + + The scheme is resolved per call rather than once at construction because + httplib2 follows redirects by calling ``self.request()`` again, without + forwarding the ``connection_type`` argument it was given. Overriding + ``request()`` catches those recursive calls too, so a redirect to another + authority -- or from http to https -- still gets keepalive. Doing it here + also keeps the bundled httplib2 unmodified. + """ + + def request( + self, + uri: str, + method: str = "GET", + body=None, + headers: Optional[Dict[str, Any]] = None, + redirections: int = DEFAULT_MAX_REDIRECTS, + connection_type=None, + ): + if connection_type is None: + scheme = urllib.parse.urlsplit(uri).scheme.lower() + # An unknown scheme is left as None so httplib2 raises its own + # error rather than being handed a connection class it cannot use. + connection_type = KEEPALIVE_CONNECTION_TYPES.get(scheme) + return super().request( + uri, + method=method, + body=body, + headers=headers, + redirections=redirections, + connection_type=connection_type, + ) + + # ---------------------------------------------------------------------------- # Errors @@ -409,6 +557,22 @@ def __init__(self, sg: "Shotgun"): # (like connection attempts) will timeout after that many seconds # (if it is not given, the global default timeout setting is used) self.timeout_secs: Optional[float] = None + # max_connection_idle_secs bounds how long a cached HTTP(S) connection + # may sit idle before it is closed and recreated rather than reused. A + # NAT, firewall or load balancer along the path can silently drop an + # idle TCP session without sending FIN or RST; reusing such a socket + # blocks in getresponse() until the socket timeout expires. 60 seconds + # sits below the idle timeouts commonly configured on that hardware. + # Set to 0 (or None) to reuse connections regardless of idle time, + # restoring the behaviour of releases before this one. + # + # sg = Shotgun(site_name, script_name, script_key) + # sg.config.max_connection_idle_secs = 30 + # + # Or by setting the ``SHOTGUN_API_MAX_CONNECTION_IDLE`` environment + # variable. In the case that the environment variable is already set, + # setting the property on the config will override it. + self.max_connection_idle_secs: Optional[float] = 60 self.api_ver = "api3" self.convert_datetimes_to_utc = True self._records_per_page: Optional[int] = None @@ -648,6 +812,21 @@ def __init__( "got '%s'." % self.config.rpc_attempt_interval ) + max_idle = os.environ.get("SHOTGUN_API_MAX_CONNECTION_IDLE") + if max_idle is not None: + try: + self.config.max_connection_idle_secs = int(max_idle) + except ValueError: + raise ValueError( + "Invalid value '%s' found in environment variable " + "SHOTGUN_API_MAX_CONNECTION_IDLE, must be int." % max_idle + ) + if self.config.max_connection_idle_secs < 0: + raise ValueError( + "Value of SHOTGUN_API_MAX_CONNECTION_IDLE must be positive, " + "got '%s'." % self.config.max_connection_idle_secs + ) + global SHOTGUN_API_DISABLE_ENTITY_OPTIMIZATION if ( os.environ.get("SHOTGUN_API_DISABLE_ENTITY_OPTIMIZATION", "0") @@ -658,6 +837,10 @@ def __init__( SHOTGUN_API_DISABLE_ENTITY_OPTIMIZATION = True self._connection: Optional[Http] = None + # Monotonic timestamp of the last request that completed on + # self._connection, used to expire connections that have gone stale + # while idle. None means the connection has not been used yet. + self._connection_last_used: Optional[float] = None self.__ca_certs = self._get_certs_file(ca_certs) @@ -3996,7 +4179,13 @@ def _http_request( LOG.debug("Request body is %s" % body) conn = self._get_connection() + # KeepaliveHttp picks the keepalive-enabled connection class itself, for + # this request and for any redirect it follows. resp, content = conn.request(url, method=verb, body=body, headers=headers) + # Record the idle-clock start only once the request has completed. A + # request that raised must not refresh it, or the next call would reuse + # a connection we have no evidence is alive. + self._connection_last_used = time.monotonic() # http response code is handled else where http_status = (resp.status, resp.reason) resp_headers = dict((k.lower(), v) for k, v in resp.items()) @@ -4215,9 +4404,24 @@ def _inbound_visitor(value): def _get_connection(self) -> Http: """ Return the current connection or creates a new connection to the current server. + + A cached connection that has been idle for longer than + ``config.max_connection_idle_secs`` is closed and recreated instead of + being reused, since the peer may have silently dropped the TCP session. """ if self._connection is not None: - return self._connection + if self._is_connection_stale(): + LOG.debug( + "Connection has been idle for more than %s seconds, " + "closing it and reconnecting." + % self.config.max_connection_idle_secs + ) + # _close_connection() resets self._connection to None, so this + # falls through to build a replacement below. httplib2 opens the + # new socket lazily on the next request. + self._close_connection() + else: + return self._connection if self.config.proxy_server: pi = ProxyInfo( @@ -4227,13 +4431,13 @@ def _get_connection(self) -> Http: proxy_user=self.config.proxy_user, proxy_pass=self.config.proxy_pass, ) - self._connection = Http( + self._connection = KeepaliveHttp( timeout=self.config.timeout_secs, ca_certs=self.__ca_certs, proxy_info=pi, ) else: - self._connection = Http( + self._connection = KeepaliveHttp( timeout=self.config.timeout_secs, ca_certs=self.__ca_certs, proxy_info=None, @@ -4241,10 +4445,28 @@ def _get_connection(self) -> Http: return self._connection + def _is_connection_stale(self) -> bool: + """ + Return True if the cached connection has been idle long enough that it + should be replaced rather than reused. + """ + max_idle = self.config.max_connection_idle_secs + if not max_idle: + return False + + # A connection that was created but never used successfully has no + # recorded idle time, so there is nothing to expire. + if self._connection_last_used is None: + return False + + return (time.monotonic() - self._connection_last_used) >= max_idle + def _close_connection(self) -> None: """ Close the current connection. """ + self._connection_last_used = None + if self._connection is None: return diff --git a/tests/test_unit.py b/tests/test_unit.py index 786a83f0..76c8a5ee 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -11,13 +11,16 @@ # not expressly granted therein are reserved by Shotgun Software Inc. import os +import socket import ssl +import threading import unittest from unittest import mock import urllib.request import urllib.error import shotgun_api3 as api +from shotgun_api3 import shotgun from shotgun_api3.lib.httplib2 import Http @@ -854,5 +857,507 @@ def test_urlib(self): assert response is not None +class _FakeClock(object): + """Controllable stand-in for time.monotonic.""" + + def __init__(self, now=1000.0): + self.now = now + + def __call__(self): + return self.now + + def advance(self, seconds): + self.now += seconds + + +class TestConnectionIdleExpiry(unittest.TestCase): + """ + Test that connections idle for longer than config.max_connection_idle_secs + are closed and recreated instead of reused (SG-44724). + + A NAT or load balancer can silently drop an idle keep-alive session, and + reusing that socket blocks until the socket timeout expires. None of these + tests make network requests. + """ + + def setUp(self): + self.sg = api.Shotgun( + "http://server_path", "script_name", "api_key", connect=False + ) + self.clock = _FakeClock() + self.created_connections = [] + + clock_patcher = mock.patch( + "shotgun_api3.shotgun.time.monotonic", side_effect=self.clock + ) + clock_patcher.start() + self.addCleanup(clock_patcher.stop) + + http_patcher = mock.patch( + "shotgun_api3.shotgun.KeepaliveHttp", side_effect=self._make_connection + ) + http_patcher.start() + self.addCleanup(http_patcher.stop) + + def _make_connection(self, *args, **kwargs): + """Build a fake Http whose request() returns a minimal 200 response.""" + conn = mock.MagicMock() + conn.connections = {"http:server_path": mock.MagicMock()} + conn.init_kwargs = kwargs + response = mock.MagicMock() + response.status = 200 + response.reason = "OK" + response.items.return_value = [("content-type", "application/json")] + conn.request.return_value = (response, "{}") + self.created_connections.append(conn) + return conn + + def _request(self): + return self.sg._http_request("GET", "/path", None, {}) + + def test_stale_connection_is_replaced(self): + """A connection idle beyond the limit is closed and recreated.""" + self._request() + first = self.sg._get_connection() + + self.clock.advance(self.sg.config.max_connection_idle_secs + 1) + self._request() + second = self.sg._get_connection() + + self.assertIsNot(first, second) + self.assertEqual(len(self.created_connections), 2) + # The stale connection's socket must actually be closed, not just + # dropped from the cache. + self.assertEqual(first.connections, {}) + + def test_fresh_connection_is_reused(self): + """A connection used recently is reused as before.""" + self._request() + first = self.sg._get_connection() + + self.clock.advance(self.sg.config.max_connection_idle_secs - 1) + self._request() + second = self.sg._get_connection() + + self.assertIs(first, second) + self.assertEqual(len(self.created_connections), 1) + + def test_expiry_is_measured_from_last_use_not_creation(self): + """Steady traffic keeps a connection alive indefinitely.""" + self._request() + first = self.sg._get_connection() + + for _ in range(5): + self.clock.advance(self.sg.config.max_connection_idle_secs - 1) + self._request() + + self.assertIs(first, self.sg._get_connection()) + self.assertEqual(len(self.created_connections), 1) + + def test_unused_connection_is_not_expired(self): + """A connection created but never used has no idle time to expire.""" + first = self.sg._get_connection() + self.clock.advance(self.sg.config.max_connection_idle_secs + 1) + + self.assertIs(first, self.sg._get_connection()) + self.assertEqual(len(self.created_connections), 1) + + def test_failed_request_does_not_refresh_idle_clock(self): + """ + A request that raised is no evidence the socket is alive, so it must not + reset the idle clock. + """ + self._request() + first = self.sg._get_connection() + first.request.side_effect = Exception("boom") + + self.clock.advance(self.sg.config.max_connection_idle_secs - 1) + with self.assertRaises(Exception): + self._request() + + # Only 1 second of headroom remains; without the failed attempt + # refreshing the clock, 2 more seconds must expire the connection. + self.clock.advance(2) + self.assertIsNot(first, self.sg._get_connection()) + + def test_expiry_can_be_disabled(self): + """None and 0 both mean 'reuse regardless of idle time'.""" + for disabled_value in (None, 0): + self.sg._close_connection() + self.created_connections = [] + self.sg.config.max_connection_idle_secs = disabled_value + + self._request() + first = self.sg._get_connection() + self.clock.advance(3600) + + self.assertIs(first, self.sg._get_connection()) + self.assertEqual(len(self.created_connections), 1) + + def test_expiry_preserves_proxy_configuration(self): + """The replacement connection is built with the same proxy settings.""" + self.sg.config.proxy_server = "proxy.example.com" + self.sg.config.proxy_port = 8080 + + self._request() + first = self.sg._get_connection() + self.clock.advance(self.sg.config.max_connection_idle_secs + 1) + self._request() + second = self.sg._get_connection() + + self.assertIsNot(first, second) + self.assertIsNotNone(second.init_kwargs["proxy_info"]) + self.assertEqual( + second.init_kwargs["proxy_info"].proxy_host, "proxy.example.com" + ) + + +class TestSocketKeepalive(unittest.TestCase): + """ + Test that sockets get TCP keepalive enabled once connected (SG-44724). + + Keepalive lets the kernel notice a peer that vanished without FIN or RST. + These tests assert only that the options are attempted, since which timers + are adjustable and whether the OS accepts them is platform dependent. No + network requests are made. + """ + + # Stand-in for the Windows-only socket.SIO_KEEPALIVE_VALS constant. + SIO_SENTINEL = 2550136836 + + def _keepalive_calls(self, sock): + return [ + call + for call in sock.setsockopt.call_args_list + if call[0][:2] == (socket.SOL_SOCKET, socket.SO_KEEPALIVE) + ] + + def _tcp_option_calls(self, sock): + return [ + call + for call in sock.setsockopt.call_args_list + if call[0][0] == socket.IPPROTO_TCP + ] + + def _addrinfo(self, port): + return [(socket.AF_INET, socket.SOCK_STREAM, 6, "", ("127.0.0.1", port))] + + def test_keepalive_enabled_on_socket(self): + sock = mock.MagicMock() + shotgun._set_socket_keepalive(sock) + + self.assertEqual(len(self._keepalive_calls(sock)), 1) + self.assertEqual(self._keepalive_calls(sock)[0][0][2], 1) + + def test_unsupported_options_are_ignored(self): + """A socket that refuses keepalive outright must not raise.""" + sock = mock.MagicMock() + sock.setsockopt.side_effect = OSError("unsupported") + sock.ioctl.side_effect = OSError("unsupported") + + # Must not raise. + shotgun._set_socket_keepalive(sock) + + # Nothing is tuned once the socket has rejected SO_KEEPALIVE. + sock.ioctl.assert_not_called() + self.assertEqual(len(self._keepalive_calls(sock)), 1) + + def test_windows_timers_tuned_via_ioctl(self): + """ + On Windows the timers are set with an ioctl rather than socket options. + SIO_KEEPALIVE_VALS is patched in so the branch runs on any platform. + """ + sock = mock.MagicMock() + with mock.patch.object( + socket, "SIO_KEEPALIVE_VALS", self.SIO_SENTINEL, create=True + ): + shotgun._set_socket_keepalive(sock) + + sock.ioctl.assert_called_once_with( + self.SIO_SENTINEL, + ( + 1, + shotgun.KEEPALIVE_IDLE_SECS * 1000, + shotgun.KEEPALIVE_INTERVAL_SECS * 1000, + ), + ) + # The POSIX socket options must not also be attempted. + self.assertEqual(len(self._tcp_option_calls(sock)), 0) + + def test_windows_ioctl_failure_is_ignored(self): + """Keepalive stays enabled even if the timers cannot be tuned.""" + sock = mock.MagicMock() + sock.ioctl.side_effect = OSError("unsupported") + with mock.patch.object( + socket, "SIO_KEEPALIVE_VALS", self.SIO_SENTINEL, create=True + ): + # Must not raise. + shotgun._set_socket_keepalive(sock) + + self.assertEqual(len(self._keepalive_calls(sock)), 1) + + def test_timers_tuned_via_socket_options(self): + """ + Off Windows the timers are socket options. SIO_KEEPALIVE_VALS is patched + out so the branch runs there too. + """ + sock = mock.MagicMock() + with mock.patch.object(socket, "SIO_KEEPALIVE_VALS", None, create=True): + shotgun._set_socket_keepalive(sock) + + sock.ioctl.assert_not_called() + # Which timers exist is platform dependent, but at least the idle timer + # is available everywhere this library is supported. + self.assertGreater(len(self._tcp_option_calls(sock)), 0) + + def test_rejected_timer_options_are_ignored(self): + """A platform that rejects the timers must still get keepalive.""" + sock = mock.MagicMock() + + def reject_tcp_options(level, option, value): + if level == socket.IPPROTO_TCP: + raise OSError("unsupported") + return None + + sock.setsockopt.side_effect = reject_tcp_options + with mock.patch.object(socket, "SIO_KEEPALIVE_VALS", None, create=True): + # Must not raise. + shotgun._set_socket_keepalive(sock) + + self.assertEqual(len(self._keepalive_calls(sock)), 1) + self.assertGreater(len(self._tcp_option_calls(sock)), 0) + + def test_http_connection_enables_keepalive(self): + sock = mock.MagicMock() + with mock.patch("socket.socket", return_value=sock), mock.patch( + "socket.getaddrinfo", return_value=self._addrinfo(80) + ): + conn = shotgun.KeepaliveHTTPConnection("server_path") + conn.connect() + + self.assertEqual(len(self._keepalive_calls(sock)), 1) + + def test_https_connection_enables_keepalive(self): + """ + For HTTPS the option lands on the SSL-wrapped socket, which delegates to + the underlying socket. + """ + wrapped = mock.MagicMock() + with mock.patch("socket.socket", return_value=mock.MagicMock()), mock.patch( + "socket.getaddrinfo", return_value=self._addrinfo(443) + ), mock.patch("ssl.SSLContext.wrap_socket", return_value=wrapped): + conn = shotgun.KeepaliveHTTPSConnection("server_path") + conn.connect() + + self.assertEqual(len(self._keepalive_calls(wrapped)), 1) + + def test_proxied_socket_enables_keepalive(self): + sock = mock.MagicMock() + proxy_info = api.lib.httplib2.ProxyInfo( + api.lib.httplib2.socks.PROXY_TYPE_HTTP, "proxy.example.com", 8080 + ) + with mock.patch.object( + api.lib.httplib2.socks, "socksocket", return_value=sock + ), mock.patch("socket.getaddrinfo", return_value=self._addrinfo(80)): + conn = shotgun.KeepaliveHTTPConnection("server_path", proxy_info=proxy_info) + conn.connect() + + self.assertEqual(len(self._keepalive_calls(sock)), 1) + + def test_keepalive_failure_does_not_break_connect(self): + """A socket that rejects keepalive must still yield a usable conn.""" + + def reject_keepalive(level, option, value): + # Leave httplib2's own TCP_NODELAY alone; rejecting that is + # pre-existing behaviour unrelated to keepalive. + if (level, option) == (socket.IPPROTO_TCP, socket.TCP_NODELAY): + return None + raise OSError("unsupported") + + sock = mock.MagicMock() + sock.setsockopt.side_effect = reject_keepalive + with mock.patch("socket.socket", return_value=sock), mock.patch( + "socket.getaddrinfo", return_value=self._addrinfo(80) + ): + conn = shotgun.KeepaliveHTTPConnection("server_path") + conn.connect() + + self.assertIs(conn.sock, sock) + + +class TestKeepaliveConnectionType(unittest.TestCase): + """ + Test that KeepaliveHttp injects the keepalive-enabled connection classes + into httplib2, so the bundled httplib2 needs no modification (SG-44724). + """ + + def _injected_for(self, uri): + """Return the connection_type KeepaliveHttp hands to httplib2.""" + http = shotgun.KeepaliveHttp() + with mock.patch.object( + shotgun.Http, "request", return_value=(mock.MagicMock(), b"") + ) as base: + http.request(uri) + return base.call_args[1]["connection_type"] + + def test_https_uses_keepalive_connection(self): + self.assertIs( + self._injected_for("https://server_path/x"), + shotgun.KeepaliveHTTPSConnection, + ) + + def test_http_uses_keepalive_connection(self): + self.assertIs( + self._injected_for("http://server_path/x"), + shotgun.KeepaliveHTTPConnection, + ) + + def test_unknown_scheme_is_left_to_httplib2(self): + """httplib2 should raise its own error rather than be handed a class.""" + self.assertIsNone(self._injected_for("ftp://server_path/x")) + + def test_explicit_connection_type_is_respected(self): + http = shotgun.KeepaliveHttp() + sentinel = shotgun.KeepaliveHTTPConnection + with mock.patch.object( + shotgun.Http, "request", return_value=(mock.MagicMock(), b"") + ) as base: + http.request("https://server_path/x", connection_type=sentinel) + self.assertIs(base.call_args[1]["connection_type"], sentinel) + + def test_shotgun_uses_keepalive_http(self): + sg = api.Shotgun("https://server_path", "script_name", "api_key", connect=False) + self.assertIsInstance(sg._get_connection(), shotgun.KeepaliveHttp) + + def test_connection_classes_are_httplib2_subclasses(self): + """httplib2 branches on the class to pick constructor arguments.""" + self.assertTrue( + issubclass( + shotgun.KeepaliveHTTPSConnection, + api.lib.httplib2.HTTPSConnectionWithTimeout, + ) + ) + self.assertTrue( + issubclass( + shotgun.KeepaliveHTTPConnection, + api.lib.httplib2.HTTPConnectionWithTimeout, + ) + ) + + +class TestMaxConnectionIdleEnvVar(unittest.TestCase): + """ + SHOTGUN_API_MAX_CONNECTION_IDLE lets operators tune or disable the idle + expiry without code changes (SG-44724). + """ + + def _make(self): + return api.Shotgun( + "http://server_path", "script_name", "api_key", connect=False + ) + + def test_default_is_60(self): + with mock.patch.dict(os.environ, {}, clear=False): + os.environ.pop("SHOTGUN_API_MAX_CONNECTION_IDLE", None) + self.assertEqual(self._make().config.max_connection_idle_secs, 60) + + def test_env_var_overrides_default(self): + with mock.patch.dict(os.environ, {"SHOTGUN_API_MAX_CONNECTION_IDLE": "15"}): + self.assertEqual(self._make().config.max_connection_idle_secs, 15) + + def test_env_var_zero_disables_expiry(self): + with mock.patch.dict(os.environ, {"SHOTGUN_API_MAX_CONNECTION_IDLE": "0"}): + sg = self._make() + self.assertEqual(sg.config.max_connection_idle_secs, 0) + self.assertFalse(sg._is_connection_stale()) + + def test_non_integer_env_var_raises(self): + with mock.patch.dict(os.environ, {"SHOTGUN_API_MAX_CONNECTION_IDLE": "banana"}): + self.assertRaises(ValueError, self._make) + + def test_negative_env_var_raises(self): + with mock.patch.dict(os.environ, {"SHOTGUN_API_MAX_CONNECTION_IDLE": "-5"}): + self.assertRaises(ValueError, self._make) + + +class TestKeepaliveAcrossRedirects(unittest.TestCase): + """ + httplib2 follows a redirect by calling self.request() again without + forwarding connection_type, so injecting it at the call site would lose + keepalive on the redirected connection. KeepaliveHttp overrides request(), + which catches those recursive calls too (SG-44724). + + Uses two loopback servers; no external network. + """ + + def setUp(self): + self.stop = threading.Event() + self.addCleanup(self.stop.set) + self.target_port = self._serve(self._target) + self.redirect_port = self._serve(self._redirect) + + def _serve(self, handler): + listener = socket.socket() + listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + listener.bind(("127.0.0.1", 0)) + listener.listen(4) + port = listener.getsockname()[1] + self.addCleanup(listener.close) + + def loop(): + while not self.stop.is_set(): + try: + conn, _ = listener.accept() + except OSError: + return + try: + conn.settimeout(5) + conn.recv(4096) + conn.sendall(handler()) + except OSError: + pass + finally: + conn.close() + + thread = threading.Thread(target=loop) + thread.daemon = True + thread.start() + return port + + def _target(self): + return ( + b"HTTP/1.1 200 OK\r\n" + b"Content-Length: 2\r\n" + b"Connection: close\r\n" + b"\r\n" + b"{}" + ) + + def _redirect(self): + # 302 to a different authority, which forces httplib2 to build a second + # connection -- the one that used to miss keepalive. + return ( + "HTTP/1.1 302 Found\r\n" + "Location: http://127.0.0.1:%d/target\r\n" + "Content-Length: 0\r\n" + "Connection: close\r\n" + "\r\n" % self.target_port + ).encode("ascii") + + def test_redirected_connection_is_keepalive_enabled(self): + http = shotgun.KeepaliveHttp() + response, _ = http.request( + "http://127.0.0.1:%d/start" % self.redirect_port, method="GET" + ) + + self.assertEqual(response["status"], "200") + # Both the original and the redirect target are cached; neither may be a + # plain httplib2 connection. + cached = list(http.connections.items()) + self.assertEqual(len(cached), 2, cached) + for key, conn in cached: + self.assertIsInstance(conn, shotgun.KeepaliveHTTPConnection, key) + + if __name__ == "__main__": unittest.main()