From cb05395280dc4004f665cf42a905e52e0125c3bf Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:48:20 +1000 Subject: [PATCH] Artwork fetch: release the DNS slot if its thread can't start; stricter GIF control blocks Final review follow-up. A failed Thread.start() leaked a resolver slot (four failures disabled artwork lookups). GIF graphic-control blocks must have the fixed 4-byte payload (otherwise dropped) and an image with no pixel data is rejected. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_frame_android_library.py | 22 ++++++++++++++++++++++ ui/apk_sources/_images.py | 8 ++++++-- ui/frame_artwork.py | 4 +++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/tests/test_frame_android_library.py b/tests/test_frame_android_library.py index 1695d4c..e7f31a8 100644 --- a/tests/test_frame_android_library.py +++ b/tests/test_frame_android_library.py @@ -232,6 +232,17 @@ class ArtworkTests(unittest.TestCase): time.sleep(0.01) _images._resolvers.release() # the stuck lookups finished and gave their slots back + def test_resolver_slot_released_when_thread_cannot_start(self): + from apk_sources import _images + with patch.object(_images.threading.Thread, 'start', side_effect=RuntimeError("can't start new thread")): + for _ in range(6): + with self.assertRaises(RuntimeError): + _images.get('https://example.org/a.png', deadline=time.monotonic() + 1) + for _ in range(4): # every slot came back + self.assertTrue(_images._resolvers.acquire(blocking=False)) + for _ in range(4): + _images._resolvers.release() + def test_deadline_covers_name_resolution(self): import threading from apk_sources import _images @@ -287,6 +298,17 @@ class ArtworkTests(unittest.TestCase): with self.subTest(cut=cut), self.assertRaises(ValueError): art.gif_frame(one[:cut]) + def test_gif_control_block_and_empty_image(self): + image = self.FRAME[8:] # the image without its graphic control block + head = b'GIF89a' + struct.pack('IIBBBBB', w, h, depth, color, 0, 0, interlace)) + \ diff --git a/ui/apk_sources/_images.py b/ui/apk_sources/_images.py index 2a2541e..da89453 100644 --- a/ui/apk_sources/_images.py +++ b/ui/apk_sources/_images.py @@ -86,8 +86,12 @@ def _resolve(host, port, timeout): found['error'] = e finally: _resolvers.release() - worker = threading.Thread(target=run, daemon=True) - worker.start() + try: + worker = threading.Thread(target=run, daemon=True) + worker.start() + except BaseException: + _resolvers.release() # the worker never ran, so it can't release its slot + raise worker.join(timeout) if 'error' in found: raise found['error'] diff --git a/ui/frame_artwork.py b/ui/frame_artwork.py index ce1db52..7977456 100644 --- a/ui/frame_artwork.py +++ b/ui/frame_artwork.py @@ -56,7 +56,7 @@ def gif_frame(data): raise ValueError('truncated GIF') if data[pos] == 0x21 and pos + 1 < len(data): # extension: keep the frame's graphic control end = _gif_blocks(data, pos + 2) - if data[pos + 1] == 0xf9: + if data[pos + 1] == 0xf9 and end - pos == 8 and data[pos + 2] == 4: # GIF89a: fixed 4-byte payload control = data[pos:end] pos = end elif data[pos] == 0x2c and pos + 10 <= len(data): # the first image @@ -70,6 +70,8 @@ def gif_frame(data): if pos >= len(data) or not 2 <= data[pos] <= 8: # the LZW minimum code size raise ValueError('invalid GIF image data') end = _gif_blocks(data, pos + 1) + if end - pos <= 2: # code size then the terminator: no pixels at all + raise ValueError('invalid GIF image data') return head + control + data[start:end] + b'\x3b' else: raise ValueError('invalid GIF block')