diff --git a/pysim_simple_server/server.py b/pysim_simple_server/server.py index 1542245..8d99de4 100644 --- a/pysim_simple_server/server.py +++ b/pysim_simple_server/server.py @@ -2561,7 +2561,7 @@ def _ram_batch_new_state(plan): 'started': time.time(), 'stopped': None, 'plan': plan, 'session': None, 'seen': {}, 'attempts': [], 'attempts_total': 0, 'counts': {'installed': 0, 'failed': 0, 'skipped': 0}, - 'current': None, 'notice': '', 'thread': None} + 'current': None, 'notice': ''} def _ram_batch_record(state, rec): @@ -2699,8 +2699,10 @@ def _ram_batch_worker(state): break server = _server_ref session = getattr(server, 'card_session', 0) if server is not None else None - connected = bool(server is not None - and getattr(server, 'connected', False)) + # The connection state is the module global the whole server uses + # (_apply_equipped_card / _handle_card_disconnect); the server + # object itself has no `connected` attribute (v3.28.0 review). + connected = bool(server is not None and _CARD_CONNECTED) with _RAM_BATCH_LOCK: if state['stop']: break @@ -2726,7 +2728,6 @@ def _ram_batch_worker(state): state['current'] = None state['phase'] = 'stopped' if state['stop'] else 'finished' state['stopped'] = time.time() - state['thread'] = None phase, total = state['phase'], state['attempts_total'] sys.stderr.write('RAM-BATCH: %s after %d attempts\n' % (phase, total)) @@ -2745,10 +2746,8 @@ def _ram_batch_start(server, body): state = _ram_batch_new_state(plan) with _RAM_BATCH_LOCK: _RAM_BATCH = state - thread = threading.Thread(target=_ram_batch_worker, args=(state,), - name='ram-batch', daemon=True) - state['thread'] = thread - thread.start() + threading.Thread(target=_ram_batch_worker, args=(state,), + name='ram-batch', daemon=True).start() sys.stderr.write('RAM-BATCH: started (%d APDUs, loadfile_aid=%s)\n' % (len(plan['seq']), plan['loadfile_aid'])) return state @@ -11208,6 +11207,12 @@ class PysimHandler(BaseHTTPRequestHandler): self._send_json(resp, 400) self._log_resp(resp) return + tars = body.get('tars') + if tars is not None and not isinstance(tars, list): + resp = {'error': 'tars must be a list of TAR entries'} + self._send_json(resp, 400) + self._log_resp(resp) + return tar_keyset = body.get('tar_keyset') if tar_keyset in (None, ''): tar_keyset = None @@ -11245,7 +11250,7 @@ class PysimHandler(BaseHTTPRequestHandler): text, name=str(body.get('name') or ''), algorithm16=algorithm16, - tars=body.get('tars'), + tars=tars, tar_keyset=tar_keyset) except ValueError as e: resp = {'error': str(e)} diff --git a/tests/test_fetch_routes.py b/tests/test_fetch_routes.py index 2bacd4e..1bd602c 100644 --- a/tests/test_fetch_routes.py +++ b/tests/test_fetch_routes.py @@ -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'}) diff --git a/tests/test_ram_batch.py b/tests/test_ram_batch.py index 5a2a77c..6a778f1 100644 --- a/tests/test_ram_batch.py +++ b/tests/test_ram_batch.py @@ -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