From 93fb694e25068ec9c71f5fdf2e8abd4e9dbd6a12 Mon Sep 17 00:00:00 2001 From: thatsatechnique <28403172+thatsatechnique@users.noreply.github.com> Date: Wed, 4 Mar 2026 14:25:15 -0800 Subject: [PATCH] fix(ook): address code review findings from Copilot PR review - Fix XSS: escape ASCII output in innerHTML via escapeHtml() - Fix deadlock: use put_nowait() for queue ops under ook_lock - Fix SSE leak: add ook to moduleDestroyMap so switching modes closes the EventSource - Fix RSSI: explicit null check preserves valid zero values in JSON export - Add frame cap: trim oldest frames at 5000 to prevent unbounded memory growth on busy bands - Validate timing params: wrap int() casts in try/except, return 400 instead of 500 on invalid input - Fix PWM hint: correct to short=0/long=1 matching rtl_433 OOK_PWM convention (UI, JS hints, and cheat sheet) - Fix inversion docstring: clarify fallback only applies when primary hex parse fails, not for valid decoded frames Co-Authored-By: Claude Opus 4.6 --- routes/ook.py | 25 +++++++++++++++++-------- static/js/core/cheat-sheets.js | 2 +- static/js/modes/ook.js | 14 +++++++++++--- templates/index.html | 1 + templates/partials/modes/ook.html | 2 +- utils/ook.py | 8 +++++--- 6 files changed, 36 insertions(+), 16 deletions(-) diff --git a/routes/ook.py b/routes/ook.py index 2fb6bdb..87c209d 100644 --- a/routes/ook.py +++ b/routes/ook.py @@ -75,12 +75,15 @@ def start_ook() -> Response: return jsonify({'status': 'error', 'message': str(e)}), 400 # OOK flex decoder timing parameters - short_pulse = int(data.get('short_pulse', 300)) - long_pulse = int(data.get('long_pulse', 600)) - reset_limit = int(data.get('reset_limit', 8000)) - gap_limit = int(data.get('gap_limit', 5000)) - tolerance = int(data.get('tolerance', 150)) - min_bits = int(data.get('min_bits', 8)) + try: + short_pulse = int(data.get('short_pulse', 300)) + long_pulse = int(data.get('long_pulse', 600)) + reset_limit = int(data.get('reset_limit', 8000)) + gap_limit = int(data.get('gap_limit', 5000)) + tolerance = int(data.get('tolerance', 150)) + min_bits = int(data.get('min_bits', 8)) + except (ValueError, TypeError) as e: + return jsonify({'status': 'error', 'message': f'Invalid timing parameter: {e}'}), 400 deduplicate = bool(data.get('deduplicate', False)) rtl_tcp_host = data.get('rtl_tcp_host') or None @@ -195,7 +198,10 @@ def start_ook() -> Response: app_module.ook_process._stop_parser = stop_event app_module.ook_process._parser_thread = parser_thread - app_module.ook_queue.put({'type': 'status', 'status': 'started'}) + try: + app_module.ook_queue.put_nowait({'type': 'status', 'status': 'started'}) + except queue.Full: + logger.warning("OOK 'started' status dropped — queue full") return jsonify({ 'status': 'started', @@ -244,7 +250,10 @@ def stop_ook() -> Response: app_module.release_sdr_device(ook_active_device) ook_active_device = None - app_module.ook_queue.put({'type': 'status', 'status': 'stopped'}) + try: + app_module.ook_queue.put_nowait({'type': 'status', 'status': 'stopped'}) + except queue.Full: + logger.warning("OOK 'stopped' status dropped — queue full") return jsonify({'status': 'stopped'}) return jsonify({'status': 'not_running'}) diff --git a/static/js/core/cheat-sheets.js b/static/js/core/cheat-sheets.js index f758078..bed4336 100644 --- a/static/js/core/cheat-sheets.js +++ b/static/js/core/cheat-sheets.js @@ -34,7 +34,7 @@ const CheatSheets = (function () { description: 'Decodes raw On-Off Keying (OOK) signals via rtl_433 flex decoder. Captures frames with configurable pulse timing and displays raw bits, hex, and ASCII — useful for reverse-engineering unknown ISM-band protocols.', whatToExpect: 'Decoded bit sequences, hex payloads, and ASCII interpretation. Each frame shows bit count, timestamp, and optional RSSI.', tips: [ - 'Identifying modulation — PWM: pulse widths vary (short=1, long=0), gaps constant — most common for ISM remotes/sensors. PPM: pulses constant, gap widths encode data. Manchester: self-clocking, equal-width pulses, data in transitions.', + 'Identifying modulation — PWM: pulse widths vary (short=0, long=1), gaps constant — most common for ISM remotes/sensors. PPM: pulses constant, gap widths encode data. Manchester: self-clocking, equal-width pulses, data in transitions.', 'Finding pulse timing — Run rtl_433 -f 433.92M -A in a terminal to auto-analyze signals. It prints detected pulse widths (short/long) and gap timings. Use those values in the Short/Long Pulse fields.', 'Common ISM timings — 300/600µs (weather stations, door sensors), 400/800µs (car keyfobs), 500/1500µs (garage doors, doorbells), 500µs Manchester (tire pressure monitors).', 'Frequencies to try — 315 MHz (North America keyfobs), 433.920 MHz (global ISM), 868 MHz (Europe ISM), 915 MHz (US ISM/meters).', diff --git a/static/js/modes/ook.js b/static/js/modes/ook.js index e48edb9..8221f4a 100644 --- a/static/js/modes/ook.js +++ b/static/js/modes/ook.js @@ -10,6 +10,7 @@ var OokMode = (function () { 'use strict'; var DEFAULT_FREQ_PRESETS = ['433.920', '315.000', '868.000', '915.000']; + var MAX_FRAMES = 5000; var state = { running: false, @@ -162,6 +163,13 @@ var OokMode = (function () { state.frames.push(msg); state.frameCount++; + // Trim oldest frames when buffer exceeds cap + if (state.frames.length > MAX_FRAMES) { + state.frames.splice(0, state.frames.length - MAX_FRAMES); + var panel = document.getElementById('ookOutput'); + if (panel && panel.firstChild) panel.removeChild(panel.firstChild); + } + var countEl = document.getElementById('ookFrameCount'); if (countEl) countEl.textContent = state.frameCount + ' frames'; var barEl = document.getElementById('ookStatusBarFrames'); @@ -237,7 +245,7 @@ var OokMode = (function () { '' + '
' + '' + - 'ascii: ' + interp.ascii + + 'ascii: ' + (typeof escapeHtml === 'function' ? escapeHtml(interp.ascii) : interp.ascii) + ''; div.style.cssText = 'font-size:11px; padding: 4px 0; border-bottom: 1px solid #1a1a1a; line-height:1.6;'; @@ -328,7 +336,7 @@ var OokMode = (function () { return { timestamp: msg.timestamp, bit_count: msg.bit_count, - rssi: msg.rssi || null, + rssi: (msg.rssi !== undefined && msg.rssi !== null) ? msg.rssi : null, hex: interp.hex, ascii: interp.ascii, inverted: msg.inverted, @@ -382,7 +390,7 @@ var OokMode = (function () { // Update timing hint var hints = { - pwm: 'Short pulse = 1, long pulse = 0. Most common for ISM OOK.', + pwm: 'Short pulse = 0, long pulse = 1. Most common for ISM OOK.', ppm: 'Short gap = 0, long gap = 1. Pulse position encoding.', manchester: 'Rising edge = 1, falling edge = 0. Self-clocking.', }; diff --git a/templates/index.html b/templates/index.html index cfbd143..8538606 100644 --- a/templates/index.html +++ b/templates/index.html @@ -4107,6 +4107,7 @@ vdl2: () => { if (vdl2MainEventSource) { vdl2MainEventSource.close(); vdl2MainEventSource = null; } }, radiosonde: () => { if (radiosondeEventSource) { radiosondeEventSource.close(); radiosondeEventSource = null; } }, meteor: () => typeof MeteorScatter !== 'undefined' && MeteorScatter.destroy?.(), + ook: () => typeof OokMode !== 'undefined' && OokMode.destroy?.(), }; return moduleDestroyMap[mode] || null; } diff --git a/templates/partials/modes/ook.html b/templates/partials/modes/ook.html index d28d995..eb2919d 100644 --- a/templates/partials/modes/ook.html +++ b/templates/partials/modes/ook.html @@ -57,7 +57,7 @@

- Short pulse = 1, long pulse = 0. Most common for ISM OOK. + Short pulse = 0, long pulse = 1. Most common for ISM OOK.

diff --git a/utils/ook.py b/utils/ook.py index f82cdd5..c4343ca 100644 --- a/utils/ook.py +++ b/utils/ook.py @@ -77,9 +77,11 @@ def ook_parser_thread( """Thread function: reads rtl_433 JSON output and emits OOK frame events. Handles the three rtl_433 hex-output field names (``codes``, ``code``, - ``data``) and falls back to bit-inverted parsing when the primary hex - parse produces no result — needed for transmitters that swap the - short/long pulse mapping. + ``data``) and, if the initial hex decoding fails, retries with an + inverted bit interpretation. This inversion fallback is only applied + when the primary parse yields no usable hex; it does not attempt to + reinterpret successfully decoded frames that merely swap the short/long + pulse mapping. Args: rtl_stdout: rtl_433 stdout pipe.