From d9cd40c035985537a714774ec730e99338769403 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:06:14 +1000 Subject: [PATCH] Remote desktop: review fixes - Write the .rdp file through open(newline=), since Path.write_text(newline=) needs Python 3.10 and CI's checks job runs 3.9 - One .rdp file per address, so overlapping launches can't swap headsets - xrdp not answering is NotListening, a 400 with its message rather than a 500 filed as an error diagnostic - /source-image/ lets ClientGone through instead of answering 404 mid-reply Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_rdp.py | 18 +++++++++++++++++- ui/frame_host.py | 17 ++++++++++++----- ui/server.py | 4 ++++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/tests/test_rdp.py b/tests/test_rdp.py index 814517e..1f5ee4a 100644 --- a/tests/test_rdp.py +++ b/tests/test_rdp.py @@ -71,12 +71,28 @@ class OpenRdp(unittest.TestCase): def test_nothing_listening_says_why_and_opens_nothing(self): self.xrdp.close() for name in ("windows", "mac", "linux"): - with self.subTest(name), platform(name), self.assertRaises(frame_host.HostError) as cm: + with self.subTest(name), platform(name), self.assertRaises(frame_host.NotListening) as cm: frame_host.open_rdp("frame", "127.0.0.1") self.assertIn("Developer Mode", str(cm.exception)) self.assertIn(f"port {frame_host.RDP_PORT}", str(cm.exception)) self.assertEqual(self.spawned, []) + def test_server_says_it_as_the_persons_to_fix(self): + # A 400 with the message, not a 500 filed as an error diagnostic. + self.xrdp.close() + with mock.patch.multiple(server, LOCAL=False, LINK=None, HOST_OPTS=["-o", "HostName=127.0.0.1"]), \ + self.assertRaises(server.Failure) as cm: + server.open_thing({"what": "rdp"}) + self.assertEqual(cm.exception.status, 400) + self.assertIn("Developer Mode", str(cm.exception)) + + def test_one_file_per_address(self): + with platform("windows"): + a, b = frame_host.rdp_file("192.168.1.5"), frame_host.rdp_file("fe80::1%eth0") + self.assertNotEqual(a, b) + self.assertIn(b"full address:s:192.168.1.5\r\n", a.read_bytes()) + self.assertIn(b"full address:s:fe80::1%eth0\r\n", b.read_bytes()) + def test_address_cant_add_lines_to_the_file(self): with platform("windows"), self.assertRaises(frame_host.HostError): frame_host.rdp_file("frame\r\nusername:s:root") diff --git a/ui/frame_host.py b/ui/frame_host.py index 9407de9..c618ebb 100644 --- a/ui/frame_host.py +++ b/ui/frame_host.py @@ -6,6 +6,7 @@ CLI (used by the Electron app, so terminal handling lives in one place): python3 ui/frame_host.py terminal -- CMD [ARG...] # open CMD in a terminal window """ import os +import re import shlex import shutil import socket @@ -33,6 +34,10 @@ class HostError(RuntimeError): pass +class NotListening(HostError): + """The Frame answers, but a service on it doesn't: something the person can turn on.""" + + def data_dir(*parts): """Per-user app data: ~/Library/Application Support, %APPDATA% or $XDG_DATA_HOME (or $FRAME_CONTROL_DATA_DIR, which the tests point at a throwaway directory).""" @@ -286,9 +291,11 @@ def rdp_file(host): computer's Windows account, which xrdp turns away; the file names steamos instead.""" if any(c in host for c in "\r\n"): raise HostError("That headset address can't be used for remote desktop") - path = cache_dir("frame.rdp") + # One file per address, so two launches close together can't swap headsets. + path = cache_dir(f"frame-{re.sub(r'[^A-Za-z0-9.-]', '_', host)}.rdp") path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(f"full address:s:{host}\nusername:s:{RDP_USER}\n", encoding="utf-8", newline="\r\n") + with open(path, "w", encoding="utf-8", newline="\r\n") as f: # Path.write_text(newline=) is 3.10+ + f.write(f"full address:s:{host}\nusername:s:{RDP_USER}\n") return path @@ -297,9 +304,9 @@ def open_rdp(alias, host=None): host = host or ssh_hostname(alias) # The client would open either way and then fail on its own, with nothing said here. if not rdp_reachable(host): - raise HostError(f"The Frame isn't accepting remote desktop at {host} (nothing answered on port " - f"{RDP_PORT}). Turn on Developer Mode in Steam Settings > System on the headset, " - "then restart it and try again.") + raise NotListening(f"The Frame isn't accepting remote desktop at {host} (nothing answered on port " + f"{RDP_PORT}). Turn on Developer Mode in Steam Settings > System on the headset, " + "then restart it and try again.") if MAC: if subprocess.run(["open", "-a", "Windows App"], capture_output=True).returncode == 0: return f"Opened Windows App: connect to {host} and {RDP_LOGIN}" diff --git a/ui/server.py b/ui/server.py index 52bc1e5..f1c798d 100755 --- a/ui/server.py +++ b/ui/server.py @@ -1192,6 +1192,8 @@ def open_thing(body): SHOTS_DIR.mkdir(parents=True, exist_ok=True) frame_host.open_path(SHOTS_DIR) return {"message": f"Opened {SHOTS_DIR} in {frame_host.FILE_MANAGER}"} + except frame_host.NotListening as e: + raise Failure(str(e), 400) # theirs to turn on; nothing failed here except frame_host.HostError as e: raise Failure(str(e), 500) raise Failure("unknown target", 400) @@ -2338,6 +2340,8 @@ class Handler(BaseHTTPRequestHandler): from apk_sources import _images try: self.send_bytes(*_images.image(path.rsplit("/", 1)[-1])) + except ClientGone: + raise except Exception: self.send_json({"error": "Artwork unavailable"}, 404) elif path == "/api/sources/details":