fix(camera): transcode non-JPEG external snapshots to JPEG (#1902)

External cameras in HTTP-snapshot mode failed to load with a repeating
"connection lost" when the endpoint served PNG/WebP/BMP stills instead of
JPEG (common on IP cameras and reverse-proxied snapshot URLs). The URL
rendered fine directly in a browser, but Bambuddy's MJPEG stream wraps
every part in a hard-coded Content-Type: image/jpeg boundary, so a
non-JPEG payload labelled as JPEG made the browser reject the frame and
tear down the whole multipart/x-mixed-replace stream.

_capture_snapshot now transcodes non-JPEG stills to JPEG via OpenCV
(already a dependency). Genuine JPEG snapshots keep a byte-for-byte fast
path; truly undecodable responses (HTML error pages, auth redirects) fall
back to the previous raw-return behaviour with a single clear warning
instead of a per-frame log flood.
This commit is contained in:
maziggy
2026-07-06 07:54:30 +02:00
parent a82eeff483
commit 379765a46a
3 changed files with 176 additions and 8 deletions
+1
View File
@@ -5,6 +5,7 @@ All notable changes to Bambuddy will be documented in this file.
## [0.2.5b2] - Unreleased
### Fixed
- **External camera "connection lost" when the snapshot URL serves a non-JPEG image (#1902)** — An external camera configured in HTTP-snapshot mode failed to load in Bambuddy with a repeating "connection lost", even though the camera URL rendered fine when opened directly in a browser. The log showed `Snapshot does not appear to be JPEG` on every polled frame followed by the stream ending. Root cause: `_capture_snapshot` returned the fetched bytes even when they weren't JPEG, and the MJPEG stream wraps every part with a hard-coded `Content-Type: image/jpeg` boundary — so a camera serving PNG/WebP/BMP stills (common on IP cameras and reverse-proxied snapshot endpoints) sent the browser a non-JPEG payload labelled as JPEG, which the browser rejected, tearing down the whole `multipart/x-mixed-replace` stream. `_capture_snapshot` now transcodes non-JPEG stills to JPEG (via OpenCV, already a dependency) before streaming; genuine JPEG snapshots keep their byte-for-byte fast path, and truly undecodable responses (HTML error pages, auth redirects) fall back to the previous raw-return behaviour with a clearer one-off warning instead of a per-frame log flood. Fixes browser playback and keeps the JPEG-only downstream (plate detection, Obico, finish photo) working for these cameras.
- **Per-user Notifications page unreachable from the sidebar (#1901, reporter @JmanB52D)** — The Notifications entry (where each user opts in/out of their own print email notifications) disappeared from the left navigation. The page (`/notifications`) and its API were both intact — only the sidebar link was gone, so the screen was reachable only by typing the URL. Root cause: the sidebar-ordering refactor in #1673 accidentally deleted the `notifications` item from `defaultNavItems` (and its `notifications:user_email` permission mapping) while extracting the ordering helpers, but left the advanced-auth visibility gate that references that id — so the gate had nothing to gate and the item could never render. Restored both the `defaultNavItems` entry and the permission gate; the item now shows for any user holding `notifications:user_email` (both default groups, Administrators and Operators, do) when advanced auth and user email notifications are enabled, exactly as before #1673.
- **Virtual Printer FTP uploads silently truncated under uvloop — a corrupt `.gcode.3mf` was archived, queued, and forwarded to the real printer with a `226 Transfer complete` (#1896, reporter @dj-oyu)** — On a native venv install (not Docker), slicing in Bambu Studio and sending to a queue-mode VP produced a truncated upload: Bambuddy logged `226 Transfer complete`, archived the file, added it to the queue, and later pushed the corrupt file to the physical printer (A1 mini), which then failed to parse/start the job. Every truncated file ended at an exact multiple of 4096 bytes with a valid `PK\x03\x04` local header but no ZIP End-Of-Central-Directory record. **Root cause — isolated deterministically by the reporter.** uvloop's SSL layer discards already-received but still-buffered data when the client closes the data connection **without a TLS `close_notify`** (a "ragged EOF") while the reader is **flow-control-paused** (slow consumer). `cmd_STOR` writes each 64 KiB chunk to disk synchronously inside the read loop; on slow storage (the reporter's data dir was on a microSD on an ARM64 SBC) the reader falls behind, the transport pauses, and the tail of the upload is lost — `read()` then returns a clean empty EOF, so the loop exits normally with **no exception and no write error**, and the server acks 226 for a file it truncated itself. The reporter's isolation matrix reproduces it on a minimal uvloop 0.22.1 TLS server (slow reader → 2,248,704 of 2,500,001 bytes, 3/3 runs) but never on CPython's default asyncio loop or on uvloop with a fast reader — which is why Docker/x86-with-SSD deployments almost never hit it (the reader keeps up, flow control never pauses, the loss window never opens). Bambuddy's Dockerfile already runs `--loop asyncio`; **every native launch path did not** and so auto-selected uvloop via `uvicorn[standard]`. **Fix — two independent layers.** (1) *Remove the trigger.* Added `--loop asyncio` to every native launch path so uploads actually arrive intact, matching the Dockerfile: `deploy/bambuddy.service`, `install/install.sh` (systemd unit + macOS launchd plist), `spoolbuddy/install/install.sh` (the bundled Bambuddy backend service), `installers/windows/service/install-service.bat` (NSSM), `README.md`, and the wiki install docs (run command, systemd, launchd) — each with an inline "do not remove, see #1896" note. The reporter verified `--loop asyncio` fully resolves it (before: 8/8 real Bambu Studio uploads truncated; after: 2/2 intact, valid ZIPs, `testzip` clean). (2) *Defense in depth, loop-independent.* `cmd_STOR` now validates that a received `.3mf` opens as a ZIP (reads the central directory — O(dir), no decompression) **before** replying 226. A truncated/corrupt 3MF is treated exactly like a failed transfer: the file is dropped and the slicer gets `426 Transfer failed: uploaded 3MF is incomplete or corrupt`, and the `on_file_received` callback that archives/queues/forwards the job never runs — so a broken upload surfaces as an immediate, actionable slicer-side send error instead of a confusing printer-side parse failure later. Validation is scoped to `.3mf` uploads; other filetypes keep the prior pass-through behaviour. This layer protects anyone who still runs uvloop for any reason (custom launch command, future `uvicorn[standard]` default). **Tests.** `test_vp_ftp_stor.py`: the happy-path test now feeds a real multi-chunk ZIP and asserts 226 (not 426); new `test_stor_rejects_truncated_3mf` drops the EOCD-bearing tail and asserts 426 + file removed + `on_file_received` never called; new `test_stor_skips_zip_validation_for_non_3mf` asserts a plain `.gcode` still gets 226 (no false-positive). 6/6 in the file green, ruff clean. **Scope.** Backend (one validation block in `cmd_STOR` + `zipfile` import) plus launch-config across repo installers, the Windows/SpoolBuddy installers, README, and the install wiki. No DB migration, no new permission, no i18n key, no frontend change. Users on a native install: after upgrade, re-run the installer (or add `--loop asyncio` to your existing service command) to stop the truncation at the source; the ZIP-validation guard takes effect on the next Bambuddy restart regardless. **Workaround for older versions:** launch uvicorn with `--loop asyncio`.
- **API keys could not manage Projects — every project mutation returned `403 "API keys cannot be used for administrative operations"` regardless of the key's granted permissions (#1893, reporter @abbasegbeyemi)** — `POST /projects/{id}/add-archives`, project create/update/delete, and every other project mutation route was unreachable for any API key. **Root cause.** `PROJECTS_CREATE` / `PROJECTS_UPDATE` / `PROJECTS_DELETE` were in `_APIKEY_DENIED_PERMISSIONS` in `core/auth.py` with no corresponding entry in `_APIKEY_SCOPE_BY_PERMISSION` and no `can_manage_projects` flag on `api_keys` at all — so under the GHSA-r2qv allowlist model they resolved to scope `None` and raised the generic administrative-operations 403. This is the exact regression class already fixed for archives (#1888) and library (#1832): the projects block sat directly between the comment blocks documenting those two carve-outs but was never itself carved out. **Fix.** New per-key scope `can_manage_projects` (column on `api_keys`, DEFAULT TRUE for keys created via the UI going forward; existing rows backfill to FALSE so the upgrade path never silently widens scope — these permissions were explicitly denied for every key before, so nothing relies on them). Unlike archives/library, the project routes gate on plain `RequirePermissionIfAuthEnabled(Permission.PROJECTS_*)` — there is no OWN/ALL ownership split for projects — so all three CRUD permissions map directly to the one scope. Project **membership** edits (`add_archives_to_project` etc.) gate on `PROJECTS_UPDATE`, so they're covered by the same toggle; `PROJECTS_READ` is unchanged (already under `can_read_status`, so API keys could always read projects). Users opt a key in from Settings → API Keys ("Manage Projects" toggle, with a "Projects" badge on the key list). The bundled SpoolBuddy kiosk key (created via the CLI) is set to `can_manage_projects=False` to stay minimally scoped. **Migration** is dialect-agnostic (`BOOLEAN` is valid on both SQLite and Postgres); verified end-to-end on a throwaway fresh SQLite and Postgres 17 that the column adds, legacy rows backfill to FALSE, and a new row defaults to TRUE. **Tests.** `test_auth_apikey_rbac.py` extended: the `_check_apikey_permissions` scope matrix now covers all three project permissions (true→allow, false→403, no cross-scope leakage), and `PROJECTS_CREATE` / `_UPDATE` / `_DELETE` added to the operational-allowed drift guard + threaded through the structural allowlist/flag-parity checks — 63 cases green. **Scope.** Backend (model + migration + allowlist + schema + route + CLI) plus the Settings API-key UI (toggle + badge + type) and 11-locale i18n for the new label/description/badge. No change to the project routes themselves — they already gated on the right permissions; only the API-key classification of those permissions was wrong.
+61 -8
View File
@@ -461,6 +461,39 @@ async def _capture_rtsp_frame(url: str, timeout: int) -> bytes | None:
await proxy_server.wait_closed()
def _transcode_to_jpeg(data: bytes) -> bytes | None:
"""Decode an arbitrary still image (PNG/WebP/BMP/GIF/...) and re-encode as JPEG.
Some camera/proxy snapshot endpoints serve stills as PNG or WebP rather than
JPEG. A browser opened directly at the URL renders those fine, but our MJPEG
``multipart/x-mixed-replace`` stream hard-labels every part
``Content-Type: image/jpeg`` — so a non-JPEG payload makes the browser reject
the frame and drop the whole stream ("connection lost", #1902). Transcoding to
JPEG keeps the stream genuinely MJPEG and also keeps the JPEG-only downstream
(plate detection, Obico, finish photo) working.
Returns None if the bytes are not a decodable image (e.g. an HTML error page)
or if the imaging libraries are unavailable — callers fall back to the raw
bytes so behaviour is never worse than before.
"""
try:
import cv2
import numpy as np
except ImportError:
return None
try:
img = cv2.imdecode(np.frombuffer(data, dtype=np.uint8), cv2.IMREAD_COLOR)
if img is None:
return None
ok, buf = cv2.imencode(".jpg", img, [cv2.IMWRITE_JPEG_QUALITY, 85])
if not ok:
return None
return buf.tobytes()
except Exception as e: # cv2 raises cv2.error (a subclass of Exception) on bad input
logger.debug("Snapshot transcode to JPEG failed: %s", e)
return None
async def _capture_snapshot(url: str, timeout: int) -> bytes | None:
"""Fetch snapshot from HTTP URL.
@@ -484,14 +517,6 @@ async def _capture_snapshot(url: str, timeout: int) -> bytes | None:
return None
data = await response.read()
# Validate it looks like JPEG
if not data.startswith(b"\xff\xd8"):
logger.warning("Snapshot does not appear to be JPEG")
# Still return it - might be valid with different header
return data
except TimeoutError:
logger.warning("Snapshot capture timed out after %ss", timeout)
return None
@@ -499,6 +524,34 @@ async def _capture_snapshot(url: str, timeout: int) -> bytes | None:
logger.error("Snapshot capture failed: %s", e)
return None
# Fast path: already JPEG (SOI marker), stream it as-is (no decode/re-encode).
if data.startswith(b"\xff\xd8"):
return data
# Not JPEG. Many snapshot endpoints serve PNG/WebP/BMP — transcode to JPEG so
# the browser's MJPEG stream (and JPEG-only downstream) keep working instead of
# dropping the connection (#1902). Run off the event loop: cv2 decode/encode is
# CPU-bound and this can be polled at up to 15 fps while a camera view is open.
transcoded = await asyncio.to_thread(_transcode_to_jpeg, data)
if transcoded is not None:
logger.debug(
"Transcoded non-JPEG snapshot (%d bytes, header %s) to JPEG",
len(data),
data[:4].hex(),
)
return transcoded
# Couldn't decode it as an image at all — most likely not an image response
# (HTML error page, auth redirect, wrong URL). Return the raw bytes as a last
# resort (unchanged behaviour) but log enough to debug.
logger.warning(
"External camera snapshot is not a decodable image "
"(%d bytes, header %s) — verify the camera URL returns an image",
len(data),
data[:4].hex(),
)
return data
async def test_connection(url: str, camera_type: str) -> dict:
"""Test camera connection.
@@ -591,3 +591,117 @@ class TestUsbCameraHandling:
result = await capture_frame("http://example.com", "usb", timeout=1)
assert result is None
def _encode_image(ext: str) -> bytes:
"""Encode a small solid test image to the given container (.png/.webp/.jpg)."""
import cv2
import numpy as np
img = np.zeros((16, 24, 3), dtype=np.uint8)
img[:, :12] = (0, 0, 255) # half red so the frame isn't uniformly black
ok, buf = cv2.imencode(ext, img)
assert ok, f"failed to encode {ext}"
return buf.tobytes()
def _fake_snapshot_session(body: bytes, status: int = 200):
"""Build an aiohttp.ClientSession stand-in whose GET yields `body`.
Matches the `async with ClientSession(...) as session, session.get(url) as
response` usage inside `_capture_snapshot`.
"""
class _Resp:
def __init__(self):
self.status = status
async def __aenter__(self):
return self
async def __aexit__(self, *a):
return False
async def read(self):
return body
class _Session:
def __init__(self, *a, **k):
pass
async def __aenter__(self):
return self
async def __aexit__(self, *a):
return False
def get(self, _url):
return _Resp()
return _Session
class TestSnapshotTranscode:
"""Regression for #1902. Snapshot endpoints that serve PNG/WebP (not JPEG)
broke the browser MJPEG stream, because every multipart part is hard-labelled
``Content-Type: image/jpeg`` — the browser rejected the non-JPEG payload and
dropped the whole stream ("connection lost"). ``_capture_snapshot`` now
transcodes non-JPEG stills to JPEG; only genuinely undecodable payloads fall
through to the raw bytes (unchanged last-resort behaviour)."""
def test_transcode_png_to_jpeg(self):
from backend.app.services.external_camera import _transcode_to_jpeg
png = _encode_image(".png")
assert not png.startswith(JPEG_START) # sanity: input really is PNG
out = _transcode_to_jpeg(png)
assert out is not None and out.startswith(JPEG_START)
def test_transcode_webp_to_jpeg(self):
from backend.app.services.external_camera import _transcode_to_jpeg
webp = _encode_image(".webp")
assert not webp.startswith(JPEG_START)
out = _transcode_to_jpeg(webp)
assert out is not None and out.startswith(JPEG_START)
def test_transcode_returns_none_for_non_image(self):
"""HTML error pages / auth redirects / empty bodies aren't images —
transcode returns None so the caller can log and fall back."""
from backend.app.services.external_camera import _transcode_to_jpeg
assert _transcode_to_jpeg(b"<html><body>404 Not Found</body></html>") is None
assert _transcode_to_jpeg(b"") is None
@pytest.mark.asyncio
async def test_capture_snapshot_transcodes_png_response(self):
"""The reported case: a snapshot URL returning PNG yields JPEG bytes."""
from backend.app.services import external_camera as ec
png = _encode_image(".png")
with patch.object(ec.aiohttp, "ClientSession", _fake_snapshot_session(png)):
out = await ec._capture_snapshot("http://192.168.50.50/snapshot.png", 10)
assert out is not None and out.startswith(JPEG_START)
@pytest.mark.asyncio
async def test_capture_snapshot_jpeg_passthrough_unchanged(self):
"""A JPEG snapshot must be returned byte-for-byte (fast path, no
re-encode) so we don't degrade quality or waste CPU on JPEG cameras."""
from backend.app.services import external_camera as ec
jpeg = _encode_image(".jpg")
assert jpeg.startswith(JPEG_START)
with patch.object(ec.aiohttp, "ClientSession", _fake_snapshot_session(jpeg)):
out = await ec._capture_snapshot("http://192.168.50.50/snapshot.jpg", 10)
assert out == jpeg # identical object bytes — proves no transcode ran
@pytest.mark.asyncio
async def test_capture_snapshot_non_image_falls_back_to_raw(self):
"""Undecodable (non-image) responses return the raw bytes unchanged, so
behaviour is never worse than before the fix."""
from backend.app.services import external_camera as ec
html = b"<html><body>unauthorized</body></html>"
with patch.object(ec.aiohttp, "ClientSession", _fake_snapshot_session(html)):
out = await ec._capture_snapshot("http://192.168.50.50/snapshot", 10)
assert out == html