mirror of
https://github.com/saphid/frame-control.git
synced 2026-10-06 05:02:50 +02:00
Devices: fixes from review round 5
- A switch publishes the new headset at once, so the page clears the old one's panels and the lists behind them (games, store, Android apps, screenshots). - The page names the headset its changes are for (X-Frame-Device); the server refuses one meant for a headset it has switched away from (409). - A rejected address edit changes nothing. - Test now goes to the IPv4 address that answered, like the connection. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
9dd57cde4e
commit
41ac28248a
6 files changed
+62
-19
No files matched your search
@@ -266,6 +266,9 @@ class Registry(Base):
|
||||
self.reg.record_success(d["id"], "192.168.1.40", "n-home", 3.2)
|
||||
self.reg.update_address(d["id"], "192.168.1.40", label="Home")
|
||||
self.assertEqual(self.reg.get(d["id"])["addresses"][1]["networks"], ["n-home"]) # a label keeps what it learned
|
||||
with self.assertRaises(fd.DeviceError):
|
||||
self.reg.update_address(d["id"], "192.168.1.40", new_host="192.168.1.41", label="bad\nlabel")
|
||||
self.assertEqual(self.reg.get(d["id"])["addresses"][1]["networks"], ["n-home"]) # rejected: unchanged
|
||||
self.reg.update_address(d["id"], "192.168.1.40", new_host="192.168.1.41")
|
||||
moved = self.reg.get(d["id"])["addresses"][1]
|
||||
self.assertEqual((moved["host"], moved["networks"], moved["last_ok"]), ("192.168.1.41", [], None))
|
||||
|
||||
+25
-11
@@ -118,6 +118,16 @@ class Connecting(unittest.TestCase):
|
||||
explain=explain)
|
||||
self.addCleanup(self.link.stop)
|
||||
|
||||
def listen6(self):
|
||||
"""A "different device": the same port on IPv6 loopback."""
|
||||
try:
|
||||
six = socket.socket(socket.AF_INET6)
|
||||
self.addCleanup(six.close)
|
||||
six.bind(("::1", self.port))
|
||||
six.listen(4)
|
||||
except OSError:
|
||||
self.skipTest("no IPv6 loopback")
|
||||
|
||||
def pin(self, device_id):
|
||||
fd.known_hosts(device_id).parent.mkdir(parents=True, exist_ok=True)
|
||||
fd.known_hosts(device_id).write_text(f"frame-control-{device_id} ssh-ed25519 AAAA\n")
|
||||
@@ -139,13 +149,7 @@ class Connecting(unittest.TestCase):
|
||||
|
||||
def test_falls_through_to_the_address_that_is_really_the_headset(self):
|
||||
# Tried in this order: a name that doesn't resolve, a different device, the headset.
|
||||
try: # the "different device": the same port on IPv6 loopback
|
||||
six = socket.socket(socket.AF_INET6)
|
||||
self.addCleanup(six.close)
|
||||
six.bind(("::1", self.port))
|
||||
six.listen(4)
|
||||
except OSError:
|
||||
self.skipTest("no IPv6 loopback")
|
||||
self.listen6()
|
||||
d = self.device("nothing.invalid", "::1", "localhost")
|
||||
self.hosts({"::1": "wrong", "localhost": "ok"})
|
||||
self.link.connect(["start"])
|
||||
@@ -258,13 +262,14 @@ class Connecting(unittest.TestCase):
|
||||
self.assertIn("Port=1", self.routes[-1][1])
|
||||
|
||||
def test_test_now_checks_every_address_without_touching_the_connection(self):
|
||||
d = self.device("127.0.0.1", "localhost", "nothing.invalid")
|
||||
self.listen6()
|
||||
d = self.device("::1", "127.0.0.1", "nothing.invalid")
|
||||
self.pin(d["id"])
|
||||
self.hosts({"127.0.0.1": "wrong", "localhost": "ok"})
|
||||
self.hosts({"::1": "wrong", "127.0.0.1": "ok"})
|
||||
self.link.test(d["id"])
|
||||
rows = {r["host"]: r for r in self.link.snapshot()["tests"][d["id"]]["rows"]}
|
||||
self.assertEqual(rows["localhost"]["ssh"], "ok")
|
||||
self.assertEqual(rows["127.0.0.1"]["ssh"], "wrong")
|
||||
self.assertEqual(rows["127.0.0.1"]["ssh"], "ok")
|
||||
self.assertEqual(rows["::1"]["ssh"], "wrong")
|
||||
self.assertEqual(rows["nothing.invalid"]["state"], "unresolved")
|
||||
self.assertEqual(self.routes, [])
|
||||
self.assertTrue(all("ControlPath=none" in c for c in self.calls()))
|
||||
@@ -432,6 +437,15 @@ class ServerConnection(unittest.TestCase):
|
||||
self.assertIn("stages", json.loads(line[6:]))
|
||||
conn.close()
|
||||
|
||||
def test_changes_meant_for_another_headset_are_refused(self):
|
||||
conn = http.client.HTTPConnection("127.0.0.1", self.port, timeout=20)
|
||||
conn.request("POST", "/api/launch", body=b'{"appid": "620"}',
|
||||
headers={"X-Frame-UI": "1", "Content-Type": "application/json", "X-Frame-Device": "someoneelse"})
|
||||
r = conn.getresponse()
|
||||
self.assertEqual(r.status, 409)
|
||||
self.assertIn("switched headsets", json.loads(r.read())["error"])
|
||||
conn.close()
|
||||
|
||||
def test_guards_and_validation(self):
|
||||
self.assertEqual(self.request("GET", "/api/connection", key="")[0], 403)
|
||||
self.assertEqual(self.request("GET", "/api/devices", key="nope")[0], 403)
|
||||
|
||||
+8
-3
@@ -562,15 +562,20 @@ class Registry:
|
||||
with self.lock:
|
||||
d = self._find(device_id)
|
||||
a = self._addr(d, host)
|
||||
if new_host is not None and new_host != host:
|
||||
# Check everything first: a rejected edit changes nothing.
|
||||
moved = new_host is not None and new_host != host
|
||||
if moved:
|
||||
new_host = check_host(new_host)
|
||||
if any(x["host"] == new_host for x in d["addresses"]):
|
||||
raise DeviceError(f"{new_host} is already on the list")
|
||||
kind = None if kind is None else check_kind(kind)
|
||||
label = None if label is None else check_text(label, "label")
|
||||
if moved:
|
||||
a.update(host=new_host, networks=[], last_ok=None, last_rtt_ms=None) # a new place: learn again
|
||||
if kind is not None:
|
||||
a["kind"] = check_kind(kind)
|
||||
a["kind"] = kind
|
||||
if label is not None:
|
||||
a["label"] = check_text(label, "label")
|
||||
a["label"] = label
|
||||
self.save()
|
||||
return copy.deepcopy(a)
|
||||
|
||||
|
||||
+11
-4
@@ -125,6 +125,11 @@ def probe(host, port, timeout=PROBE_TIMEOUT, update=None):
|
||||
return last or {"state": "timeout", "detail": "No answer"}
|
||||
|
||||
|
||||
def ssh_target(host, ip):
|
||||
"""Where ssh should go for an address whose probe answered from `ip`."""
|
||||
return ip if ip and re.fullmatch(r"\d{1,3}(\.\d{1,3}){3}", ip) else host
|
||||
|
||||
|
||||
def probe_raw(host, port, result):
|
||||
"""ssh's own wording for a failed probe, so the server's UNREACHABLE table explains it."""
|
||||
return {"unresolved": f"ssh: Could not resolve hostname {host}: not found",
|
||||
@@ -269,7 +274,9 @@ class Link:
|
||||
self.apply(device["alias"], self.first_route(device))
|
||||
self.routed = None # the next attempt routes again
|
||||
with self.cond:
|
||||
self.state["phase"] = "connecting"
|
||||
# Say so at once: the page clears the old headset's panels when the device changes.
|
||||
self.state.update(phase="connecting", device=self.public_device(device), via=None, error=None,
|
||||
retry_at=None, probes=[], stages=[])
|
||||
self.kicks.append("switch")
|
||||
self.version += 1
|
||||
self.cond.notify_all()
|
||||
@@ -650,8 +657,7 @@ class Link:
|
||||
# An IPv4 address that answered is used as is, so ssh doesn't look the name up
|
||||
# again and try an address that didn't answer (a dead IPv6 route, say). IPv6
|
||||
# answers keep the name: a link-local one needs its zone, which ssh adds itself.
|
||||
ip = found.get("ip") or ""
|
||||
opts = self.host_opts(device, ip if re.fullmatch(r"\d{1,3}(\.\d{1,3}){3}", ip) else a["host"])
|
||||
opts = self.host_opts(device, ssh_target(a["host"], found.get("ip")))
|
||||
alias = device["alias"]
|
||||
with self.route_lock:
|
||||
if self.attempt_gen != self.gen:
|
||||
@@ -818,7 +824,8 @@ class Link:
|
||||
rows[i]["ssh"] = "checking"
|
||||
put()
|
||||
argv = [*self.mux_base[:3], "-o", "ControlPath=none", "-o", "ConnectTimeout=8",
|
||||
*self.host_opts(device, a["host"]), "-o", "StrictHostKeyChecking=yes", device["alias"], "true"]
|
||||
*self.host_opts(device, ssh_target(a["host"], res.get("ip"))),
|
||||
"-o", "StrictHostKeyChecking=yes", device["alias"], "true"]
|
||||
try:
|
||||
r = subprocess.run(argv, capture_output=True, stdin=subprocess.DEVNULL, text=True,
|
||||
errors="replace", timeout=20)
|
||||
|
||||
+9
-1
@@ -925,8 +925,11 @@ const savesToDevice = () => !!(window.frameApp && window.frameApp.saveImages);
|
||||
let devGen = 0;
|
||||
async function api(path, body) {
|
||||
const gen = devGen;
|
||||
// Changes name the headset the page was showing, so the server refuses one meant
|
||||
// for a headset it has since switched away from.
|
||||
const dev = window.connDevice ? { "X-Frame-Device": window.connDevice } : {};
|
||||
const opts = body === undefined ? { headers: {"X-Frame-UI": UI_KEY} } : {
|
||||
method: "POST", headers: {"Content-Type": "application/json", "X-Frame-UI": UI_KEY}, body: JSON.stringify(body) };
|
||||
method: "POST", headers: {"Content-Type": "application/json", "X-Frame-UI": UI_KEY, ...dev}, body: JSON.stringify(body) };
|
||||
let r;
|
||||
try { r = await fetch(path, opts); }
|
||||
catch {
|
||||
@@ -1536,6 +1539,7 @@ function upload(file, mode) {
|
||||
const xhr = new XMLHttpRequest();
|
||||
xhr.open("POST", "/api/upload");
|
||||
xhr.setRequestHeader("X-Frame-UI", UI_KEY);
|
||||
if (window.connDevice) xhr.setRequestHeader("X-Frame-Device", window.connDevice);
|
||||
xhr.setRequestHeader("X-Filename", encodeURIComponent(file.name));
|
||||
xhr.setRequestHeader("X-Mode", mode);
|
||||
const bar = $("prog").firstElementChild;
|
||||
@@ -2527,6 +2531,7 @@ function onConnection(s) {
|
||||
if (!link.live) return;
|
||||
renderPill(); renderConnDlg();
|
||||
const devId = s.device && s.device.id;
|
||||
window.connDevice = devId || null;
|
||||
if (link.device !== null && devId !== link.device) {
|
||||
log(`Now using ${s.device.name}`, "ok");
|
||||
state = null;
|
||||
@@ -2545,6 +2550,9 @@ function onConnection(s) {
|
||||
if ($(id)) $(id).innerHTML = `<div class="wait">Waiting for ${esc(s.device.name)}</div>`;
|
||||
});
|
||||
disp.list = []; disp.port = null;
|
||||
// The lists behind those panels too, so filters can't bring the old ones back.
|
||||
gm.owned = null; gm.byId = new Map(); gm.results = []; gm.store = []; gm.storeQ = null; gm.shown = 0; gm.seq++;
|
||||
androidApps = []; shots.list = [];
|
||||
$("dispSel").innerHTML = "<option>Loading…</option>"; $("dispSel").disabled = true; $("dispCtl").hidden = true;
|
||||
link.reload = true; // everything on the page was the other headset's
|
||||
$("battChip").hidden = true;
|
||||
|
||||
@@ -1468,6 +1468,12 @@ class Handler(BaseHTTPRequestHandler):
|
||||
if not self.local_request():
|
||||
return
|
||||
path = urlparse(self.path).path
|
||||
# A change the page made for a headset the app has since switched away from
|
||||
# (its buttons were still showing): refuse it rather than do it to this one.
|
||||
meant = self.headers.get("X-Frame-Device")
|
||||
if LINK and meant and path != "/api/devices" and meant != LINK.active_device()["id"]:
|
||||
self.send_json({"error": "Frame Control switched headsets; try again on this one"}, 409)
|
||||
return
|
||||
try:
|
||||
if path == "/api/upload":
|
||||
with working():
|
||||
|
||||
Reference in new issue
Block a user