mirror of
https://github.com/saphid/frame-control.git
synced 2026-10-06 01:00:18 +02:00
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
3b36feff52
commit
cb05395280
3 files changed
+31
-3
No files matched your search
@@ -232,6 +232,17 @@ class ArtworkTests(unittest.TestCase):
|
|||||||
time.sleep(0.01)
|
time.sleep(0.01)
|
||||||
_images._resolvers.release() # the stuck lookups finished and gave their slots back
|
_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):
|
def test_deadline_covers_name_resolution(self):
|
||||||
import threading
|
import threading
|
||||||
from apk_sources import _images
|
from apk_sources import _images
|
||||||
@@ -287,6 +298,17 @@ class ArtworkTests(unittest.TestCase):
|
|||||||
with self.subTest(cut=cut), self.assertRaises(ValueError):
|
with self.subTest(cut=cut), self.assertRaises(ValueError):
|
||||||
art.gif_frame(one[:cut])
|
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('<HHBBB', 1, 1, 0x80, 0, 0) + b'\xff\xff\xff\x00\x00\x00'
|
||||||
|
good = b'\x21\xf9\x04\x00\x00\x00\x00\x00'
|
||||||
|
self.assertEqual(art.gif_frame(head + good + image + b'\x3b'), head + good + image + b'\x3b')
|
||||||
|
bad = b'\x21\xf9\x02\x00\x00\x00' # wrong payload size: dropped, not passed on
|
||||||
|
self.assertEqual(art.gif_frame(head + bad + image + b'\x3b'), head + image + b'\x3b')
|
||||||
|
empty = b'\x2c' + struct.pack('<HHHHB', 0, 0, 1, 1, 0) + b'\x02\x00'
|
||||||
|
with self.assertRaises(ValueError):
|
||||||
|
art.gif_frame(head + empty + b'\x3b')
|
||||||
|
|
||||||
def test_png_variants_left_to_chromium_and_limits(self):
|
def test_png_variants_left_to_chromium_and_limits(self):
|
||||||
def png(w, h, depth, color, interlace):
|
def png(w, h, depth, color, interlace):
|
||||||
return art.PNG + art.chunk(b'IHDR', struct.pack('>IIBBBBB', w, h, depth, color, 0, 0, interlace)) + \
|
return art.PNG + art.chunk(b'IHDR', struct.pack('>IIBBBBB', w, h, depth, color, 0, 0, interlace)) + \
|
||||||
|
|||||||
@@ -86,8 +86,12 @@ def _resolve(host, port, timeout):
|
|||||||
found['error'] = e
|
found['error'] = e
|
||||||
finally:
|
finally:
|
||||||
_resolvers.release()
|
_resolvers.release()
|
||||||
worker = threading.Thread(target=run, daemon=True)
|
try:
|
||||||
worker.start()
|
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)
|
worker.join(timeout)
|
||||||
if 'error' in found:
|
if 'error' in found:
|
||||||
raise found['error']
|
raise found['error']
|
||||||
|
|||||||
+3
-1
@@ -56,7 +56,7 @@ def gif_frame(data):
|
|||||||
raise ValueError('truncated GIF')
|
raise ValueError('truncated GIF')
|
||||||
if data[pos] == 0x21 and pos + 1 < len(data): # extension: keep the frame's graphic control
|
if data[pos] == 0x21 and pos + 1 < len(data): # extension: keep the frame's graphic control
|
||||||
end = _gif_blocks(data, pos + 2)
|
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]
|
control = data[pos:end]
|
||||||
pos = end
|
pos = end
|
||||||
elif data[pos] == 0x2c and pos + 10 <= len(data): # the first image
|
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
|
if pos >= len(data) or not 2 <= data[pos] <= 8: # the LZW minimum code size
|
||||||
raise ValueError('invalid GIF image data')
|
raise ValueError('invalid GIF image data')
|
||||||
end = _gif_blocks(data, pos + 1)
|
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'
|
return head + control + data[start:end] + b'\x3b'
|
||||||
else:
|
else:
|
||||||
raise ValueError('invalid GIF block')
|
raise ValueError('invalid GIF block')
|
||||||
|
|||||||
Reference in new issue
Block a user