From c3422ed0d53e54b0784302e9579e69f2c4ea1846 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Thu, 14 May 2026 22:34:32 +0200 Subject: [PATCH] fix(marge): accept trailing slash on POST /streaming/account/{id}/group/ SoundTouch 10 firmware 27.x posts the addGroup payload to the Marge URL with a trailing slash ("/streaming/account//group/") when the master is forming a stereo pair. AfterTouch only registered the no- slash form, so chi returned 404, the master's MargeClient retried every 15 s, the slave kept connecting to the master's audio transport but was rejected with "Group STP NOT FOUND" because the master never finished AddingMaster, and the group eventually reverted -- the symptom reported in #252. Register POST /group/ alongside POST /group in both Marge route trees (the /marge/streaming/... mount and the bare /streaming/... mount that serves direct device traffic). The GET device-group routes already had both forms; this brings the POST in line. Add TestMargeAddGroup_FromSpeakerCapture, which replays the exact request captured live from BirdyBA's master log: URL with trailing slash, Authorization Bearer header, vendor Content-Type, and the minimal XML body (no , no per-role , no , no numeric group id). The test failed with 404 before this change and now returns 201 Created with the proper Location header, pinning the exact wire contract so future refactors fail loudly. Refs #252 Co-Authored-By: Claude Opus 4.7 (1M context) --- cmd/soundtouch-service/main.go | 5 + .../testdata/router_routes.txt | 2 + pkg/service/handlers/handlers_marge_test.go | 105 ++++++++++++++++++ pkg/service/handlers/main_test.go | 5 + 4 files changed, 117 insertions(+) diff --git a/cmd/soundtouch-service/main.go b/cmd/soundtouch-service/main.go index 7536726..9717f7e 100644 --- a/cmd/soundtouch-service/main.go +++ b/cmd/soundtouch-service/main.go @@ -948,7 +948,11 @@ func setupRouter(server *handlers.Server) *chi.Mux { r.Get("/group/member", server.HandleMargeDeviceGroupMember) }) + // Speakers POST to /group/ (with trailing slash) when forwarding + // the addGroup payload to Marge during stereo-pair formation -- + // see issue #252. Register both forms so chi accepts either. r.Post("/group", server.HandleMargeAddGroup) + r.Post("/group/", server.HandleMargeAddGroup) r.Post("/group/{groupId}", server.HandleMargeModifyGroup) r.Delete("/group/{groupId}", server.HandleMargeDeleteGroup) @@ -997,6 +1001,7 @@ func setupRouter(server *handlers.Server) *chi.Mux { r.Get("/devices/{device}/group/member", server.HandleMargeDeviceGroupMember) r.Post("/group", server.HandleMargeAddGroup) + r.Post("/group/", server.HandleMargeAddGroup) r.Post("/group/{groupId}", server.HandleMargeModifyGroup) r.Delete("/group/{groupId}", server.HandleMargeDeleteGroup) r.Get("/devices/{device}/presets", server.HandleMargePresets) diff --git a/cmd/soundtouch-service/testdata/router_routes.txt b/cmd/soundtouch-service/testdata/router_routes.txt index f8b49ea..bc81325 100644 --- a/cmd/soundtouch-service/testdata/router_routes.txt +++ b/cmd/soundtouch-service/testdata/router_routes.txt @@ -94,6 +94,7 @@ POST /accounts/{account}/devices handlers.( POST /accounts/{account}/devices/{device}/presets/{presetNumber} handlers.(*Server).HandleMargeUpdatePreset-fm POST /accounts/{account}/devices/{device}/recents handlers.(*Server).HandleMargeAddRecent-fm POST /accounts/{account}/group handlers.(*Server).HandleMargeAddGroup-fm +POST /accounts/{account}/group/ handlers.(*Server).HandleMargeAddGroup-fm POST /accounts/{account}/group/{groupId} handlers.(*Server).HandleMargeModifyGroup-fm POST /alexa/certificate handlers.(*Server).HandleAlexaCertificate-fm POST /bmx/core02/svc-bmx-adapter-orion/prod/orion/token handlers.(*Server).HandleOrionToken-fm @@ -141,6 +142,7 @@ POST /streaming/account/{account}/device/{device} handlers.( POST /streaming/account/{account}/device/{device}/presets/{presetNumber} handlers.(*Server).HandleMargeUpdatePreset-fm POST /streaming/account/{account}/device/{device}/recent handlers.(*Server).HandleMargeAddRecent-fm POST /streaming/account/{account}/group handlers.(*Server).HandleMargeAddGroup-fm +POST /streaming/account/{account}/group/ handlers.(*Server).HandleMargeAddGroup-fm POST /streaming/account/{account}/group/{groupId} handlers.(*Server).HandleMargeModifyGroup-fm POST /streaming/account/{account}/source handlers.(*Server).HandleMargeAddSource-fm POST /streaming/device_setting/account/{account}/device/{device}/device_settings handlers.(*Server).HandleMargeUpdateDeviceSettings-fm diff --git a/pkg/service/handlers/handlers_marge_test.go b/pkg/service/handlers/handlers_marge_test.go index ab944ce..f9f1c3f 100644 --- a/pkg/service/handlers/handlers_marge_test.go +++ b/pkg/service/handlers/handlers_marge_test.go @@ -1855,3 +1855,108 @@ func TestMargeGroupCRUD(t *testing.T) { } }) } + +// TestMargeAddGroup_FromSpeakerCapture replays the exact request a SoundTouch +// 10 master sends when it forwards an addGroup to its configured Marge server +// while forming a stereo pair. The shape is taken verbatim from a live capture +// in issue #252; account ID and device IDs are anonymised: +// +// POST /streaming/account/{account}/group/ +// Authorization: Bearer +// Content-Type: application/vnd.bose.streaming-v1.2+xml +// ...TEST +// +// {master}LEFT +// {slave}RIGHT +// +// +// +// Notable differences from CLI-side requests this codebase already tests: +// - URL has a trailing slash ("/group/", not "/group") +// - elements have no +// - is absent (correct for the master-bound payload) +// - Content-Type is the vendor-specific media type +// +// The speaker retries this POST every 15 s while in AddingMaster state; if +// AfterTouch doesn't accept it the group never completes and reverts to +// NoGroup after a timeout. This test pins down the exact wire contract so +// any future change that breaks it fails loudly. +func TestMargeAddGroup_FromSpeakerCapture(t *testing.T) { + tempDir, err := os.MkdirTemp("", "st-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer func() { _ = os.RemoveAll(tempDir) }() + + ds := datastore.NewDataStore(tempDir) + r, _ := setupRouter("http://localhost:8001", ds) + + ts := httptest.NewServer(r) + defer ts.Close() + + const ( + account = "1234567" + masterDevID = "001122334455" + slaveDevID = "AABBCCDDEEFF" + ) + + // Body matches the captured MargeClient payload structure verbatim -- + // no , no per-role , no , no group id. + reqBody := `` + masterDevID + + `TEST` + masterDevID + + `LEFT` + slaveDevID + + `RIGHT` + + url := ts.URL + "/streaming/account/" + account + "/group/" + + req, err := http.NewRequest(http.MethodPost, url, strings.NewReader(reqBody)) + if err != nil { + t.Fatalf("build request: %v", err) + } + + // Headers copied from the captured CMargeHttpInterface::Post lines. + req.Header.Set("Authorization", "Bearer test-token") + req.Header.Set("Content-Type", "application/vnd.bose.streaming-v1.2+xml") + + res, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("do request: %v", err) + } + defer func() { _ = res.Body.Close() }() + + if res.StatusCode != http.StatusCreated { + respBody, _ := io.ReadAll(res.Body) + t.Fatalf("POST %s: expected 201 Created, got %d. Body: %s", url, res.StatusCode, respBody) + } + + if got := res.Header.Get("Content-Type"); got != "application/vnd.bose.streaming-v1.2+xml" { + t.Errorf("response Content-Type = %q, want %q", got, "application/vnd.bose.streaming-v1.2+xml") + } + + location := res.Header.Get("Location") + if !strings.Contains(location, "/account/"+account+"/group/") { + t.Errorf("Location header should reference the new group under account %s, got %q", account, location) + } + + respBody, err := io.ReadAll(res.Body) + if err != nil { + t.Fatalf("read response: %v", err) + } + + var got models.Group + if err := xml.Unmarshal(respBody, &got); err != nil { + t.Fatalf("decode response: %v\nbody: %s", err, respBody) + } + + if got.MasterDeviceID != masterDevID { + t.Errorf("response masterDeviceId = %q, want %q", got.MasterDeviceID, masterDevID) + } + + if got.Name != "TEST" { + t.Errorf("response name = %q, want %q", got.Name, "TEST") + } + + if len(got.Roles.Roles) != 2 { + t.Fatalf("response roles = %d, want 2", len(got.Roles.Roles)) + } +} diff --git a/pkg/service/handlers/main_test.go b/pkg/service/handlers/main_test.go index 41221e2..6deb796 100644 --- a/pkg/service/handlers/main_test.go +++ b/pkg/service/handlers/main_test.go @@ -58,7 +58,11 @@ func setupRouter(targetURL string, ds *datastore.DataStore) (*chi.Mux, *Server) r.Get("/account/{account}/device/{device}/group/", server.HandleMargeDeviceGroup) r.Get("/account/{account}/device/{device}/group/server", server.HandleMargeDeviceGroupServer) r.Get("/account/{account}/device/{device}/group/member", server.HandleMargeDeviceGroupMember) + // Speakers POST to /group/ (with trailing slash) when forwarding the + // addGroup payload to Marge during stereo-pair formation -- see issue + // #252. Register both forms so chi accepts either. r.Post("/account/{account}/group", server.HandleMargeAddGroup) + r.Post("/account/{account}/group/", server.HandleMargeAddGroup) r.Post("/account/{account}/group/{groupId}", server.HandleMargeModifyGroup) r.Delete("/account/{account}/group/{groupId}", server.HandleMargeDeleteGroup) r.Post("/device_setting/account/{account}/device/{device}/device_settings", server.HandleMargeUpdateDeviceSettings) @@ -91,6 +95,7 @@ func setupRouter(targetURL string, ds *datastore.DataStore) (*chi.Mux, *Server) r.Get("/{account}/devices/{device}/group/server", server.HandleMargeDeviceGroupServer) r.Get("/{account}/devices/{device}/group/member", server.HandleMargeDeviceGroupMember) r.Post("/{account}/group", server.HandleMargeAddGroup) + r.Post("/{account}/group/", server.HandleMargeAddGroup) r.Post("/{account}/group/{groupId}", server.HandleMargeModifyGroup) r.Delete("/{account}/group/{groupId}", server.HandleMargeDeleteGroup) }