mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 06:15:09 +00:00
main
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 ChuckandClaude Opus 5.5
|
f6c0fe55d9 |
fix(core): font zip cache, monotonic timers, resolver back-off, and other core/common fixes (#654)
* fix(core): font zip cache, monotonic timers, resolver back-off, and other core/common fixes - font_manager: a .zip font URL is served as its extracted font after a restart (the cached-file check returned the archive first); downloads use requests with a 30s timeout into a temp file + os.replace. - api_helper / sync_manager: rate-limit and heartbeat/leader timeouts use time.monotonic(); last_request_time and the status file's ts stay wall-clock. set_on_new_cycle docstring no longer claims core uses it. - logo_helper: the placeholder uses the same scaled box as a real logo. - permission_utils: one _sudo_bash_candidates() helper (with the sudoers exact-argv rationale) shared by sudo_remove_directory, which now retries the next bash path on a sudo refusal, and install_requirements_file. - dynamic_team_resolver: failed/empty fetch backs off 5 min; duplicate INFO log and contradictory docstring example fixed. - element_style: scale default looked up through element aliases. - background_data_service: cache-hit callback runs outside the lock. - config_arrays: union-aware type check (["array","null"]); stale dotToNested() reference removed. - auto_update_setup: non-dict auto_update reads as off; temp result file unlinked when the write fails. - exceptions: constructors copy the caller's context dict. - logging_config: StructuredFormatter json.dumps(default=str). - error_aggregator: removed unused export_path/export_to_file/_auto_export. - Docstrings: validate_file_upload max_size_mb, raise_on_errors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(sync): retry the status-file rename like the other atomic writers On Windows os.replace can fail with "Access is denied" while a scanner briefly holds the target open; config_manager_atomic._replace already retries that (and re-raises at once on other platforms). The sync status writer called os.replace directly, which made test_concurrent_writers_each_use_their_own_temp_file flaky on Windows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
ChuckandClaude Opus 5.5
|
3f8edf5113 |
fix(logos): one hardened logo download path; shared, real HTTP headers (#612)
* fix(logos): harden the plugin logo download and share core HTTP headers download_missing_logo / LogoDownloader.download_logo, the path the scoreboard plugins use, read response.content with no size cap and wrote straight to the final path, so a failed or corrupt download could be left in place and cached as the logo. It now goes through fetch_logo: streamed with a 10 MB cap, image/* only, decoded by Pillow, converted to RGBA once, and moved into place atomically. A failure leaves no partial or temp file and keeps any logo already on disk. LogoHelper._download_logo delegates to the same code. Public signatures and return values are unchanged; saved files are pixel-identical to before (RGBA, palette+tRNS, L+tRNS, LA, JPEG). download_missing_logo reuses one downloader per thread instead of a new Session per logo. Per thread rather than behind a lock: Session is not documented thread-safe, and a lock would serialise every plugin's downloads behind the slowest one. Placeholders are written atomically, without the test_write.tmp probe. The logo downloader and background data service now send the real ChuckBuilds User-Agent from src.common.api_helper (USER_AGENT, DEFAULT_HTTP_HEADERS) instead of a yourusername/contact@example.com placeholder, and no longer hand-set Accept-Encoding: br (brotli is not installed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(http): drop APIHelper's hand-set brotli encoding; LogoHelper sends the real UA APIHelper advertised `br` though brotli isn't installed, so a server that honoured it would send a body requests can't decode. LogoHelper sent a bare `LEDMatrix-Common/1.0`, the kind of User-Agent ESPN has been rejecting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
ChuckandClaude Opus 5
|
28bc79566f |
fix(logo): remember a missing logo instead of re-warning every rotation (#548)
* fix(logo): remember a missing logo instead of re-warning every rotation load_logo() stat'd the path and logged a WARNING on every call, and the positive cache never covered it because a miss returns None and caches nothing. A file that is simply not there therefore produced one warning per rotation for as long as the process ran -- measured on a live rig at 114 lines in 24 hours for a single missing ticker icon, for a file nobody was going to add. Misses are now remembered for 10 minutes: warn once, then return None without touching the disk. Bounded rather than permanent because logo_downloader writes logos at runtime, so a file that appears later must still be picked up without a restart. Downloads through load_logo_with_download() clear the entry outright -- load_logo() consults the miss record before it stats the disk, so without that a freshly downloaded logo would stay invisible for the whole window. This is in the core rather than in ledmatrix-stocks, where it was found, so every plugin that goes through LogoHelper gets it. _cache_order stays a list. Swapping the pair for an OrderedDict would shave an O(n) scan per cache hit, but n is capped at cache_size (100 by default) and test_logo_helper.py pins the current structure; not worth the churn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * fix(logo): make the miss TTL longer than the rotation it is meant to outlast Deployed the previous commit to a live rig and measured it: no change at all. "Logo not found for VOO" stayed at ~6 lines an hour, exactly the baseline. The TTL was 600s and the display rotation is ~618s, so every recheck expired just as the plugin came round again and the negative cache never once got to suppress a warning. The fix was correct in shape and useless in practice, which only measuring on the rig would show. An hour instead. That is safe because the TTL is not the main way an entry clears: load_logo_with_download() drops it the moment a download succeeds and clear_cache() drops all of them. The TTL only covers a file that appeared some other way -- someone copying one in by hand -- and waiting up to an hour for that, or restarting, is a fair trade for not re-warning about a file nobody is going to add. The general lesson is in the comment: a TTL has to be long relative to the loop that does the asking, not merely "a while". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
ChuckandClaude Opus 5
|
92f9d06af9 |
fix(logos): stop a failed download pinning a team to a grey box forever (#512)
* fix(logos): stop a failed download pinning a team to a grey box forever When a logo download fails, create_placeholder_logo writes a 64x64 grey PNG under the *real* logo's filename. Every later call then hits `if filepath.exists(): return True` and reports success, so the real logo is never attempted again. One transient failure -- no network at boot, ESPN blipping -- permanently costs that team its logo. This is not hypothetical. Five of the eleven cached AFL logos in my checkout were 384-byte stubs written in a single bad minute, and they had stayed that way ever since; the scoreboard rendered COLL, FRE, NMFC, PORT and SYD as grey text boxes on every card. Placeholders are now stamped with a `ledmatrix_placeholder` PNG text chunk carrying their creation time, and `is_placeholder_logo` recognises them. It also matches on the placeholder's exact geometry and background colour, so the stubs already sitting on users' disks are picked up too -- without that, this fix would only help teams whose logos break in future. Verified against the real stubs: all five detected, all six real logos untouched. `download_missing_logo` now treats an existing placeholder as the failed download it is and retries, rather than as a satisfied request. The retry is rate-limited to PLACEHOLDER_RETRY_SECONDS (6h) so this does not trade a permanent grey box for an ESPN request every frame; a failed retry rewrites the placeholder, restarting the clock. The age comes from the stamp rather than mtime, so a backup restore, an rsync, or a permissions script cannot silently reset it. `download_missing_logos_for_league` gets the same treatment -- a bulk pass is exactly where a previously failed logo should get another chance -- and `LogoHelper.load_logo_with_download` no longer accepts a stale placeholder as a cache hit. That import is lazy and guarded so the module still works against a core build predating the marker. `LogoHelper._create_placeholder_logo` needs no change: it returns an in-memory image and never writes it to disk, which is the behaviour this bug argues for. Tests cover marked and legacy-unmarked detection, the two false-positive cases (a real 500x500 logo, and a 64x64 image that is merely the same size), the retry, the rate limit, and that the age survives an mtime touch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(logos): address review — unify eligibility, invalidate cache, restart back-off Three findings from the review on #512, all confirmed against the code: 1. The three download sites each had their own idea of "already have it". download_missing_logos_for_league() retried *any* placeholder, ignoring the back-off entirely, while download_all_ncaa_football_logos() was never updated and still skipped placeholders forever. They now share one should_attempt_download(), which also covers force_download, so the sites cannot drift apart again. download_missing_logo() reads through the same helper. 2. LogoHelper.load_logo_with_download() answered from the in-memory cache before touching the disk, so after a stale placeholder was successfully replaced the *cached placeholder image* was still returned -- the real logo would not have appeared until the process restarted. The cache entry for that file (every size of it) is now dropped after a successful download. 3. A failed retry left the stale placeholder on disk with its old timestamp, so the next call saw it as stale again and retried immediately: a download attempt per call, which is precisely what the back-off exists to prevent. refresh_placeholder_timestamp() restamps it, and the helper calls that on the failure path. It refuses to touch anything that is not a placeholder. Tests cover both bulk loops in both directions (fresh placeholder skipped, stale one retried), the eligibility rule including force_download, the timestamp refresh, and the two LogoHelper paths -- including that a freshly-downloaded logo is actually what comes back rather than the cached placeholder. Two of the new bulk-loop tests initially passed for the wrong reason: the fetch_teams_data stub returned {}, which is falsy, so the loops bailed before reaching the eligibility check at all. Fixed to return a truthy payload. Re-verified end to end: with both halves in place, rendering the AFL scoreboard took FRE.png from a 362-byte stub to a 12,928-byte logo. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
ChuckandClaude Opus 5
|
10e75b977f |
Cover the next tier of untested modules and endpoints, and fix the 43 bugs that surfaced (#459)
* test(sync): cover the display sync protocol, and fix what that surfaced DisplaySyncManager had no tests at all — it appeared in the suite only as a MagicMock() stand-in, so none of its framing, handshake, or socket handling was ever exercised. Writing that coverage surfaced three bugs. Both receive loops caught the generic Exception and immediately retried. A socket left in a bad state raises on every call, so the thread spun at 100% CPU logging the same line; the reverted-code run of the new regression test takes 24 seconds where the fixed one takes 0.2. Both now back off briefly before retrying. The follower dispatched on `data[:8] == _RAW_MAGIC or len(data) > 512`. That size threshold is not part of either wire format: a control message over 512 bytes — a hello_ack carrying a long incompatibility error, for instance — went to the image decoder and was dropped, and a raw frame under 512 bytes went to the JSON parser. Both formats are already self-describing, so dispatch on the magic prefix and treat a JSON parse failure as the legacy unmarked PNG, with the shared frame bookkeeping factored into _handle_received_frame(). _oversized_frame_warned was created on first use through getattr(self, ..., False) rather than in __init__, alone among the instance attributes. 75 tests: role parsing, the hello compatibility matrix, watchdog timeouts, both receive loops, the TCP image server's length and dimension caps and decompression-bomb guard, status shape per role, and one end-to-end loopback handshake so the wire format is exercised for real and not only against mocks. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(logos): cover LogoHelper, and stop bad downloads poisoning the cache Nothing in test/ referenced logo_helper.py, so its caching, resizing and download-fallback logic was entirely unexercised. Two bugs surfaced. _download_logo wrote response.content to disk with no size limit and no check that the bytes were an image. A logo URL is remote input, so the response chose how much went into the assets directory; worse, an undecodable one stayed there, and because load_logo() only reports the decode failure and returns None, every later call re-read the same corrupt file. The download path never retried, so a single bad response made a logo permanently blank rather than falling back to the placeholder. Cap the response, verify it decodes, and delete it if not, which lets the existing fallback in load_logo_with_download do its job. get_cache_stats() divided by self.cache_size with no guard, so a helper built with cache_size=0 raised ZeroDivisionError from what is only a stats call. 37 tests: size-qualified cache keys, LRU eviction and refresh, the four load_logo_with_download paths, download permissions and timeout, placeholder generation, and the abbreviation normalizer — including a test pinning its deliberate divergence from LogoDownloader.normalize_abbreviation, since logo filenames on existing installs depend on both behaviors staying put. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(web): cover the error and response builders, and stop dropping empty values errors.py and error_handler.py's response builders had no direct tests, though every API response passes through them. Two bugs surfaced. WebInterfaceError set suggested_fixes with `or`, so a caller passing [] to mean "I have no suggestions for this one" got the default list instead. Only None should fall back. create_success_response gated `data` on `is not None` but `message` and `metadata` on truthiness, so an explicitly-passed "" or {} vanished from the response while 0 and False survived — the response shape depended on the value. api_helpers.success_response() then re-gated metadata the same way, which is the path every api_v3 endpoint actually calls, so fixing only the inner function would have changed nothing observable. Both now use `is not None`. That wrapper also merged request timing into the caller's own metadata dict in place. A caller reusing a dict across requests would accumulate previous responses' timings; it now copies before adding. 79 tests: category inference for every error code, mapped vs fallback suggestions, the JSON shape including which keys are omitted when empty, exception-to-code inference, and the success/error builders end to end. Two behaviours are pinned as deliberate rather than fixed: an empty context stays out of the response body, and from_exception's `message` is the fixed per-code string, never the raw exception text. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(web): cover the input validators, and close three holes in them validators.py had tests for dedup_unique_arrays only; the other eight functions were untested. Three bugs surfaced. validate_image_url checked for '..' only inside its relative-path branch, so http://host/../secret passed validation while /../secret was rejected — the traversal check now runs before the branch split, which is where a safety check on the whole URL belongs. validate_file_upload lowercased the uploaded filename's extension but compared it against the caller's list verbatim, so allowed_extensions of ['.TTF'] rejected every valid .ttf file. Both sides are lowercased now. The one in-tree caller passes lowercase already, so this only widens what future callers can hand it. validate_numeric_range accepted True and False, because bool subclasses int; a boolean then compared as 1 or 0 against the range and validated cleanly. Excluded explicitly, matching how base_plugin.py already handles the same trap for display_duration. 84 tests. Two behaviours are pinned rather than changed: sanitize_plugin_config deliberately does not HTML-escape strings, since escaping at this layer would store the escaped form in config.json — the docstring said "prevent injection", which read as a promise it does not keep, and now says what it actually does. validate_font_awesome_class's second 'fa-' check is unreachable behind its own regex; harmless, so characterized rather than removed. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(api): cover wifi and registry endpoints, and fix bodyless POSTs The /wifi/* routes drive the host's real networking and the registry routes reach GitHub, and neither had endpoint-level tests. Covering them surfaced a bug affecting six endpoints. Six handlers read their body as `request.get_json() or {}`. The `or {}` says every field is optional and a missing body should fall back to defaults — but get_json() without silent=True raises UnsupportedMediaType when there is no JSON Content-Type, and it raises before `or {}` is ever evaluated. Each handler's catch-all then reported that as a 500. So POSTing with no body — what curl sends by default, and what a fetch() without options sends — failed on /plugins/store/refresh, /display/on-demand/start, /plugins/config/reset, /plugins/of-the-day/json/delete, /plugins/{id}/limits and /plugins/authenticate/spotify. The shipped UI always sends a JSON object, which is why this stayed hidden. All six now use silent=True. test_api_v3_optional_body.py covers the affected endpoints and adds a source check, since the combination of `or <default>` with a non-silent read is self-contradictory wherever it appears and is easier to catch by inspection than by exercising each endpoint by hand. Also adds test/_api_v3_test_helpers.py: the blueprint holds its managers on a module-level singleton rather than in Flask app state, so a test that mocks them leaks into every later test unless the originals are restored. The existing _make_client() does this for unittest classes; this is the pytest-fixture equivalent, for the five suites still to come. 69 endpoint tests: connect/disconnect/AP/radio including the string-aware boolean coercion these endpoints deliberately use, the radio's lockout-refusal path, registry refresh and fetch-from-URL, and a guard that WiFiManager is never constructed for real. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(api): cover the music auth endpoints, and always clean up the wrapper The Spotify step-2 handler writes a Python wrapper script to a temp file with the user's redirect URL embedded in its source, then executes it. That is the most dangerous shape in the blueprint and had no tests. The wrapper was deleted in the success/failure branch and again in the TimeoutExpired handler. Any other failure from subprocess.run — no interpreter, a fork failure, an interrupted call — reached neither, and left a world-readable temp file containing the user's redirect URL on disk. Cleanup moves to a finally block, which is what "delete this whatever happens" should have been from the start. The injection tests are the point of this file. Eight adversarial redirect URLs (embedded quotes, backslashes, newlines, triple quotes, a full `"; import os; os.system("id"); "`) are each pushed through the endpoint and the generated wrapper is parsed with ast: it must still be valid Python, the URL must still be a single string literal bound to redirect_url, and no os.system call may appear anywhere in the tree. json.dumps holds up, but nothing was checking that it does. 40 tests. Also pins that the two endpoints are not symmetrical despite the matching names — only Spotify has a two-step flow and a wrapper; YTM runs its script directly — so a later change does not "restore" a parity that was never there. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(api): cover the credentials upload, and stop it hoarding secrets The endpoint that receives the user's Google OAuth credentials file had no tests. Two bugs surfaced. The OAuth-shape check ran inside `except Exception: pass`. A JSON document that parses but is not an object — a bare 42, true, null, a list — makes `'installed' not in creds_data` raise TypeError, which the bare except swallowed, and the file was then written out as credentials.json regardless. The check now decides the outcome instead of being advisory, so anything not credentials-shaped is refused up front rather than failing later inside the calendar plugin. Every overwrite copies the old file to credentials.json.backup.<ts> and nothing removed them, so a user who re-uploaded ten times had ten complete sets of OAuth client credentials sitting in the plugin directory, indefinitely. Keep the newest five. Pruning is housekeeping, so a backup that cannot be removed logs and leaves the upload alone. 27 tests: size and extension limits, malformed JSON, the shape check, 0600 permissions on the written file, backup-on-overwrite, and pruning including the repeated-upload case that stays bounded. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(api): cover the install endpoints, and make 14 dead guards reachable /plugins/install and /plugins/install-from-url were tested only at the PluginStoreManager layer, so the route logic — the queue-versus-direct branch, schema invalidation, discovery, state and history recording — was unexercised. Covering them surfaced the wider form of the body-parsing bug fixed for the `or {}` handlers in the previous commit. Fourteen handlers read `data = request.get_json()` and immediately guard with `if not data: return 400, 'No data provided'`. That guard cannot run: get_json() without silent=True raises UnsupportedMediaType for a request with no JSON body, so the catch-all answered 500 "an error occurred; see logs for details" where the handler plainly meant to answer 400 and say which field was missing. Every one of these endpoints told a caller who simply forgot the body to go read the server logs. All fourteen now use silent=True, so the guard each author already wrote is the one that runs. This covers /config/raw/main and /config/raw/secrets among them, whose own bodyless case had the same shape. The two remaining bare reads are left alone: neither declares what a missing body should do, so there is no stated intent to honour. 31 install tests plus 17 body tests. The install pair is checked against each other rather than only individually — the same install logic is written twice, once in the queue callback and once in the fallback, so the tests assert both produce identical schema, discovery, state and history effects. They agree today; the one difference is the success message wording, which is characterized rather than changed. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(api): cover the raw config write endpoints /config/raw/main and /config/raw/secrets write whatever JSON they are given straight to config.json and config_secrets.json, bypassing the secret-separation path the rest of the config surface goes through. Given how carefully that surface keeps secrets out of config.json, the pair that skips it was worth pinning precisely. Backed by a real ConfigManager over tmp_path, so the assertions are against files on disk. 20 tests covering both routes: what lands in which file, that a raw secrets write never touches config.json and vice versa, the GitHub token reload, the uninitialized-manager and empty-body branches, and the ConfigError path that carries config_path through to the response. The bypass itself is pinned as intentional rather than changed — these back the raw JSON editor, so writing the body verbatim is the feature. The test says so explicitly, because the failure mode is someone later routing plugin config through here as a convenience and silently losing secret separation. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(api): cover backup restore and path containment, and fix restore scope Restore is the most destructive thing the web interface can do — it overwrites config, secrets, WiFi settings and fonts, then reinstalls plugins — and neither it nor the file routes beside it had tests. A malformed `options` field fell back to {}. Every RestoreOptions flag defaults to True, so a caller who asked for a narrow restore and mis-serialized the request got a full one instead, secrets included, and was told it succeeded. Valid JSON that is not an object was worse: `"null"` or `"[1,2]"` reached .get() on a non-dict and raised, so the request died as a generic 500. Both are now refused with a 400 that says what was wrong, and restore_backup is never reached. The other file routes take a filename straight out of the URL and turn it into a path — one to read, one to unlink. _safe_backup_path is the only thing keeping those inside the export directory, and it was untested. No bypass was found; the thirteen traversal shapes are pinned so a later loosening of that pattern has to argue with something. The delete route's by-name enumeration is covered too, including that a directory sharing a backup's name is not removed. 84 tests. Two behaviours are pinned as intentional: a failed plugin reinstall turns the whole restore into an error even though file restoration succeeded, and omitting `options` entirely still means restore everything — that is the documented default, and it is only the mis-serialized case that was wrong. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * ci: raise coverage floor to 52% Measured 54.45% after the Tier 1 and Tier 2 suites, up from 50%. Keeping the same two points of headroom the 45 -> 48 ratchet used. The modules this branch set out to cover: sync_manager 0 -> 97%, logo_helper 0 -> 98%, errors and error_handler 0 -> 100%, validators 0 -> 97%. api_v3 moved less in percentage terms because it is 4,341 statements, but the endpoints covered are the destructive and credential-handling ones. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(sync): probe for a free port on loopback, not every interface CodeQL flagged the ephemeral-port probe in the handshake test for binding to all interfaces. The probe only needs a free port number, so loopback is both sufficient and correct — a test should not open a port to the network to discover one. The manager under test still binds to all interfaces, which is deliberate and already marked nosec: a follower has to receive the leader's UDP broadcast. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * fix: bound the logo download, and stop malformed input reading as a fault Review findings on the coverage branch. The download size cap I added checked len(response.content), which has already buffered the whole body -- it stopped the bytes reaching disk but not memory, which was the point. A server that omits Content-Length and never stops sending would still exhaust the process. Stream it instead, counting as it arrives, into a sibling .part file that is replaced over the target only once it decodes. A transfer that dies midway now leaves nothing behind rather than a truncated logo for load_logo() to cache. The follower's control-message handler caught three exception types, but two reachable UDP payloads raise others: a bare JSON scalar makes msg.get() raise AttributeError, and an "sx" carrying a non-numeric x raises ValueError or TypeError from float(). Those escaped to the outer handler, skipping the legacy-PNG fallback and -- since this branch added a backoff there -- charging one malformed packet a 0.1s stall on the receive path. The legacy-PNG path also decoded without the dimension cap its TCP counterpart applies, so a crafted 65KB frame could force a large allocation on the render thread; both paths now share one constant. Three repo_url handlers called .strip() on client input without checking it was a string, so {"repo_url": 12345} answered 500. The credentials upload parsed the same file twice, the second time inside a bare except that a preceding parse had already made unreachable. And both raw-config handlers kept a json.JSONDecodeError arm that get_json(silent=True) had turned into dead code, collapsing "sent something unparseable" into "sent nothing" -- they now say which. Two of the new tests were not testing what they claimed. The pruning round-trip wrote ten backups inside one second, so all ten landed on the same int(time.time()) filename and overwrote each other; it never reached the limit it asserted. And the sync clock helper patched attributes on the stdlib time module, freezing time process-wide for every daemon thread earlier tests had left running. Full suite: 3352 passed, coverage 54%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(sync): probe broadcast by sending, not by listening The broadcast check added in the previous commit bound INADDR_ANY to receive its own probe datagram, and the free-port probe did the same to pick a port. CodeQL flagged both, correctly: a test suite has no reason to open a socket the whole network can reach. Sending is enough for what the probe is actually for. An environment that refuses broadcast raises on sendto, which is the case that occurs in sandboxes and is the one worth skipping over; confirming delivery would have required the listening socket. A network that accepts the send and silently drops it still reaches the assertion, exactly as it did before either commit. The port probe binds loopback -- it only needs a number, and the manager's own bind is the one that has to succeed, with the retry loop already covering a port taken elsewhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * fix: keep callback faults out of the frame-decode fallback Review follow-up on the previous two commits. Widening the control-message except tuple put the callback dispatch inside it, so an _on_new_cycle() that raised ValueError, TypeError or AttributeError sent a perfectly good control packet to the legacy PNG decoder -- which reported it as an image decode error and buried the real fault. Split the two: whether the payload parses as JSON decides frame vs control message, a second guard covers reading the fields of an attacker-shaped body, and the callback fires outside both. It still cannot kill the receive thread; the loop's own handler catches it, and now says what actually went wrong. The logo download's temp file was a fixed "<name>.part". Two plugins asking for the same logo at once would interleave writes into it, publish the mixture, or delete each other's partial. mkstemp gives each download its own name in the same directory, so os.replace stays atomic. Its descriptor is adopted by fdopen before the request runs, since a request that raises before the write would otherwise leak the fd -- quietly, because load_logo_with_download swallows that. Two test fixes: the oversized-frame test replaced PIL.Image.open process-wide, the same hazard the clock helper documents, and Ruff B007 on an unused loop variable. Full suite: 3355 passed, coverage 54%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test(sync): cover the announce loop, and reject non-finite scroll positions Three review findings from the follower receive path. Non-finite scroll x reached follower rendering. json.loads accepts the bare NaN/Infinity literals and float() accepts them as strings, so "x": NaN arrived as a real float and was stored verbatim. NaN loses every comparison the scroll code makes, so a follower given one sits on a position it can never advance past. It now raises through the existing malformed-control-message guard, which logs and drops the packet and leaves the last good position in place. _broadcast_available() only proves the host accepts sendto() for a broadcast; a network that accepts the send and drops the packet would let TestRealSocketHandshake run to its five-second deadline and fail on assertions the code did not break. The deadline now distinguishes the two: if not one packet crossed in either direction, that is the environment, and the test skips rather than reporting a protocol failure. That skip could hide a real regression in the announcing side, so TestFollowerAnnounceLoop covers it on mock sockets, where no network is involved and nothing can skip: hello carries this display's hardware config and goes to the broadcast address, heartbeats follow, an empty hardware config falls back to 32x64x1, hello is not resent before its interval, and a send failure is swallowed rather than killing the loop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh --------- Co-authored-by: Claude <noreply@anthropic.com> |