From a0e7e53f228a0fad466fc92e1583094d32a16830 Mon Sep 17 00:00:00 2001 From: Kay Date: Wed, 16 Sep 2026 10:51:55 +0200 Subject: [PATCH] M7: REST-Transport erstmals live verifiziert, vier Bugs gefunden+gefixt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Erstmaliger REST-Schreibtest gegen echte Hardware (www-ssl vorher auf jedem Testgerät aus, Pfad lief nur gegen Mocks). Vier Bugs gefunden: - baseURL fehlte trailing slash, relative URL-Auflösung warf "/rest" aus jedem Pfad (404, fiel still auf SSH zurück statt Zertifikat- Dialog zu zeigen) - .add nutzte POST statt PUT (RouterOS' REST-API erwartet PUT für neue Einträge) - RouterOS sendet Cache-Control: max-age=31536000 auf jede REST- Antwort, URLSession cachte dadurch die erste (leere) GET-Antwort für den Rest der App-Laufzeit - Löschen aktualisierte die Experte-Tab-Liste nicht in-place (SwiftUI- Render-Problem, kein Datenfehler) — Anlegen/Bearbeiten laufen über ein Sheet, dessen Schließen automatisch neu rendert, Löschen über ein confirmationDialog ohne Remount Anlegen/Bearbeiten (inkl. Feld-Leeren)/Löschen live bestätigt. Co-Authored-By: Claude Sonnet 5 --- HANDOFF.md | 89 +++++++++++++++++-- README.md | 1 + .../Core/Networking/RestTransport.swift | 51 +++++++++-- .../Expert/ExpertMenuDetailView.swift | 10 +++ 4 files changed, 135 insertions(+), 16 deletions(-) diff --git a/HANDOFF.md b/HANDOFF.md index 705873b..4c39afa 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -629,13 +629,79 @@ wiederholen. nur zum Löschen dieser einen Datei erschien nicht aufwandsgerecht. Harmlos (überschreibt sich beim nächsten Restore selbst), aber bewusst in Kauf genommen, nicht vergessen. -- **REST-Pfad ungetestet für Schreibvorgänge** — auf beiden bisherigen - Testgeräten war `www-ssl` (Port 443) aus, jeder Schreibtest lief über - SSH. Der REST-`apply()`-Pfad (`POST`/`PATCH`, `findItemID` für `.set`) - ist nur gegen Mocks getestet. Der REST-Zweig von `fetchFieldValues` - (M12/Bug 14) ist zusätzlich auf einer nirgends offiziell dokumentierten - Annahme aufgebaut (Query-String-Filter `?feld=wert` auf einem GET) — - komplett unverifiziert. +- ~~REST-Pfad ungetestet für Schreibvorgänge~~ — am 2026-09-16 erstmals + live getestet: `www-ssl` auf dem hEX-Testrouter aktiviert (eigene CA + `local-ca` + davon signiertes `rest-cert` angelegt, da direktes + Selbstsignieren ohne separate CA mit "failure: CA not found" + scheiterte — RouterOS braucht dafür `key-usage=key-cert-sign,crl-sign` + auf einem eigenen CA-Zertifikat, das per `ca=` referenziert + wird). Dabei **drei echte, vorher unentdeckte Bugs gefunden und + gefixt**, alle drei ausschließlich möglich, weil kein Testgerät vorher + je `www-ssl` an hatte: + - **Bug 30:** `RestTransport.baseURL` fehlte der abschließende `/` + (`".../rest"` statt `".../rest/"`). `URL(string:relativeTo:)` + ersetzt bei relativer Auflösung das *letzte Pfadsegment* der Basis + (RFC 3986 §5.3) — ohne trailing slash ist "rest" selbst das letzte + Segment, ein Pfad wie "system/resource" ersetzt es also statt + danach angehängt zu werden (`.../rest` + "system/resource" → + `.../system/resource`, 404). Jede REST-Verbindung scheiterte dadurch + mit stillschweigend verschlucktem "HTTP 404" (der generische + `catch`-Zweig in `ConnectionService.connect` loggte den Fehler + nirgends), fiel auf SSH zurück, ohne je den Zertifikats-Dialog zu + zeigen — sah aus wie "kein Zertifikat-Prompt trotz neuem + Zertifikat", war tatsächlich ein komplett anderer, tieferliegender + Bug. Gefunden über einen temporären Debug-`print` im Fallback-Pfad. + - **Bug 31:** `RestTransport.apply()`s `.add`-Fall nutzte `POST`, + RouterOS' REST-API erwartet dafür aber `PUT` — `POST` auf + `/rest/ip/firewall/address-list` lieferte `{"detail":"no such + command","error":400,"message":"Bad Request"}`, `PUT` mit + identischem Body `201` mit vollem Echo des neuen Items. Nebenbei + `send()`s Fehlerbehandlung verbessert: zeigte bisher nur den nackten + Statuscode ("HTTP 400"), jetzt wird RouterOS' eigener JSON-Fehler- + Body (`detail`-Feld) mit ausgegeben, falls vorhanden — genau dieser + Body war es, der den Bug überhaupt auffindbar machte (per direktem + `curl`-Replay derselben Anfrage). + - **Bug 32, am folgenreichsten:** RouterOS sendet auf *jede* + `/rest/...`-Antwort `Cache-Control: max-age=31536000` (ein Jahr) + + passenden `Expires`-Header — vermutlich ein Blanket-Default für + statische Web-UI-Assets, der auch auf die REST-API durchschlägt. + `URLSession(configuration: .ephemeral)` bedeutet nur "nichts auf + Platte persistieren", hält aber weiterhin einen Im-Speicher- + `URLCache` und hält sich an diese Header — die *erste* GET-Antwort + auf einen Pfad (z.B. eine leere Artikelliste, abgerufen durch + `loadCrossReferenceOptions` oder ein früheres Öffnen desselben + Menüs) wurde für den Rest der App-Laufzeit unverändert zurückgegeben, + komplett unabhängig von späteren `.add`/`.set`/`.remove`-Änderungen. + Live-Symptom: ein frisch angelegter Adress-Listen-Eintrag (per + `curl` als real bestätigt) erschien nie in der Experte-Tab-Liste, + auch nach Weg-und-zurück-Navigation nicht. Fix: `urlCache = nil` + + `.reloadIgnoringLocalCacheData` auf Session- *und* Request-Ebene + (RouterOS' Schreib-Endpunkte sind nicht idempotent genug, um sich + nur auf eine der beiden Ebenen zu verlassen). + Nach allen drei Fixes live durchgespielt: Anlegen (`PUT`), Bearbeiten + inkl. Feld-Leeren (`PATCH`, Kommentar gesetzt dann wieder gelöscht — + REST-seitig direkt per `curl` verifiziert, dass ein leerer String das + Feld tatsächlich löscht, genau wie bei SSH/Bug 25), Löschen (`DELETE`) + — alle drei vom Nutzer bestätigt ("funktioniert"). Damit ist der + REST-Transport erstmals end-to-end gegen echte Hardware verifiziert, + nicht mehr nur gegen Mocks. Der REST-Zweig von `fetchFieldValues` + (M12/Bug 14, Query-String-Filter `?feld=wert` auf einem GET) bleibt + weiterhin unverifiziert — dabei nicht auf den Weg gekreuzt. + - **Bug 33, direkt danach gefunden:** Löschen eines Eintrags im + Experte-Tab aktualisierte die sichtbare Liste nicht, bis der Nutzer + in einen anderen Tab und zurück wechselte — sah zunächst wieder wie + ein Cache-/Refresh-Problem aus, war es aber nicht: ein temporärer + Debug-`print` in `confirmRemoval` bestätigte `items.count` geht + korrekt von 1 auf 0 nach `reloadItems()`, das `@Published`-Modell + war also die ganze Zeit richtig. Reines SwiftUI-Render-Problem: + Anlegen/Bearbeiten laufen über `ExpertItemEditView` als `.sheet(item:)`, + dessen Schließen `ExpertMenuDetailView` automatisch neu mountet — + genau das erzwingt implizit den Refresh, den Löschen (über ein + direkt an `ExpertMenuDetailView` gehängtes `.confirmationDialog`, + kein Sheet, kein Remount) nie bekam. Fix: `.id(viewModel.items.count)` + auf der Einträge-`Section`, zwingt SwiftUI bei jeder Mengenänderung + zu einer frischen View-Identität statt sich auf In-Place-Diffing zu + verlassen. Live bestätigt ("funktioniert"). - **`fetchMenuItems`s `.id`-Positions-Überlagerung: Verlässlichkeit für andere Menüs ungeprüft** (siehe Bug 15) — bei `/ip dhcp-server lease` live als falsch bestätigt (`.id` landete auf der falschen Zeile), für @@ -1331,8 +1397,13 @@ passenden Namen lesbar. Live bestätigt ("sieht gut aus"). alles erledigt, live bestätigt ("funktioniert"). Damit sind alle fünf Tabs vollständig zweisprachig (DE/EN). 2. WLAN (M5) an einem Gerät mit echtem WLAN-Chip nachholen. -2. Rest von M7: Fehlerzustände/Politur, REST-Schreibtest an einem Gerät - mit aktivem `www-ssl`. +2. ~~Rest von M7: REST-Schreibtest an einem Gerät mit aktivem + `www-ssl`~~ — erledigt (2026-09-16), siehe "Bekannte Einschränkungen" + oben (Bug 30–32). `www-ssl` bleibt auf dem hEX-Testrouter aktiv + (eigene CA `local-ca` + `rest-cert`), damit die App weiterhin + REST-zuerst verbindet statt SSH-Fallback. Fehlerzustände/Politur im + REST-Pfad noch nicht gezielt geprüft (z.B. Verbindungsabbruch + mitten im Apply) — optional für später. 3. M9 UI (Einfach/Experte-Modusumschalter im Einrichten-Tab selbst) noch manuell durchklicken — M10s Experte-Tab wurde bereits vom Nutzer bestätigt (siehe oben), der Moduswechsel im Wizard noch nicht. diff --git a/README.md b/README.md index 90b5f14..dccd6f8 100644 --- a/README.md +++ b/README.md @@ -151,6 +151,7 @@ nur die zugehörigen Passwörter liegen weiterhin im macOS-Schlüsselbund. | M19 | Übersicht-Tab: Flussanimation + verschiebbare Knoten | ✅ live verifiziert | | M20 | LAN-Scanner: Umbenennung, Netzwerk-Tools (Ping/Traceroute/DNS/Port-Scan) | ✅ live verifiziert | | M21 | Zweisprachigkeit (DE/EN) auf alle fünf Tabs ausgerollt (Einrichten/Übersicht/LAN-Scanner/Sicherungen) | ✅ live verifiziert | +| M22 | REST-Transport (M7) erstmals live gegen Hardware verifiziert, 3 Bugs gefunden+gefixt | ✅ live verifiziert | | — | LAN-Port-Konflikt-Prüfung + "Fertig"-Button (Einrichten) | 🔶 gebaut, Live-Test offen | Ausführlicher Stand inkl. aller gefundenen Bugs, offener Punkte und diff --git a/RouterOSAssistant/Core/Networking/RestTransport.swift b/RouterOSAssistant/Core/Networking/RestTransport.swift index 4cb53fe..1e3c75b 100644 --- a/RouterOSAssistant/Core/Networking/RestTransport.swift +++ b/RouterOSAssistant/Core/Networking/RestTransport.swift @@ -8,19 +8,40 @@ final class RestTransport: NSObject, RouterOSTransport { private let certificateTrust: CertificateTrustStore private var lastRejectedFingerprint: String? - private lazy var session: URLSession = URLSession( - configuration: .ephemeral, - delegate: self, - delegateQueue: nil - ) + /// RouterOS sends `Cache-Control: max-age=31536000` (one year) + a matching `Expires` header + /// on every `/rest/...` response, live-confirmed (2026-09-16) — almost certainly a blanket + /// default meant for static web-UI assets that leaks onto the REST API too. `.ephemeral` + /// only means "no data persisted to disk", it still keeps an in-memory `URLCache` by default + /// and honors those headers — so the *first* GET to a given path (e.g. an empty item list, + /// fetched once by `loadCrossReferenceOptions` or an earlier menu open) got served back for + /// every subsequent identical GET for the rest of the app's lifetime, even after a `.set`/ + /// `.add`/`.remove` changed the underlying data. Symptom, live: a freshly-created address-list + /// entry (confirmed to exist via `curl`) never appeared in the Experte-Tab list, even after + /// navigating away and back. `urlCache = nil` + `.reloadIgnoringLocalCacheData` disable + /// caching at both the session and per-request level, since RouterOS' write endpoints (unlike + /// its GETs) aren't idempotent enough to risk relying on just one of the two. + private lazy var session: URLSession = { + let configuration = URLSessionConfiguration.ephemeral + configuration.urlCache = nil + configuration.requestCachePolicy = .reloadIgnoringLocalCacheData + return URLSession(configuration: configuration, delegate: self, delegateQueue: nil) + }() init(credentials: RouterOSCredentials, certificateTrust: CertificateTrustStore) { self.credentials = credentials self.certificateTrust = certificateTrust } + /// Trailing slash is load-bearing: `URL(string:relativeTo:)` resolves a relative reference + /// like "system/resource" against a base's *last path segment* per RFC 3986 §5.3 — without + /// the trailing "/", that segment is "rest" itself, so the merge replaces it instead of + /// appending after it (`.../rest` + "system/resource" → `.../system/resource`, silently + /// dropping "/rest" and 404ing). Confirmed live (2026-09-16, first-ever successful REST + /// connection to real hardware): app fell back to SSH with a swallowed "HTTP 404" instead of + /// showing the certificate-trust dialog, traced via a temporary debug print in + /// `ConnectionService.connect`. private var baseURL: URL { - URL(string: "https://\(credentials.host):\(credentials.httpsPort)/rest")! + URL(string: "https://\(credentials.host):\(credentials.httpsPort)/rest/")! } func connect() async throws { @@ -108,7 +129,13 @@ final class RestTransport: NSObject, RouterOSTransport { func apply(_ command: RouterOSCommand) async throws { switch command.operation { case .add: - _ = try await send(path: command.restPath, method: "POST", jsonBody: command.arguments) + // RouterOS' REST API creates new entries via PUT, not POST — confirmed live + // (2026-09-16, first-ever REST write test against real hardware): POST returned + // HTTP 400 `{"detail":"no such command","error":400,"message":"Bad Request"}` on + // `/rest/ip/firewall/address-list`, PUT with the identical body succeeded (201, + // full item echoed back). Never caught before — every prior test device had + // `www-ssl` off, so this path only ever ran against mocks (see HANDOFF.md). + _ = try await send(path: command.restPath, method: "PUT", jsonBody: command.arguments) case .set(let matchField, let matchValue): // Empty matchField = singleton menu (e.g. "/ip dns") — PATCH the resource directly, // there's no list/id to look up. See RouterOSCommand.cliLine for the SSH counterpart. @@ -170,6 +197,7 @@ final class RestTransport: NSObject, RouterOSTransport { } var request = URLRequest(url: url) request.httpMethod = method + request.cachePolicy = .reloadIgnoringLocalCacheData let authString = "\(credentials.username):\(credentials.password)" guard let authData = authString.data(using: .utf8) else { @@ -193,6 +221,15 @@ final class RestTransport: NSObject, RouterOSTransport { throw RouterOSError.authenticationFailed } guard (200...299).contains(httpResponse.statusCode) else { + // RouterOS' REST error body is JSON (e.g. `{"detail":"no such command", + // "error":400,"message":"Bad Request"}`) — surface "detail" when present instead + // of just the bare status code, or diagnosing a REST write failure means guessing + // blind (confirmed live, 2026-09-16: the PUT-vs-POST bug below was only found by + // replaying the exact same request with curl, since the app only showed "HTTP 400"). + if let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any], + let detail = object["detail"] as? String { + throw RouterOSError.invalidResponse("HTTP \(httpResponse.statusCode): \(detail)") + } throw RouterOSError.invalidResponse("HTTP \(httpResponse.statusCode)") } return data diff --git a/RouterOSAssistant/Features/Expert/ExpertMenuDetailView.swift b/RouterOSAssistant/Features/Expert/ExpertMenuDetailView.swift index 9ff8840..017d9ae 100644 --- a/RouterOSAssistant/Features/Expert/ExpertMenuDetailView.swift +++ b/RouterOSAssistant/Features/Expert/ExpertMenuDetailView.swift @@ -30,6 +30,15 @@ struct ExpertMenuDetailView: View { } else if viewModel.items.isEmpty { Text(L10n.t("Keine Einträge unter", appLanguage) + " \(schema.menuPath).").foregroundStyle(.secondary) } else { + // `.id(viewModel.items.count)` forces SwiftUI to treat this as a fresh view + // identity whenever the count changes — without it, deleting an item (via the + // `.confirmationDialog` below, not a `.sheet`) updates `viewModel.items` + // correctly (confirmed live via a temporary debug print: item count goes + // 1 → 0) but the rendered list still showed the removed row until switching + // tabs and back forced a full remount. Add/edit never hit this because their + // `.sheet(item:)` dismissal already forces a remount of this view on its own; + // delete's confirmationDialog doesn't tear this view down at all, so nothing + // was forcing SwiftUI to re-diff the Section in place. ForEach(viewModel.items) { item in HStack { Button { @@ -63,6 +72,7 @@ struct ExpertMenuDetailView: View { } } } + .id(viewModel.items.count) } .formStyle(.grouped) .navigationTitle(LocalizedStringKey(L10n.t(schema.displayName, appLanguage)))