Artwork fetches: the deadline bounds the whole request, trickling servers included

Name resolution runs in a thread within the budget, a watchdog shuts the
socket at the deadline, and the body is read one receive at a time with the
remaining time as timeout. SteamGridDB goes through the same bounded fetch,
without redirects.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

tests/test_frame_android_library.py

@@ -189,6 +189,57 @@ class ArtworkTests(unittest.TestCase):
         with self.assertRaises(SourceError):
             art.fetch('file:///etc/passwd')
 
+    def trickle(self, head, seconds):
+        # A server that answers one byte every 20 ms, over a socketpair standing in for the network.
+        import socket
+        import threading
+        from apk_sources import _images
+        client, server = socket.socketpair()
+        def serve():
+            try:
+                server.recv(65536)
+                for byte in head + b'x' * 1000:
+                    server.sendall(bytes([byte]))
+                    time.sleep(0.02)
+            except OSError:
+                pass
+            finally:
+                server.close()
+        threading.Thread(target=serve, daemon=True).start()
+        public = [(2, 1, 6, '', ('93.184.216.34', 80))]
+        with patch.object(_images.socket, 'getaddrinfo', return_value=public), \
+                patch.object(_images.socket, 'create_connection', return_value=client):
+            start = time.monotonic()
+            with self.assertRaisesRegex(_images.SourceError, 'too long'):
+                _images.get('http://example.org/a.png', deadline=start + seconds)
+            return time.monotonic() - start
+
+    def test_deadline_bounds_trickling_headers_and_body(self):
+        self.assertLess(self.trickle(b'HTTP/1.1 200 OK\r\nContent-Length: 1000\r\n\r\n', 0.15), 0.4)
+        self.assertLess(self.trickle(b'HTTP/1.1 200 OK\r\n', 0.15), 0.4)  # headers never finish
+
+    def test_deadline_covers_name_resolution(self):
+        import threading
+        from apk_sources import _images
+        gate = threading.Event()
+        with patch.object(_images.socket, 'getaddrinfo', side_effect=lambda *a, **k: gate.wait(5) and []):
+            start = time.monotonic()
+            with self.assertRaisesRegex(_images.SourceError, 'too long'):
+                _images.get('https://example.org/a.png', deadline=start + 0.1)
+            self.assertLess(time.monotonic() - start, 0.4)
+        gate.set()
+        with self.assertRaisesRegex(_images.SourceError, 'too long'):
+            _images.get('https://example.org/a.png', deadline=time.monotonic() - 1)
+
+    def test_steamgriddb_uses_the_bounded_fetch_without_redirects(self):
+        import frame_steamgriddb as sgdb
+        from apk_sources import _images
+        with patch.object(_images, 'get', return_value=b'{"success": true, "data": [1]}') as get:
+            self.assertEqual(sgdb._get('/search/x', 'secret', time.monotonic() + 5), [1])
+        self.assertEqual(get.call_args.kwargs['redirects'], 0)
+        self.assertEqual(get.call_args.args[1]['Authorization'], 'Bearer secret')
+        self.assertLessEqual(get.call_args.kwargs['deadline'] - time.monotonic(), 5)
+
     def test_supplied_jpeg(self):
         data = (FIXTURES / 'icon.jpg').read_bytes()
         self.assertEqual(art.image_type(data), 'jpg')

ui/apk_sources/_images.py

@@ -65,53 +65,94 @@ def image_type(data):
 
 def fetch(url, redirects=3, deadline=None, limit=MAX_IMAGE):
     """deadline: time.monotonic() value by which the whole fetch, redirects included, must finish."""
+    data = get(url, {'Accept': 'image/png,image/jpeg,image/webp,image/gif'}, redirects, deadline, limit)
+    return data, image_type(data)
+
+
+def _resolve(host, port, timeout):
+    # getaddrinfo has no timeout of its own; a thread keeps a slow resolver inside the budget.
+    found = {}
+
+    def run():
+        try:
+            found['addresses'] = socket.getaddrinfo(host, port, type=socket.SOCK_STREAM)
+        except OSError as e:
+            found['error'] = e
+    worker = threading.Thread(target=run, daemon=True)
+    worker.start()
+    worker.join(timeout)
+    if 'error' in found:
+        raise found['error']
+    if 'addresses' not in found:
+        raise SourceError('Artwork download took too long')
+    return found['addresses']
+
+
+def get(url, headers=None, redirects=3, deadline=None, limit=MAX_IMAGE):
+    """GET a public HTTP(S) URL within an overall deadline (default 60 s), redirects included.
+
+    A watchdog shuts the socket at the deadline, so a server trickling bytes can't outlast it."""
+    deadline = time.monotonic() + 60 if deadline is None else deadline
+
+    def left():
+        remaining = deadline - time.monotonic()
+        if remaining <= 0:
+            raise SourceError('Artwork download took too long')
+        return remaining
     if not valid_url(url):
         raise SourceError('Artwork URL is not allowed')
