diff --git a/CHANGELOG.md b/CHANGELOG.md index ab9430971..355cd922f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ All notable changes to Bambuddy will be documented in this file. - **`docker-compose.yml`: bridge-mode warning about the 1001-port FTP passive range + docker-proxy RAM footprint (#1646, reported by @TheFou)** — Reporter on bridge mode (Docker default `userland-proxy: true`) saw ~2000 `docker-proxy` host processes spawn from the commented `"50000-51000:50000-51000"` line, pinning ~3.5 GB of host RAM before they had even logged in for the first time. Linux's host-mode default in the same compose file sidesteps this entirely (zero docker-proxy cost) — the issue only fires when a user forces bridge mode (typically macOS/Windows / Docker Desktop). The 1001-port range itself is load-bearing on the VP server side (`virtual_printer/ftp_server.py:567-574` documents the widening from 100 ports as multi-VP collision-avoidance headroom; reverting would regress that), so the fix is documentation, not code. Added a warning block above the commented FTP-passive line pointing bridge-mode users at `{ "userland-proxy": false }` in `/etc/docker/daemon.json` — the reporter confirmed this clears the issue on their setup. Kernel does NAT directly via iptables/nftables in that mode, no per-port host process needed; only side-effect is that connections originating from 127.0.0.1 on the host itself can't reach the container, which doesn't matter for nearly every Bambuddy install. ### Fixed +- **VP MQTT bridge `net.info[].ip` rewrite never armed when the printer was added by hostname/FQDN (#1429, root-caused by @Mape6, also hit @TrickShotMLG02)** — Reporter on a flat 192.168.3.0/24 LAN had added a P1S to Bambuddy by its router-provided DNS name `p1s.fritz.box` instead of its IPv4. On 0.2.4+ that one detail kept Bambu Studio Send going to the real printer instead of the Bambuddy archive whenever the printer was powered on — exact same surface symptom #1429 was originally about, but a separate root cause from the bind-IP encoding work shipped on 2026-06-02. The defensive `NOT armed` logging ([[issue1429_vp_ip_leak]]) added in this release pinpointed it on the reporter's bundle: `MQTT bridge IP encoding NOT armed: invalid IPv4 (target='p1s.fritz.box', vp='192.168.3.27'): invalid literal for int() with base 10: 'p1s'`. The encoder `_ip_to_uint32_le` (and the host-interface picker `find_interface_for_ip`) both assume dotted-quad IPv4 and bail on anything else, so `BambuMQTTClient.ip_address` being the configured FQDN string short-circuited the rewrite path and `net.info[*].ip` kept leaking the real printer's IPv4. Switching the printer record to an IPv4 cleared the issue immediately for the reporter — that workaround confirms the diagnosis exactly. **Why this didn't bite pre-0.2.4**: the bridge didn't do `net.info[].ip` rewriting at all before #1429 shipped, so FQDN-configured printers worked by accident — nothing was trying to parse the host as IPv4. **Fix** adds `_resolve_target_to_ipv4(target)` in `mqtt_bridge.py`: pass-through when `target` already parses as `ipaddress.IPv4Address`, otherwise `socket.getaddrinfo(target, None, family=socket.AF_INET)` to filter to IPv4-only (the `net.info[*].ip` field is uint32 LE — there's no IPv6 representation that fits, so an AF_INET6 result must not slip through). Returns `None` on empty input *and* on `OSError` from getaddrinfo so transient DNS hiccups don't break the encoding permanently; `_refresh_ip_encoding` falls back to the existing `NOT armed` throttle which re-resolves on every 30s refresh tick (DHCP / DNS churn picks itself up). Both the `_ip_to_uint32_le(target_ip)` call AND the `_resolve_host_interface_for_target(target_ip)` call now receive the resolved IPv4, so the same fix covers the bind-address auto-resolve path used on default-config (0.0.0.0 bind) installs that don't have a dedicated VP bind IP. The configured FQDN is preserved into the armed log line as `configured→resolved` (`target=p1s.fritz.box→192.168.3.153`) so a bad-DNS regression stays legible in `docker logs` without grepping back to the not-armed lines. The unresolvable-input not-armed reason is now `could not resolve printer host '' to IPv4 (invalid address and DNS lookup failed)` — names the actual configured value, not just `invalid IPv4 (target=...)`, so future bundles distinguish "DNS gave us a v6 address" from "user typed garbage" without a guess. **Tests**: 5 new in `TestHostnameResolution` (pass-through for IPv4, empty/None → None, FQDN → resolved IPv4 with AF_INET filter asserted, `OSError` → None, end-to-end FQDN-targeted bridge arms with the resolved IPv4 in `_target_ip_uint32_le` and the `configured→resolved` shape in the armed log). The existing `test_invalid_ipv4_logs_value_error` renamed to `test_unresolvable_target_logs_reason` and now patches `getaddrinfo` to `OSError` so the test is hermetic; asserts the new `could not resolve printer host 'not.an.ip'` message. 49 bridge tests pass; ruff clean. + - **MakerWorld URL imports into a writable external folder wrote bytes to internal storage, not the NAS (#1645, reported and root-caused by @needo37)** — Reporter linked a writable external SMB folder, selected it as the destination in the MakerWorld import dialog, the import succeeded, the file card appeared in the File Manager under the external folder's view — but `ls` on the NAS turned up nothing, and a `find` across the whole NAS and the container for the original filename matched nothing either. The bytes had landed in Bambuddy's internal `/archive/library/files/.3mf` instead of `/` on the mount. Root cause was the byte-import save helper `save_3mf_bytes_to_library` at `backend/app/api/routes/library.py:422`: it accepts `folder_id` but never loaded the folder or inspected `is_external` / `external_path`, hardcoded the destination to `get_library_files_dir() / `, and left the `LibraryFile` row with `is_external=False`. So the row's `folder_id` pointed at the external folder while its bytes + `is_external` flag both said "managed/internal" — exact same class of bug as #1112 (which got fixed for the multipart-upload and move paths but never applied to the byte-import path). Compounded by the UUID-renamed on-disk copy: searching for the human-readable basename anywhere — NAS or container — never matches. **Fix** is a direct mirror of the multipart-upload path that's done this correctly since #1112: load the target `LibraryFolder` (when `folder_id` is non-None), feed it to the existing `_resolve_upload_destination(target_folder, filename)` helper which already produces `(dest, is_external)` and enforces the 403-read-only / 400-unwritable-or-missing / 409-collision rejections, write the bytes to that destination (real filename for external, UUID for managed), and persist the row via `_stored_file_path(dest, is_external)` + `is_external=is_external`. The route-layer read-only guard at `makerworld.py:256-260` is preserved — it returns the friendlier error before the upstream download burns bandwidth — and `_resolve_upload_destination`'s identical check stays as defence-in-depth for any future caller that skips the route gate. Thumbnails continue to live under the managed `get_library_thumbnails_dir()` regardless of the 3MF's location, matching the upload path. **Tests**: 4 new in `TestImport` (writable external → bytes on mount + `is_external=True` + absolute file_path persisted; read-only external → 403 at route, no download; missing external_path → 400; filename collision → 409 with the pre-existing file's bytes untouched). 21 existing makerworld tests + 72 library-route tests stay green. Ruff clean. - **X2D archives lose 3MF metadata because FTPS handshake fails on firmware 01.01.00.00 (#1638, reported by @vasmarfas)** — Reporter's first archive entries from a brand-new X2D landed almost empty (only print time visible, no filament weight / layers / MakerWorld link / thumbnail), and Spoolman filament-usage tracking also went silent. The support bundle traces the symptom end-to-end: at print start `backend/app/main.py::on_print_start` tries the usual FTP-download dance for the 3MF, every connect attempt to the printer fails with `[SSL: WRONG_VERSION_NUMBER] wrong version number (_ssl.c:1032)`, and ~2 minutes later `Could not find 3MF file for print: /data/Metadata/plate_1.gcode` → `Created fallback archive N for (no 3MF available)`. The fallback path writes the row with `file_path=""`, `file_size=0`, `content_hash=NULL`, and no layers / filament / model-link fields — exactly the "almost empty card" in the reporter's screenshot. Spoolman tracking and reprint-grouping also degrade from the same root cause: both depend on metadata pulled out of the 3MF by `ThreeMFParser`. The proximate cause is the FTPS handshake: Python 3.13's default `ssl.create_default_context()` negotiates TLS 1.3, and the X2D's implicit-FTPS server on port 990 rejects the ClientHello with `WRONG_VERSION_NUMBER`. This is the same shape of symptom as the P2S 01.02.00.00 FTPS bug from #1401 — handshake / data-channel breakage triggered by the move to Python 3.13's TLS-1.3 default — but the wire-level failure mode is different (P2S completes the handshake and truncates mid-stream with 426; X2D fails the handshake outright). Both are addressed via the per-model registry that #1401 established: `backend/app/services/ftp_profiles.py` gains an `X2D` entry with `cap_tls_v1_2=True` plus a `N6 → X2D` SSDP alias, so the X2D's `ImplicitFTP_TLS` connection caps the SSL context's `maximum_version` to TLS 1.2 and the ClientHello looks like the one the firmware accepted before the Python upgrade. Deliberately conservative — every other model stays on negotiated TLS 1.3, only X2D-tagged sessions flip. **Honest caveat**: this ships as a hypothesis-driven trial rather than a confirmed root-cause fix. The TLS-1.2 cap is the most likely cure given the symptom's family resemblance to #1401, but `WRONG_VERSION_NUMBER` could equally describe the X2D switching to explicit FTPS (AUTH TLS on a plaintext greeting) or moving the FTPS service to a different port — both would need a different code path. The reporter has been asked to test this build; if the cap doesn't clear the error, the registry slot stays useful as a tuning anchor and the next round of diagnostics (`openssl s_client -connect :990 -tls1_2` from a network-adjacent host) will tell us which of (2)/(3) applies. **Tests**: 3 new in `test_ftp_profiles.py` mirroring the existing P2S coverage — `X2D` resolves to `cap_tls_v1_2=True`, `N6` SSDP code aliases to the X2D profile, lowercase `x2d` still hits the cap. Existing P2S + default + unknown-model + frozen-dataclass + non-capped-spot-check (X1C / H2D / P1S / A1) tests stay green. **Verified**: ruff clean; the integration test at `test_cap_tls_v1_2_actually_applied_to_ssl_context` already pins the profile→`ImplicitFTP_TLS`→`ssl_context.maximum_version` wiring so this entry can't silently fail to apply. diff --git a/backend/app/services/virtual_printer/mqtt_bridge.py b/backend/app/services/virtual_printer/mqtt_bridge.py index 7c81724e0..8ddf66e8b 100644 --- a/backend/app/services/virtual_printer/mqtt_bridge.py +++ b/backend/app/services/virtual_printer/mqtt_bridge.py @@ -35,8 +35,10 @@ from __future__ import annotations import asyncio import copy +import ipaddress import json import logging +import socket from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -90,6 +92,38 @@ def _ip_to_uint32_le(ip_str: str) -> int: return parts[0] | (parts[1] << 8) | (parts[2] << 16) | (parts[3] << 24) +def _resolve_target_to_ipv4(target: str) -> str | None: + """Return a dotted-quad IPv4 for `target`, resolving hostnames if needed. + + The printer client may be configured by IPv4 *or* by hostname/FQDN + (e.g. `p1s.fritz.box`) — the latter is common on home LANs with a + DNS-providing router. The downstream `net.info[].ip` field is a + 32-bit little-endian integer though, so a hostname can't round-trip + through it; we have to pick *one* concrete IPv4 to write in. + + Returns None if `target` is empty, not parseable as IPv4, and DNS + resolution fails — caller logs that as the not-armed reason and + re-tries on the next refresh tick (DHCP/DNS churn picks itself up). + """ + if not target: + return None + try: + return str(ipaddress.IPv4Address(target)) + except (ValueError, ipaddress.AddressValueError): + pass + try: + # AF_INET filters to IPv4 only; the rewrite field is uint32 LE, + # there's no IPv6 representation that fits. + infos = socket.getaddrinfo(target, None, family=socket.AF_INET) + except OSError: + return None + for info in infos: + sockaddr = info[4] + if sockaddr and isinstance(sockaddr[0], str): + return sockaddr[0] + return None + + def _resolve_host_interface_for_target(target_ip: str) -> str | None: """Pick a host-side IPv4 for `net.info[].ip` when the VP has no dedicated bind IP. @@ -411,11 +445,21 @@ class MQTTBridge: _log_not_armed("target_client is None (bridge not bound to a printer)") return - target_ip = getattr(client, "ip_address", None) - if not target_ip: + configured_target = getattr(client, "ip_address", None) + if not configured_target: _log_not_armed("printer client has no ip_address yet") return + # Printers configured by hostname/FQDN (e.g. `p1s.fritz.box`) need to + # be resolved to an IPv4 before encoding: net.info[*].ip is uint32 LE + # and can't carry a hostname (#1429 follow-up). + target_ip = _resolve_target_to_ipv4(configured_target) + if not target_ip: + _log_not_armed( + f"could not resolve printer host {configured_target!r} to IPv4 (invalid address and DNS lookup failed)" + ) + return + vp_ip = getattr(self._mqtt_server, "bind_address", None) vp_ip_source = "bind_address" if not vp_ip or vp_ip in ("0.0.0.0", ""): # nosec B104 @@ -446,11 +490,12 @@ class MQTTBridge: self._vp_ip_uint32_le = new_vp_le # Clear the dedup so a future failure re-emits the diagnostic line. self._not_armed_reason = None + target_display = target_ip if target_ip == configured_target else f"{configured_target}→{target_ip}" logger.info( "[%s] MQTT bridge IP encoding %s: target=%s vp=%s (%s)", self.vp_name, "updated" if was_armed else "armed", - target_ip, + target_display, vp_ip, vp_ip_source, ) diff --git a/backend/tests/unit/test_vp_mqtt_bridge.py b/backend/tests/unit/test_vp_mqtt_bridge.py index 5e3045dcf..1512bf93f 100644 --- a/backend/tests/unit/test_vp_mqtt_bridge.py +++ b/backend/tests/unit/test_vp_mqtt_bridge.py @@ -3,6 +3,7 @@ import asyncio import json import logging +import socket from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch @@ -12,6 +13,7 @@ from backend.app.services.virtual_printer.mqtt_bridge import ( MQTTBridge, _ip_to_uint32_le, _resolve_host_interface_for_target, + _resolve_target_to_ipv4, ) from backend.app.services.virtual_printer.mqtt_server import SimpleMQTTServer @@ -1088,6 +1090,62 @@ class TestIpEncoding: _ip_to_uint32_le("not.an.ip.actually") +class TestHostnameResolution: + """#1429 follow-up: users who configured the printer by FQDN (common on + LANs with router-provided DNS like `p1s.fritz.box`) hit `invalid IPv4` + on the encoder and the rewrite never armed — slicer kept FTPing direct + to the real printer. The bridge now resolves hostname→IPv4 first.""" + + def test_pass_through_for_valid_ipv4(self): + assert _resolve_target_to_ipv4("192.168.1.50") == "192.168.1.50" + + def test_empty_returns_none(self): + assert _resolve_target_to_ipv4("") is None + assert _resolve_target_to_ipv4(None) is None # type: ignore[arg-type] + + def test_hostname_resolves_via_getaddrinfo(self): + with patch( + "backend.app.services.virtual_printer.mqtt_bridge.socket.getaddrinfo", + return_value=[(2, 1, 6, "", ("192.168.3.153", 0))], + ) as mock_gai: + assert _resolve_target_to_ipv4("p1s.fritz.box") == "192.168.3.153" + # AF_INET filter prevents an IPv6-only result from being picked, + # since net.info[*].ip is a uint32 LE that can't carry v6. + assert mock_gai.call_args.kwargs.get("family") == socket.AF_INET + + def test_dns_failure_returns_none(self): + with patch( + "backend.app.services.virtual_printer.mqtt_bridge.socket.getaddrinfo", + side_effect=OSError("Name or service not known"), + ): + assert _resolve_target_to_ipv4("nope.invalid") is None + + def test_fqdn_target_arms_encoding(self, caplog): + """End-to-end: a client whose `ip_address` is an FQDN should arm + the bridge once DNS resolves, and the cached rewrite uses the + resolved IPv4 (not the hostname string) for the `net.info[].ip` + encoding.""" + server = _make_server(bind_address=VP_IP) + bridge = _make_bridge(server) + client = _make_paho_client(ip="p1s.fritz.box") + bridge._target_client = client + with ( + patch( + "backend.app.services.virtual_printer.mqtt_bridge.socket.getaddrinfo", + return_value=[(2, 1, 6, "", (H2D_IP, 0))], + ), + caplog.at_level(logging.INFO, logger="backend.app.services.virtual_printer.mqtt_bridge"), + ): + bridge._refresh_ip_encoding() + assert bridge._target_ip_uint32_le == _ip_to_uint32_le(H2D_IP) + assert bridge._vp_ip_uint32_le == _ip_to_uint32_le(VP_IP) + armed = [r for r in caplog.records if "MQTT bridge IP encoding armed" in r.getMessage()] + assert len(armed) == 1 + # Operator should see configured→resolved in the log line so a + # bad-DNS regression is immediately legible. + assert "p1s.fritz.box→192.168.255.133" in armed[0].getMessage() + + # --------------------------------------------------------------------------- # Auto-resolve fallback for default-config (bind_address = "0.0.0.0") # --------------------------------------------------------------------------- @@ -1236,17 +1294,26 @@ class TestNotArmedDiagnosticLogging: assert H2D_IP in msg assert "no host interface" in msg - def test_invalid_ipv4_logs_value_error(self, caplog): + def test_unresolvable_target_logs_reason(self, caplog): + """When `ip_address` isn't a valid IPv4 *and* doesn't resolve via DNS, + the bridge must report a single concrete not-armed reason naming the + configured value — operator can then see exactly what input failed.""" server = _make_server(bind_address=VP_IP) bridge = _make_bridge(server) client = _make_paho_client() client.ip_address = "not.an.ip" bridge._target_client = client - with caplog.at_level(logging.INFO, logger="backend.app.services.virtual_printer.mqtt_bridge"): + with ( + patch( + "backend.app.services.virtual_printer.mqtt_bridge.socket.getaddrinfo", + side_effect=OSError("nodename nor servname provided"), + ), + caplog.at_level(logging.INFO, logger="backend.app.services.virtual_printer.mqtt_bridge"), + ): bridge._refresh_ip_encoding() not_armed = [r for r in caplog.records if "NOT armed" in r.getMessage()] assert len(not_armed) == 1 - assert "invalid IPv4" in not_armed[0].getMessage() + assert "could not resolve printer host 'not.an.ip'" in not_armed[0].getMessage() def test_successful_arm_clears_dedup_so_future_failure_relogs(self, caplog): """After a successful arm, the dedup must reset so a subsequent