diff --git a/cmd/soundtouch-service/main.go b/cmd/soundtouch-service/main.go index 0211d9b..9bc2fce 100644 --- a/cmd/soundtouch-service/main.go +++ b/cmd/soundtouch-service/main.go @@ -936,6 +936,16 @@ func setupRouter(server *handlers.Server) *chi.Mux { r.Get("/presets/all", server.HandleMargeAccountPresets) r.Get("/provider_settings", server.HandleMargeProviderSettings) + // All `/device` routes share one chi subrouter. Two + // overlapping subrouters (`/device` + `/device/{device}`) + // caused chi's radix-tree resolver to bind a runtime + // request to the more-specific prefix even when only the + // less-specific subrouter had a matching method handler, + // producing the [UNHANDLED] → upstream-proxy fall-through + // behind issue #285's first-attempted fix. One subrouter + // keeps every device-scoped path resolvable; see + // TestPUTRenameRoutesToLocalHandler for the regression + // against the production router. r.Route("/device", func(r chi.Router) { r.Post("/", server.HandleMargeAddDevice) r.Post("/{device}", server.HandleMargeAddDevice) @@ -944,21 +954,20 @@ func setupRouter(server *handlers.Server) *chi.Mux { // when the user renames via Bose App or // `soundtouch-cli name set`. Issue #285. r.Put("/{device}", server.HandleMargeUpdateDevice) - }) + r.Delete("/{device}", server.HandleMargeRemoveDevice) - r.Route("/device/{device}", func(r chi.Router) { - r.Get("/presets", server.HandleMargePresets) - r.Post("/presets/{presetNumber}", server.HandleMargeUpdatePreset) - r.Put("/preset/{presetNumber}", server.HandleMargeUpdatePreset) - r.Delete("/preset/{presetNumber}", server.HandleMargeRemovePreset) - r.Get("/recent", server.HandleMargeRecents) - r.Get("/recents", server.HandleMargeRecents) - r.Post("/recent", server.HandleMargeAddRecent) + r.Get("/{device}/presets", server.HandleMargePresets) + r.Post("/{device}/presets/{presetNumber}", server.HandleMargeUpdatePreset) + r.Put("/{device}/preset/{presetNumber}", server.HandleMargeUpdatePreset) + r.Delete("/{device}/preset/{presetNumber}", server.HandleMargeRemovePreset) + r.Get("/{device}/recent", server.HandleMargeRecents) + r.Get("/{device}/recents", server.HandleMargeRecents) + r.Post("/{device}/recent", server.HandleMargeAddRecent) - r.Get("/group", server.HandleMargeDeviceGroup) - r.Get("/group/", server.HandleMargeDeviceGroup) - r.Get("/group/server", server.HandleMargeDeviceGroupServer) - r.Get("/group/member", server.HandleMargeDeviceGroupMember) + r.Get("/{device}/group", server.HandleMargeDeviceGroup) + r.Get("/{device}/group/", server.HandleMargeDeviceGroup) + r.Get("/{device}/group/server", server.HandleMargeDeviceGroupServer) + r.Get("/{device}/group/member", server.HandleMargeDeviceGroupMember) }) // Speakers POST to /group/ (with trailing slash) when forwarding @@ -968,8 +977,6 @@ func setupRouter(server *handlers.Server) *chi.Mux { r.Post("/group/", server.HandleMargeAddGroup) r.Post("/group/{groupId}", server.HandleMargeModifyGroup) r.Delete("/group/{groupId}", server.HandleMargeDeleteGroup) - - r.Delete("/device/{device}", server.HandleMargeRemoveDevice) }) r.Get("/device/{device}/streaming_token", server.HandleMargeStreamingToken) diff --git a/cmd/soundtouch-service/router_test.go b/cmd/soundtouch-service/router_test.go index d11db27..fc3a735 100644 --- a/cmd/soundtouch-service/router_test.go +++ b/cmd/soundtouch-service/router_test.go @@ -3,6 +3,7 @@ package main import ( "fmt" "net/http" + "net/http/httptest" "os" "reflect" "runtime" @@ -10,6 +11,7 @@ import ( "strings" "testing" + "github.com/gesellix/bose-soundtouch/pkg/service/datastore" "github.com/gesellix/bose-soundtouch/pkg/service/handlers" "github.com/go-chi/chi/v5" ) @@ -102,3 +104,56 @@ func TestPrintRoutes(t *testing.T) { t.Errorf("Router routes changed! Diff the snapshot at %s with %s", snapshotPath, actualPath) } } + +// TestPUTRenameRoutesToLocalHandler reproduces the runtime routing +// behaviour the user saw on their deployed v0.80.0: a PUT to +// /streaming/account/{a}/device/{d} should land on +// HandleMargeUpdateDevice, not fall through to the [UNHANDLED] +// proxy. The handlers-package test (TestIssue285_*) uses a simplified +// router that doesn't have the overlapping `/device` and +// `/device/{device}` route groups, so it can't catch a chi radix- +// tree resolution that prefers the more-specific subrouter. +// +// This test exercises the actual production setupRouter so a +// regression in the route topology is caught against the same chi +// behaviour speakers will see. +func TestPUTRenameRoutesToLocalHandler(t *testing.T) { + tempDir, err := os.MkdirTemp("", "router-rename-") + if err != nil { + t.Fatalf("mkdir temp: %v", err) + } + defer os.RemoveAll(tempDir) + + ds := datastore.NewDataStore(tempDir) + _ = ds.Initialize() + + server := handlers.NewServer(ds, nil, "http://localhost:8000", false, false, false) + r := setupRouter(server) + ts := httptest.NewServer(r) + defer ts.Close() + + body := `Sound MachinechenA81B6A536A98` + + req, err := http.NewRequest(http.MethodPut, + ts.URL+"/streaming/account/1111111/device/A81B6A536A98", + strings.NewReader(body)) + if err != nil { + t.Fatalf("build request: %v", err) + } + + req.Header.Set("Content-Type", "application/xml") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("PUT: %v", err) + } + defer func() { _ = resp.Body.Close() }() + + // 200 means our local HandleMargeUpdateDevice handled it. + // 401 / 502 / anything else means the request fell through to + // the [UNHANDLED] proxy and got the upstream response — which + // is exactly the failure mode #285 was supposed to fix. + if resp.StatusCode != http.StatusOK { + t.Fatalf("PUT status = %d, want 200 (local handler). Anything else means the request fell through to [UNHANDLED] proxy — chi is routing to a different subrouter than the PUT registration intended.", resp.StatusCode) + } +} diff --git a/cmd/soundtouch-service/testdata/router_routes.txt b/cmd/soundtouch-service/testdata/router_routes.txt index 4995f77..a21fab3 100644 --- a/cmd/soundtouch-service/testdata/router_routes.txt +++ b/cmd/soundtouch-service/testdata/router_routes.txt @@ -8,6 +8,7 @@ DELETE /setup/dns-discoveries handlers.( DELETE /setup/interactions/sessions handlers.(*Server).HandleCleanupSessions-fm DELETE /setup/interactions/sessions/{session} handlers.(*Server).HandleDeleteSession-fm DELETE /setup/parity-mismatches handlers.(*Server).HandleClearParityMismatches-fm +DELETE /streaming/account/{account}/device/{device} handlers.(*Server).HandleMargeRemoveDevice-fm DELETE /streaming/account/{account}/device/{device}/preset/{presetNumber} handlers.(*Server).HandleMargeRemovePreset-fm DELETE /streaming/account/{account}/group/{groupId} handlers.(*Server).HandleMargeDeleteGroup-fm GET / handlers.(*Server).HandleRoot-fm