-
-    def remaining():
-        if deadline is None:
-            return 10
-        left = deadline - time.monotonic()
-        if left <= 0:
-            raise SourceError('Artwork download took too long')
-        return min(10, left)
     p = urlsplit(url)
     port = p.port or (443 if p.scheme == 'https' else 80)
-    addresses = socket.getaddrinfo(p.hostname, port, type=socket.SOCK_STREAM)
+    addresses = _resolve(p.hostname, port, left())
     if not addresses or any(not ipaddress.ip_address(a[4][0]).is_global for a in addresses):
         raise SourceError('Private network artwork is not allowed')
     # Connect to the checked IP, never resolve again between validation and use.
-    sock = socket.create_connection((addresses[0][4][0], port), timeout=remaining())
+    live = [socket.create_connection((addresses[0][4][0], port), timeout=min(10, left()))]
+
+    def expire():
+        try:
+            live[0].shutdown(socket.SHUT_RDWR)
+        except OSError:
+            pass
+    watchdog = threading.Timer(left(), expire)
+    watchdog.daemon = True
+    watchdog.start()
     conn = http.client.HTTPConnection(p.hostname, port, timeout=10)
     try:
         if p.scheme == 'https':
-            sock = ssl.create_default_context().wrap_socket(sock, server_hostname=p.hostname)
-        conn.sock = sock
+            live[0] = ssl.create_default_context().wrap_socket(live[0], server_hostname=p.hostname)
+        conn.sock = live[0]
         path = p.path or '/'
         if p.query:
             path += '?' + p.query
-        conn.request('GET', path, headers={'User-Agent': 'FrameControl/0.3.1', 'Accept': 'image/png,image/jpeg,image/webp,image/gif'})
-        sock.settimeout(remaining())
+        conn.request('GET', path, headers={'User-Agent': 'FrameControl/0.3.1', **(headers or {})})
+        live[0].settimeout(min(10, left()))
         response = conn.getresponse()
         if response.status in (301, 302, 303, 307, 308) and redirects:
             target = urljoin(url, response.getheader('Location', ''))
             conn.close()
-            return fetch(target, redirects - 1, deadline, limit)
+            return get(target, headers, redirects - 1, deadline, limit)
         if response.status != 200:
             raise SourceError('Artwork is unavailable')
         data = b''
         while len(data) <= limit:
-            sock.settimeout(remaining())
-            chunk = response.read(min(65536, limit + 1 - len(data)))
+            live[0].settimeout(min(10, left()))
+            chunk = response.read1(min(16384, limit + 1 - len(data)))  # one receive at most
             if not chunk:
                 break
             data += chunk
         if len(data) > limit:
             raise SourceError('Artwork is too large')
-        return data, image_type(data)
+        left()
+        return data
+    except (OSError, http.client.HTTPException) as e:
+        if time.monotonic() >= deadline:
+            raise SourceError('Artwork download took too long') from e
+        raise
     finally:
+        watchdog.cancel()
         conn.close()
-        sock.close()
+        live[0].close()
 
 
 def remember(url, data):

ui/frame_steamgriddb.py

@@ -6,7 +6,6 @@ import tempfile
 import time
 import unicodedata
 import urllib.parse
-import urllib.request
 
 import frame_host
 
@@ -51,19 +50,11 @@ def save_settings(body):
 
 
 def _get(path, key, deadline=None):
-    timeout = 12 if deadline is None else min(12, deadline - time.monotonic())
-    if timeout <= 0:
-        raise ValueError('SteamGridDB lookup took too long')
-    request = urllib.request.Request(API + path, headers={'Authorization': 'Bearer ' + key,
-                                                         'User-Agent': 'FrameControl/1.0'})
-    # Do not carry the credential to redirects or include it in error messages.
-    class NoRedirect(urllib.request.HTTPRedirectHandler):
-        def redirect_request(self, *args, **kwargs):
-            return None
-    with urllib.request.build_opener(NoRedirect()).open(request, timeout=timeout) as response:
-        data = response.read(MAX_JSON + 1)
-    if len(data) > MAX_JSON:
-        raise ValueError('SteamGridDB response too large')
+    from apk_sources import _images
+    # No redirects: the credential never goes anywhere but the API. Errors never contain it.
+    data = _images.get(API + path, {'Authorization': 'Bearer ' + key, 'Accept': 'application/json'}, redirects=0,
+                       deadline=time.monotonic() + 12 if deadline is None else min(deadline, time.monotonic() + 12),
+                       limit=MAX_JSON)
     result = json.loads(data)
     if not isinstance(result, dict) or not result.get('success') or not isinstance(result.get('data'), list):
         raise ValueError('SteamGridDB lookup failed')