catarrh/simple
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: follow-up test-script review findings (v3.5.14)

Browse Source 
- The SCP80 counter write-back resolves the preset by NAME first (the run
  snapshot prefers the name) and only falls back to an ICCID-looking
  value: presets with digits in their names are found after a page reload
  (the previous fallback ran cardsNormIccid on the name and gave up).
- testScriptProblem validates the *selected* SCP80 source (like the form
  and the server), so source=apdu with only sp filled is caught locally
  instead of failing with a server 400.
- _test_run_start cleans up (_TEST_RUNNING=False, run state error) when
  the worker thread cannot be created/started, and the endpoint answers
  500 with a clear error instead of leaving the card blocked behind a run
  that never started.
- The 5 s poll writes the counter back whenever a run is finished
  (idempotent), so a reloaded page saves it without visiting the pill.
- Mask wildcards ('?') are stripped from the hex fields that cannot carry
  a mask (APDU, secured packet, event/file data, TAR/SPI overrides, DCS,
  extra TLVs); check values keep them.

Tests: frontend +3 (write-back by name / by ICCID, mask-free fields,
source-aware script check), Python +1 (failed thread start unblocks).
586 frontend / 471 Python green; version 3.5.14; sw simple-v267.
This commit is contained in:
Антон Трошин
2026-09-27 01:52:03 +03:00
parent d1eb88d4d7
commit 2d2a40a73c
6 changed files with 115 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/index.html
+25 -15
View File
@@ -1616,7 +1616,7 @@
// ===== Version =====
// Single source of truth for the PWA version: shown in the header and used
// by the server version check in pysimConnect().
const SIMPLE_VERSION = '3.5.13';
const SIMPLE_VERSION = '3.5.14';
document.getElementById('app-version').textContent = 'v' + SIMPLE_VERSION;
// ===== Tab switching =====
@@ -6479,7 +6479,11 @@ function testScriptProblem(script) {
if (TEST_ACTION_KINDS.indexOf(s.kind) < 0) return at + t('unknown action kind');
const p = s.params || {};
if (s.kind === 'apdu' && !String(p.apdu || '').trim()) return at + t('APDU is empty');
if (s.kind === 'scp80' && !String(p.apdu || p.sp || '').trim()) return at + t('SCP80 needs an APDU or a secured packet');
if (s.kind === 'scp80') {
const src = p.source || (p.sp ? 'sp' : 'apdu');
const value = src === 'sp' ? p.sp : p.apdu;
if (!String(value || '').trim()) return at + t('SCP80 needs an APDU or a secured packet');
}
if (s.kind === 'menu-select' && !(p.item_id >= 1 && p.item_id <= 255)) return at + t('item id must be 1..255');
if ((s.kind === 'file-write' || s.kind === 'file-read') && !String(p.path || '').trim()) return at + t('file path is empty');
} else if (s.type === 'expect') {
@@ -6928,33 +6932,34 @@ function testStepCollect() {
if (!step) return;
const val = id => { const el = document.getElementById(id); return el ? el.value.trim() : ''; };
const hex = id => val(id).replace(/[^0-9a-fA-F?]/g, '').toUpperCase();
const hexNoMask = id => val(id).replace(/[^0-9a-fA-F]/g, '').toUpperCase();
const num = (id, dflt) => { const v = parseInt(val(id), 10); return isNaN(v) ? dflt : v; };
if (step.type === 'action') {
const kind = val('test-step-kind') || step.kind;
step.kind = kind;
const p = {};
if (kind === 'envelope') { p.event = num('test-f-event', 3); p.data = hex('test-f-data'); }
if (kind === 'envelope') { p.event = num('test-f-event', 3); p.data = hexNoMask('test-f-data'); }
else if (kind === 'menu-select') { p.item_id = num('test-f-item', 1); }
else if (kind === 'file-write' || kind === 'file-read') {
p.path = val('test-f-path');
p.mode = val('test-f-mode') || 'auto';
const rec = num('test-f-record', 0);
if (rec) p.record = rec;
if (kind === 'file-write') p.data = hex('test-f-fdata');
if (kind === 'file-write') p.data = hexNoMask('test-f-fdata');
}
else if (kind === 'apdu') { p.apdu = hex('test-f-apdu'); }
else if (kind === 'apdu') { p.apdu = hexNoMask('test-f-apdu'); }
else if (kind === 'scp80') {
// the source is stored explicitly: switching it must not drop the
// value of the other source (and must stick when the field is empty)
const prev = step.params || {};
const src = val('test-f-src') || prev.source || (prev.sp ? 'sp' : 'apdu');
// keep both values: the field of the inactive source is not rendered
if (document.getElementById('test-f-sp')) p.sp = hex('test-f-sp');
if (document.getElementById('test-f-sp')) p.sp = hexNoMask('test-f-sp');
else p.sp = prev.sp || '';
if (document.getElementById('test-f-apdu')) p.apdu = hex('test-f-apdu');
if (document.getElementById('test-f-apdu')) p.apdu = hexNoMask('test-f-apdu');
else p.apdu = prev.apdu || '';
p.source = src;
['tar', 'spi1', 'spi2'].forEach(k => { const v = hex('test-f-' + k); if (v) p[k] = v; });
['tar', 'spi1', 'spi2'].forEach(k => { const v = hexNoMask('test-f-' + k); if (v) p[k] = v; });
}
else if (kind === 'status') { p.attempts = num('test-f-attempts', 1); p.interval_ms = num('test-f-interval', 200); }
step.params = p;
@@ -6984,8 +6989,8 @@ function testStepCollect() {
const ri = num('test-f-ritem', 0);
if (ri) r.item_id = ri;
const text = val('test-f-rtext');
if (text) { r.text = text; r.dcs = hex('test-f-rdcs') || '00'; }
const raw = hex('test-f-rraw');
if (text) { r.text = text; r.dcs = hexNoMask('test-f-rdcs') || '00'; }
const raw = hexNoMask('test-f-rraw');
if (raw) r.raw = raw;
step.respond = r;
step.on_fail = val('test-f-fail') || 'error';
@@ -7176,10 +7181,14 @@ function testWriteBackCounter() {
const next = String(st.scp80_counter).toUpperCase();
let idx = _testLastPresetIdx;
if (idx < 0 && st.preset) {
// a run started elsewhere (or observed after a page reload): find the
// preset the snapshot names, by ICCID or by name
const norm = cardsNormIccid(st.preset);
idx = norm ? cardsFindByIccid(norm) : cards.findIndex(c => (c.name || '') === st.preset);
// A run started elsewhere (or observed after a page reload): the run
// snapshot prefers the preset name, so look that up first and only
// fall back to an ICCID-looking value.
idx = cards.findIndex(c => (c.name || '') === st.preset);
if (idx < 0) {
const norm = cardsNormIccid(st.preset);
if (norm && norm.length >= 5) idx = cardsFindByIccid(norm);
}
}
if (idx < 0) return;
const preset = cards[idx];
@@ -10962,7 +10971,8 @@ function pysimStartBackendPoll() {
_testRunState = ts;
if (wasRunning !== !!ts.running) pysimApplyAvailability();
if (isViewVisible('phone-sub-test')) testRenderRun(ts);
if (wasRunning && !ts.running) testWriteBackCounter();
// idempotent: saves the counter of a run finished elsewhere
if (!ts.running) testWriteBackCounter();
}
} catch (e) { /* ignore */ }
}, 5000);
frontend/sw.js
+1 -1
View File
@@ -1,4 +1,4 @@
const CACHE = 'simple-v266';
const CACHE = 'simple-v267';
const URLS = [
'index.html',
'help.html',
frontend/tests/testscripts.test.js
+53 -1
View File
@@ -43,9 +43,11 @@ eval(extractFunc(html, 'testFormSelect'));
eval(extractFunc(html, 'testStepRender'));
eval(extractFunc(html, 'testRenderChecks'));
eval(extractFunc(html, 'testStepCollect'));
eval(extractFunc(html, 'testWriteBackCounter'));
eval(extractFunc(html, 'testChecksCollect'));
eval('var _testEditStep = null; var _testEditChecks = []; var _testEditStepIndex = -1;'
+ ' var _testScripts = null; var _testCurrentIdx = -1; var _testRunState = null;');
+ ' var _testScripts = null; var _testCurrentIdx = -1; var _testRunState = null;'
+ ' var _testLastPresetIdx = -1;');
globalThis.localStorage = { getItem: () => null, setItem: () => {}, removeItem: () => {} };
function fakeForm(values) {
@@ -82,6 +84,13 @@ test('testScriptProblem accepts good scripts and names bad ones', () => {
assert.match(testScriptProblem({
steps: [{ type: 'action', kind: 'file-read', params: {} }] }), /file path/);
assert.match(testScriptProblem({ steps: [{ type: 'expect' }] }), /command is required/);
// the selected SCP80 source decides which value is required
assert.match(testScriptProblem({ steps: [{ type: 'action', kind: 'scp80',
params: { source: 'apdu', sp: 'AA' } }] }), /SCP80/);
assert.strictEqual(testScriptProblem({ steps: [{ type: 'action', kind: 'scp80',
params: { source: 'apdu', apdu: '80E2' } }] }), '');
assert.strictEqual(testScriptProblem({ steps: [{ type: 'action', kind: 'scp80',
params: { source: 'sp', sp: 'AA', apdu: '80E2' } }] }), '');
});
test('testStepSummary renders actions', () => {
@@ -185,6 +194,49 @@ test('the step form renders the chosen source and no pre-filled SW', () => {
assert.match(body, /id="test-f-sw" value=""/);
});
test('hex fields strip mask wildcards, check values keep them', () => {
_testEditStep = { type: 'action', kind: 'apdu', params: {} };
fakeForm({ 'test-step-kind': 'apdu', 'test-f-apdu': '80E2??',
'test-f-sw': '', 'test-f-cdata': '', 'test-f-fail': 'error' });
testStepCollect();
assert.strictEqual(_testEditStep.params.apdu, '80E2');
fakeForm({ 'test-step-kind': 'apdu', 'test-f-apdu': '80E2', 'test-f-sw': '91??',
'test-f-sw-mode': 'mask', 'test-f-cdata': '', 'test-f-fail': 'error' });
testStepCollect();
assert.deepStrictEqual(_testEditStep.check.sw, { mode: 'mask', value: '91??' });
});
test('the counter write-back finds the preset by name after a reload', () => {
let saved = 0;
globalThis.cards = [{ name: 'Card 1', iccid: '8970119000004600098', cntr: '0000000A' }];
globalThis.cardsSave = () => { saved++; };
globalThis.ioStatus = () => {};
// the name lookup must win: the ICCID helpers are not even called
globalThis.cardsNormIccid = () => { throw new Error('ICCID lookup for a name'); };
globalThis.cardsFindByIccid = () => { throw new Error('ICCID lookup for a name'); };
_testLastPresetIdx = -1;
_testRunState = { running: false, scp80_counter: '0000000B', preset: 'Card 1' };
testWriteBackCounter();
assert.strictEqual(cards[0].cntr, '0000000B');
assert.strictEqual(saved, 1);
testWriteBackCounter(); // idempotent
assert.strictEqual(saved, 1);
});
test('the counter write-back falls back to an ICCID snapshot', () => {
let saved = 0;
globalThis.cards = [{ name: 'Other', iccid: '8970119000004600098', cntr: '0000000A' }];
globalThis.cardsSave = () => { saved++; };
globalThis.ioStatus = () => {};
globalThis.cardsNormIccid = v => String(v).replace(/\D/g, '');
globalThis.cardsFindByIccid = v => (globalThis.cardsNormIccid(v) === '8970119000004600098' ? 0 : -1);
_testLastPresetIdx = -1;
_testRunState = { running: false, scp80_counter: '0000000C', preset: '8970119000004600098' };
testWriteBackCounter();
assert.strictEqual(cards[0].cntr, '0000000C');
assert.strictEqual(saved, 1);
});
test('the item text check offers contains/exact only', () => {
const els = {
'test-step-modal': { classList: { add: () => {}, remove: () => {} } },
pyproject.toml
+1 -1
View File
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
[project]
name = "pysim-simple-server"
version = "3.5.13"
version = "3.5.14"
description = "HTTP REST server wrapping pysim for the SIMple PWA"
requires-python = ">=3.8"
# pysim is a git-only dependency installed explicitly by setup.bat/setup.sh.
pysim_simple_server/server.py
+24 -6
View File
@@ -31,7 +31,7 @@ from osmocom.tlv import BER_TLV_IE
from osmocom.utils import rpad
VERSION = '3.5.13'
VERSION = '3.5.14'
MAX_ENVELOPE_SEGMENTS = 5 # max SMS segments for outgoing C-APDU in ENVELOPE
@@ -4598,10 +4598,22 @@ def _test_run_start(server, script, preset):
_finish_pending_menu(server, server.scc)
except Exception as e:
sys.stderr.write('TEST-RUN: finishing pending menu failed: %s\n' % e)
_TEST_THREAD = threading.Thread(target=_test_run_worker,
args=(server, script, preset),
name='test-script', daemon=True)
_TEST_THREAD.start()
try:
_TEST_THREAD = threading.Thread(target=_test_run_worker,
args=(server, script, preset),
name='test-script', daemon=True)
_TEST_THREAD.start()
except Exception:
# A run that never starts must not leave the card blocked (the 409
# guard keys off _TEST_RUNNING, and no worker would ever clear it).
_TEST_THREAD = None
_TEST_RUNNING = False
with _TEST_LOCK:
_TEST_RUN['running'] = False
_TEST_RUN['status'] = 'error'
_TEST_RUN['error'] = 'could not start the run worker'
_TEST_RUN['finished'] = time.time()
raise
class PysimHandler(BaseHTTPRequestHandler):
@@ -4931,7 +4943,13 @@ class PysimHandler(BaseHTTPRequestHandler):
self._send_json(resp, 400)
self._log_resp(resp)
return
_test_run_start(self.server, script, preset)
try:
_test_run_start(self.server, script, preset)
except Exception as e:
resp = {'error': 'could not start the test run: %s' % e}
self._send_json(resp, 500)
self._log_resp(resp)
return
resp = _test_state_snapshot()
self._send_json(resp)
self._log_resp(resp)
tests/test_testscript.py
+11
View File
@@ -316,6 +316,17 @@ class TestRunnerDialogue(RunnerTestCase):
self.assertEqual(sum(1 for a in scc.sent if a.startswith('80F2')), 3)
self.assertEqual(run['steps'][0]['sent'], 'STATUS x3')
def test_failed_thread_start_unblocks_the_card(self):
script = T.normalise_script({'steps': [
{'type': 'action', 'kind': 'status', 'params': {}}]}, S._test_command_type)
server = FakeServer(FakeScc())
with mock.patch.object(S.threading, 'Thread', side_effect=RuntimeError('no threads')):
with self.assertRaises(RuntimeError):
S._test_run_start(server, script, {})
self.assertFalse(S._TEST_RUNNING)
self.assertFalse(S._TEST_RUN['running'])
self.assertEqual(S._TEST_RUN['status'], 'error')
def test_stop_before_the_first_step(self):
run = self.run_script(FakeServer(FakeScc()), [
{'type': 'action', 'kind': 'status', 'params': {}},