mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-09-30 03:01:21 +02:00
fix(virtual-printer): #1429 follow-up — auto-resolve VP IP when bind_address is 0.0.0.0
The original #1429 fix's _refresh_ip_encoding early-returned when mqtt_server.bind_address was "0.0.0.0" or empty (the default for VPs created without a dedicated bind IP). On a flat-LAN install that's the typical case, so the encoding never armed, _rewrite_net_info_ips was a no-op on every push, and the slicer kept following the real printer IP to its SD card. @Mape6 reported this on the 2026-06-02 daily that supposedly fixed the bug. New helper _resolve_host_interface_for_target() consults the existing network_utils.find_interface_for_ip() to pick the host interface in the printer's subnet. _refresh_ip_encoding falls back to it when bind_address is unspecified; an explicit bind IP still wins. INFO log line distinguishes the two paths ("armed: ... (bind_address)" vs "(auto-resolved)") so future bundles directly answer which IP the rewrite picked. Tests: 4 new under TestBindAddressAutoResolve — rewrite arms via auto-resolved IP at bind_address=0.0.0.0; stays disabled if no interface matches (no crash); explicit bind_ip still takes precedence; helper returns None defensively when find_interface_for_ip does.
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -90,6 +90,33 @@ def _ip_to_uint32_le(ip_str: str) -> int:
|
||||
return parts[0] | (parts[1] << 8) | (parts[2] << 16) | (parts[3] << 24)
|
||||
|
||||
|
||||
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.
|
||||
|
||||
Used when `mqtt_server.bind_address` is empty or 0.0.0.0 — the listener
|
||||
accepts on every interface but we still need ONE concrete IPv4 to write
|
||||
into the rewritten `net.info[].ip` field so the slicer's FTP target
|
||||
resolves to Bambuddy rather than the real printer. Returns the IPv4 of
|
||||
the host interface that shares a subnet with the printer (best fit
|
||||
because the slicer is typically on the same LAN as the printer), or
|
||||
None if no interface matches — in which case the bridge leaves
|
||||
encoding unarmed and the previous (still-leaky) behaviour stands.
|
||||
"""
|
||||
try:
|
||||
from backend.app.services.network_utils import find_interface_for_ip
|
||||
except Exception: # pragma: no cover - import shielding
|
||||
return None
|
||||
try:
|
||||
iface = find_interface_for_ip(target_ip)
|
||||
except Exception:
|
||||
logger.exception("MQTT bridge: find_interface_for_ip(%s) crashed", target_ip)
|
||||
return None
|
||||
if not iface:
|
||||
return None
|
||||
ip = iface.get("ip")
|
||||
return ip if isinstance(ip, str) and ip else None
|
||||
|
||||
|
||||
def _merge_ams_dict(prev_ams: dict, new_ams: dict) -> dict:
|
||||
"""Merge a new ``ams`` blob from an incremental push onto the previous one.
|
||||
|
||||
@@ -354,16 +381,31 @@ class MQTTBridge:
|
||||
printer IP, also sweep the existing cache so the slicer's next pull
|
||||
sees the rewritten value (#1429). Without this sweep the sticky-key
|
||||
preservation keeps the poisoned `net.info[].ip` alive forever.
|
||||
|
||||
VP bind IP resolution: when `mqtt_server.bind_address` is empty or
|
||||
`0.0.0.0` (the default for VPs that were never assigned a dedicated
|
||||
bind IP), fall back to auto-resolving the host interface in the same
|
||||
subnet as the printer's IP. Without this fallback, the rewrite never
|
||||
arms on a default-config flat-LAN install and `net.info[].ip` leaks
|
||||
the real printer IP — slicer follows it on Send (#1429 residual).
|
||||
"""
|
||||
client = self._target_client
|
||||
if client is None:
|
||||
return
|
||||
|
||||
target_ip = getattr(client, "ip_address", None)
|
||||
vp_ip = getattr(self._mqtt_server, "bind_address", None)
|
||||
if not target_ip or not vp_ip or vp_ip in ("0.0.0.0", "", None): # nosec B104
|
||||
if not target_ip:
|
||||
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
|
||||
resolved = _resolve_host_interface_for_target(target_ip)
|
||||
if not resolved:
|
||||
return
|
||||
vp_ip = resolved
|
||||
vp_ip_source = "auto-resolved"
|
||||
|
||||
try:
|
||||
new_target_le = _ip_to_uint32_le(target_ip)
|
||||
new_vp_le = _ip_to_uint32_le(vp_ip)
|
||||
@@ -379,11 +421,12 @@ class MQTTBridge:
|
||||
self._target_ip_uint32_le = new_target_le
|
||||
self._vp_ip_uint32_le = new_vp_le
|
||||
logger.info(
|
||||
"[%s] MQTT bridge IP encoding %s: target=%s vp=%s",
|
||||
"[%s] MQTT bridge IP encoding %s: target=%s vp=%s (%s)",
|
||||
self.vp_name,
|
||||
"updated" if was_armed else "armed",
|
||||
target_ip,
|
||||
vp_ip,
|
||||
vp_ip_source,
|
||||
)
|
||||
|
||||
cached = self._latest_print_state
|
||||
|
||||
@@ -7,7 +7,11 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.app.services.virtual_printer.mqtt_bridge import MQTTBridge, _ip_to_uint32_le
|
||||
from backend.app.services.virtual_printer.mqtt_bridge import (
|
||||
MQTTBridge,
|
||||
_ip_to_uint32_le,
|
||||
_resolve_host_interface_for_target,
|
||||
)
|
||||
from backend.app.services.virtual_printer.mqtt_server import SimpleMQTTServer
|
||||
|
||||
H2D_SERIAL = "0948BB540200427"
|
||||
@@ -1081,3 +1085,102 @@ class TestIpEncoding:
|
||||
def test_invalid_ip_raises(self):
|
||||
with pytest.raises(ValueError):
|
||||
_ip_to_uint32_le("not.an.ip.actually")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Auto-resolve fallback for default-config (bind_address = "0.0.0.0")
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestBindAddressAutoResolve:
|
||||
"""#1429 residual: VPs created without a dedicated bind IP run on
|
||||
`bind_address=0.0.0.0`. The original fix's `_refresh_ip_encoding`
|
||||
early-returned on 0.0.0.0, so the rewrite never armed and `net.info[].ip`
|
||||
kept leaking the real printer IP. Now the bridge auto-resolves a host
|
||||
interface in the printer's subnet and uses that as the VP IP."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rewrite_arms_via_auto_resolved_host_ip(self):
|
||||
"""When bind_address is 0.0.0.0, fall back to the host interface in
|
||||
the target printer's subnet and rewrite to that IP."""
|
||||
server = _make_server(bind_address="0.0.0.0")
|
||||
bridge = _make_bridge(server)
|
||||
with patch(
|
||||
"backend.app.services.virtual_printer.mqtt_bridge._resolve_host_interface_for_target",
|
||||
return_value=VP_IP,
|
||||
):
|
||||
await bridge.start()
|
||||
|
||||
h2d_le = _ip_to_uint32_le(H2D_IP)
|
||||
vp_le = _ip_to_uint32_le(VP_IP)
|
||||
payload = json.dumps(
|
||||
{
|
||||
"print": {
|
||||
"command": "push_status",
|
||||
"net": {"info": [{"ip": h2d_le, "mask": 0xFFFFFF}]},
|
||||
}
|
||||
}
|
||||
).encode()
|
||||
bridge._on_printer_raw(f"device/{H2D_SERIAL}/report", payload)
|
||||
await asyncio.sleep(0.01)
|
||||
|
||||
cached = bridge.get_latest_print_state()
|
||||
assert cached["net"]["info"][0]["ip"] == vp_le
|
||||
assert bridge._vp_ip_uint32_le == vp_le
|
||||
|
||||
await bridge.stop()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rewrite_disabled_when_no_matching_host_interface(self):
|
||||
"""If no host interface shares a subnet with the printer, the bridge
|
||||
cannot pick a sensible VP IP — leave encoding unarmed and let the
|
||||
push through unrewritten (no crash, no wrong rewrite)."""
|
||||
server = _make_server(bind_address="")
|
||||
bridge = _make_bridge(server)
|
||||
with patch(
|
||||
"backend.app.services.virtual_printer.mqtt_bridge._resolve_host_interface_for_target",
|
||||
return_value=None,
|
||||
):
|
||||
await bridge.start()
|
||||
|
||||
h2d_le = _ip_to_uint32_le(H2D_IP)
|
||||
payload = json.dumps(
|
||||
{
|
||||
"print": {
|
||||
"command": "push_status",
|
||||
"net": {"info": [{"ip": h2d_le, "mask": 0xFFFFFF}]},
|
||||
}
|
||||
}
|
||||
).encode()
|
||||
bridge._on_printer_raw(f"device/{H2D_SERIAL}/report", payload)
|
||||
await asyncio.sleep(0.01)
|
||||
|
||||
assert bridge._vp_ip_uint32_le is None
|
||||
assert bridge._target_ip_uint32_le is None
|
||||
|
||||
await bridge.stop()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_explicit_bind_ip_takes_precedence_over_auto_resolve(self):
|
||||
"""Auto-resolve only kicks in when bind_address is empty/0.0.0.0; an
|
||||
explicitly-set bind IP must be used verbatim even if there's also a
|
||||
same-subnet host interface."""
|
||||
server = _make_server(bind_address=VP_IP)
|
||||
bridge = _make_bridge(server)
|
||||
# Auto-resolver would have returned a DIFFERENT IP — we must not use it.
|
||||
with patch(
|
||||
"backend.app.services.virtual_printer.mqtt_bridge._resolve_host_interface_for_target",
|
||||
return_value="10.99.99.99",
|
||||
):
|
||||
await bridge.start()
|
||||
assert bridge._vp_ip_uint32_le == _ip_to_uint32_le(VP_IP)
|
||||
await bridge.stop()
|
||||
|
||||
def test_resolve_helper_returns_none_for_unreachable_target(self):
|
||||
"""The helper itself must be defensive — if `find_interface_for_ip`
|
||||
raises or returns None, we get None (no crash)."""
|
||||
with patch(
|
||||
"backend.app.services.network_utils.find_interface_for_ip",
|
||||
return_value=None,
|
||||
):
|
||||
assert _resolve_host_interface_for_target("203.0.113.1") is None
|
||||
|
||||
Reference in New Issue
Block a user