v3.28.0 audit: the batch worker's connection check
The v3.26.0-v3.28.0 audit found the worker reading getattr(server, 'connected', False) - an attribute the server object has never had. The connection state is the module global _CARD_CONNECTED (set by __main__ / _apply_equipped_card / _handle_card_disconnect and read by /api/status the same way), so the check was always False and a batch never acted on an equipped card: it would wait forever. The unit test hid it - the fake server invented a `connected` attribute the real object does not have. - The worker reads _CARD_CONNECTED. The fake server carries only attributes the real object has, the worker test patches _CARD_CONNECTED (verified to fail on the old line), and a source pin keeps the worker off server.connected. - The write-only `thread` field of the batch state is gone (nothing read it). - The outfile endpoint refuses a non-list `tars` with 400 instead of silently falling back to the role defaults. 915 frontend + 922 Python; sw cache simple-v438 (unchanged).
This commit is contained in:
@@ -955,6 +955,11 @@ class PresetStoreHttpTests(unittest.TestCase):
|
||||
status, resp = self._post('/api/presets/import-outfile', {
|
||||
'text': text, 'tar_keyset': 99})
|
||||
self.assertEqual(status, 400, resp)
|
||||
# a non-list TAR table is the caller's error, never a silent fallback
|
||||
status, resp = self._post('/api/presets/import-outfile', {
|
||||
'text': text, 'tars': 'nope'})
|
||||
self.assertEqual(status, 400, resp)
|
||||
self.assertIn('tars', resp['error'])
|
||||
# a misspelled algorithm or mode is refused, never silently defaulted
|
||||
status, resp = self._post('/api/presets/import-outfile', {
|
||||
'text': text, 'algorithm16': 'aes-256'})
|
||||
|
||||
+21
-5
@@ -57,8 +57,12 @@ def _preset(iccid, name='Card', keyset=1):
|
||||
'tars': [tar]}
|
||||
|
||||
|
||||
def _fake_server(iccid, store, connected=True, session=1):
|
||||
return SimpleNamespace(iccid=iccid, card_presets=store, connected=connected,
|
||||
def _fake_server(iccid, store, session=1):
|
||||
"""A server-shaped object carrying only the attributes the real server
|
||||
object has. The connection state is the module global `_CARD_CONNECTED`
|
||||
(v3.28.0 review: a fake `connected` attribute here hid a bug - the worker
|
||||
read it and never saw a card)."""
|
||||
return SimpleNamespace(iccid=iccid, card_presets=store,
|
||||
card_session=session, scc=object())
|
||||
|
||||
|
||||
@@ -220,10 +224,11 @@ class BatchCardTests(unittest.TestCase):
|
||||
preset = _preset('8970119000004600098', 'Alfa')
|
||||
srv._RAM_BATCH = st
|
||||
srv._server_ref = _fake_server('8970119000004600098', _FakeStore([preset]),
|
||||
connected=True, session=7)
|
||||
session=7)
|
||||
calls = []
|
||||
with mock.patch.object(srv, '_ram_install_run',
|
||||
lambda *a, **k: calls.append(1) or {'success': True}):
|
||||
with mock.patch.object(srv, '_CARD_CONNECTED', True), \
|
||||
mock.patch.object(srv, '_ram_install_run',
|
||||
lambda *a, **k: calls.append(1) or {'success': True}):
|
||||
t = threading.Thread(target=srv._ram_batch_worker, args=(st,), daemon=True)
|
||||
t.start()
|
||||
for _ in range(60):
|
||||
@@ -262,6 +267,17 @@ class RamInstallPlanTests(unittest.TestCase):
|
||||
srv._ram_install_plan({'cap_hex': ''})
|
||||
|
||||
|
||||
class BatchWorkerSourceTests(unittest.TestCase):
|
||||
def test_the_worker_reads_the_module_connection_state(self):
|
||||
"""The server object has no `connected` attribute - the worker must
|
||||
read `_CARD_CONNECTED`, the global the rest of the server uses
|
||||
(v3.28.0 review)."""
|
||||
import inspect
|
||||
src = inspect.getsource(srv._ram_batch_worker)
|
||||
self.assertIn('_CARD_CONNECTED', src)
|
||||
self.assertNotIn("'connected'", src)
|
||||
|
||||
|
||||
class BatchLifecycleTests(unittest.TestCase):
|
||||
def setUp(self):
|
||||
srv._RAM_BATCH = None
|
||||
|
||||
Reference in New Issue
Block a user