From ae21552878ab113201b3f206949f9dc3dd13bf0a Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Sun, 10 May 2026 20:27:25 +0200 Subject: [PATCH] fix(setup,web): parse the protobuf-text getpdo reply real devices send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The live SoundTouch firmware (FW 27.0.6.46330.5043500, ST 20) replies to `getpdo CurrentSystemConfiguration` with a Protobuf-text-like nested-block format, not the key=value format my parser was written against: margeServerUrl { text: "https://streaming.bose.com" } statsServerUrl { text: "https://events.api.bosecm.com" } ... ->OK -> Effect of the bug: the four "Current on Device" cells in the telnet URL Targets table stayed empty after a summary load, and the crossCheckPreflights helper silently produced no warnings even when SSH-XML and telnet-getpdo would have disagreed. Both behaviours were reported from a real-device summary fetched against the running service. Both parsers (Go setup.parseGetpdoConfig and JS parseTelnetVerifiedConfig) now accept the protobuf-text shape and keep the legacy key=value path as a tolerance fallback. An isIdentifier guard prevents protobuf "text: …" lines from being misread as flat fields and keeps prompt characters (->, ->OK) out of the result map. A new TestParseGetpdoConfig_ProtobufTextRealDevice test pins the parser to the verbatim live response so this regression cannot recur silently. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/service/handlers/web/js/script.js | 49 +++++++++-- pkg/service/setup/preflight_crosscheck.go | 81 ++++++++++++++++--- .../setup/preflight_crosscheck_test.go | 52 ++++++++++++ 3 files changed, 164 insertions(+), 18 deletions(-) diff --git a/pkg/service/handlers/web/js/script.js b/pkg/service/handlers/web/js/script.js index 75b8070..a9aea78 100644 --- a/pkg/service/handlers/web/js/script.js +++ b/pkg/service/handlers/web/js/script.js @@ -2486,22 +2486,55 @@ function renderTelnetPreflight(summary) { } } -// parseTelnetVerifiedConfig extracts key=value pairs from the device's +// parseTelnetVerifiedConfig extracts field values from the device's // `getpdo CurrentSystemConfiguration` reply. Mirrors -// setup.parseGetpdoConfig (Go) — see that function's docstring for the -// tolerance contract. +// setup.parseGetpdoConfig (Go); supports both the protobuf-text-like +// nested-block format observed on FW 27.0.6 (`key { text: "value" }`) +// and the flat key=value format kept as a tolerance path. Banner text, +// prompt characters (`->`, `->OK`), and unrelated lines are silently +// ignored. function parseTelnetVerifiedConfig(text) { const out = {}; if (!text) return out; + + const isIdentifier = (s) => !!s && /^[A-Za-z0-9_]+$/.test(s); + + let currentKey = ""; for (const raw of text.split("\n")) { const line = raw.trim(); if (!line) continue; - const i = line.indexOf("="); - if (i <= 0) continue; - const key = line.slice(0, i).trim(); - const val = line.slice(i + 1).trim(); - if (key) out[key] = val; + + // Block open: " {". + if (line.endsWith("{")) { + const head = line.slice(0, -1).trim(); + if (isIdentifier(head)) currentKey = head; + continue; + } + + // Block close. + if (line === "}") { + currentKey = ""; + continue; + } + + // "text: ..." inside a block is the field value. + if (currentKey && line.startsWith("text:")) { + let val = line.slice("text:".length).trim(); + if (val.startsWith('"') && val.endsWith('"')) val = val.slice(1, -1); + out[currentKey] = val; + continue; + } + + // Flat key=value (tolerance path). + const eq = line.indexOf("="); + if (eq > 0) { + const key = line.slice(0, eq).trim(); + if (isIdentifier(key)) { + out[key] = line.slice(eq + 1).trim(); + } + } } + return out; } diff --git a/pkg/service/setup/preflight_crosscheck.go b/pkg/service/setup/preflight_crosscheck.go index e01d8ac..adbbbb2 100644 --- a/pkg/service/setup/preflight_crosscheck.go +++ b/pkg/service/setup/preflight_crosscheck.go @@ -52,31 +52,92 @@ func (m *Manager) crossCheckPreflights(summary *MigrationSummary) { } } -// parseGetpdoConfig extracts key=value pairs from `getpdo CurrentSystemConfiguration` -// output. The format observed in the wild is one pair per line; any line -// that does not match key=value is silently skipped, so the parser is -// tolerant to banner text or trailing prompt characters. +// parseGetpdoConfig extracts field values from a `getpdo +// CurrentSystemConfiguration` reply. Two formats are accepted: +// +// 1. Protobuf-text-like nested blocks (the format observed on FW +// 27.0.6 ST 10/20/300 in the wild): +// +// margeServerUrl { +// text: "https://streaming.bose.com" +// } +// +// 2. Flat key=value lines (kept as a tolerance path for firmware +// variants that report differently or for hand-crafted test +// fixtures). +// +// Any line that doesn't match either shape is silently ignored, so the +// parser tolerates banner text, prompt characters (`->`, `->OK`), +// blank lines, and unrelated fields. func parseGetpdoConfig(text string) map[string]string { out := map[string]string{} + var currentKey string + for _, raw := range strings.Split(text, "\n") { line := strings.TrimSpace(raw) if line == "" { continue } - i := strings.IndexByte(line, '=') - if i <= 0 { + // Block open: " {". + if strings.HasSuffix(line, "{") { + head := strings.TrimSpace(strings.TrimSuffix(line, "{")) + if head != "" && isIdentifier(head) { + currentKey = head + } + continue } - key := strings.TrimSpace(line[:i]) - val := strings.TrimSpace(line[i+1:]) + // Block close. + if line == "}" { + currentKey = "" + continue + } - if key != "" { - out[key] = val + // "text: ..." inside a block is the field value. + if currentKey != "" && strings.HasPrefix(line, "text:") { + val := strings.TrimSpace(strings.TrimPrefix(line, "text:")) + val = strings.Trim(val, `"`) + out[currentKey] = val + + continue + } + + // Flat key=value, only if the key is a bare identifier (so we + // don't misread protobuf "text: value" as a key=value pair via + // some other separator). + if i := strings.IndexByte(line, '='); i > 0 { + key := strings.TrimSpace(line[:i]) + if key != "" && isIdentifier(key) { + out[key] = strings.TrimSpace(line[i+1:]) + } } } return out } + +// isIdentifier reports whether s looks like a configuration field name — +// alphanumeric or underscore only. Used to keep parseGetpdoConfig from +// promoting random "x: y" or "x = y" lines (with spaces, punctuation, +// arrows) into the result map. +func isIdentifier(s string) bool { + if s == "" { + return false + } + + for _, r := range s { + switch { + case r >= 'a' && r <= 'z': + case r >= 'A' && r <= 'Z': + case r >= '0' && r <= '9': + case r == '_': + default: + return false + } + } + + return true +} diff --git a/pkg/service/setup/preflight_crosscheck_test.go b/pkg/service/setup/preflight_crosscheck_test.go index a67b2eb..0bb877a 100644 --- a/pkg/service/setup/preflight_crosscheck_test.go +++ b/pkg/service/setup/preflight_crosscheck_test.go @@ -37,6 +37,58 @@ func TestParseGetpdoConfig_TolerantToNoise(t *testing.T) { } } +// TestParseGetpdoConfig_ProtobufTextRealDevice pins the parser to the +// live response captured from a SoundTouch 20 (FW 27.0.6.46330.5043500) +// against http://mac.fritz.box:8000/setup/summary. This is the format +// the parser actually has to handle in production — the prior +// key=value-only implementation returned an empty map for this input, +// which surfaced as empty "Current on Device" cells in the migration +// UI. +func TestParseGetpdoConfig_ProtobufTextRealDevice(t *testing.T) { + in := `margeServerUrl { + text: "https://streaming.bose.com" +} +statsServerUrl { + text: "https://events.api.bosecm.com" +} +swUpdateUrl { + text: "https://worldwide.bose.com/updates/soundtouch" +} +isZeroconfEnabled { + text: true +} +usePandoraProductionServer { + text: true +} +saveMargeCustomerReport { + text: false +} +bmxRegistryUrl { + text: "https://content.api.bose.io/bmx/registry/v1/services" +} + +->OK +->` + + got := parseGetpdoConfig(in) + + want := map[string]string{ + "margeServerUrl": "https://streaming.bose.com", + "statsServerUrl": "https://events.api.bosecm.com", + "swUpdateUrl": "https://worldwide.bose.com/updates/soundtouch", + "bmxRegistryUrl": "https://content.api.bose.io/bmx/registry/v1/services", + "isZeroconfEnabled": "true", + "usePandoraProductionServer": "true", + "saveMargeCustomerReport": "false", + } + + for k, v := range want { + if got[k] != v { + t.Errorf("%s = %q, want %q", k, got[k], v) + } + } +} + func TestCrossCheckPreflights_AgreementProducesNoWarnings(t *testing.T) { m := &Manager{ServerURL: "http://example:8000"}