From ca2e2f92571cbb55c9483ad6a00adf399460a63f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lipinsk=C3=BD?= <6032558+Mr-Tao@users.noreply.github.com> Date: Thu, 27 Aug 2026 00:40:50 +0200 Subject: [PATCH] fix(player): guard multiroom zone creation --- pkg/service/soundtouchweb/handler.go | 46 ++++++ pkg/service/soundtouchweb/handler_test.go | 146 ++++++++++++++++++ .../static/js/components/Zone.js | 28 +++- 3 files changed, 216 insertions(+), 4 deletions(-) diff --git a/pkg/service/soundtouchweb/handler.go b/pkg/service/soundtouchweb/handler.go index 6cab50de..698b7cba 100644 --- a/pkg/service/soundtouchweb/handler.go +++ b/pkg/service/soundtouchweb/handler.go @@ -1079,6 +1079,10 @@ func (app *WebApp) HandleGetZone(w http.ResponseWriter, r *http.Request) { func (app *WebApp) HandleZoneAdd(w http.ResponseWriter, r *http.Request) { masterIP := chi.URLParam(r, "id") slaveIP := chi.URLParam(r, "slaveId") + if masterIP == slaveIP { + app.sendError(w, "A device cannot be added to its own zone", http.StatusBadRequest) + return + } masterConn, ok := app.GetDevice(masterIP) if !ok { @@ -1096,6 +1100,27 @@ func (app *WebApp) HandleZoneAdd(w http.ResponseWriter, r *http.Request) { app.sendError(w, "Device not ready", http.StatusInternalServerError) return } + if masterConn.DeviceInfo.DeviceID == slaveConn.DeviceInfo.DeviceID { + app.sendError(w, "A device cannot be added to its own zone", http.StatusBadRequest) + return + } + + nowPlaying, err := masterConn.Client.GetNowPlaying() + if err != nil { + app.sendError(w, err.Error(), http.StatusInternalServerError) + return + } + + sources, err := masterConn.Client.GetSources() + if err != nil { + app.sendError(w, err.Error(), http.StatusInternalServerError) + return + } + + if !currentSourceAllowsMultiroom(nowPlaying, sources) { + app.sendError(w, "Start a multiroom-capable source before grouping speakers", http.StatusConflict) + return + } masterHwID := masterConn.DeviceInfo.DeviceID slaveHwID := slaveConn.DeviceInfo.DeviceID @@ -1119,6 +1144,27 @@ func (app *WebApp) HandleZoneAdd(w http.ResponseWriter, r *http.Request) { app.sendControlResponse(w, masterConn.Client.SetZone(zoneReq), "Device added to zone") } +func currentSourceAllowsMultiroom(nowPlaying *models.NowPlaying, sources *models.Sources) bool { + if nowPlaying == nil || sources == nil { + return false + } + + source := strings.TrimSpace(nowPlaying.Source) + if source == "" || source == "STANDBY" || source == "INVALID_SOURCE" { + return false + } + + for i := range sources.SourceItem { + item := &sources.SourceItem[i] + if item.Source == source && item.MultiroomAllowed && + (nowPlaying.SourceAccount == "" || item.SourceAccount == nowPlaying.SourceAccount) { + return true + } + } + + return false +} + // HandleZoneRemove removes a slave from the zone. func (app *WebApp) HandleZoneRemove(w http.ResponseWriter, r *http.Request) { masterIP := chi.URLParam(r, "id") diff --git a/pkg/service/soundtouchweb/handler_test.go b/pkg/service/soundtouchweb/handler_test.go index 96609a1b..67e5f413 100644 --- a/pkg/service/soundtouchweb/handler_test.go +++ b/pkg/service/soundtouchweb/handler_test.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/httptest" + "reflect" "strings" "testing" "time" @@ -844,6 +845,151 @@ func TestHandleSourceControl_ForwardsAccount(t *testing.T) { } } +func TestHandleZoneAddRejectsSelf(t *testing.T) { + app := NewWebApp() + req := httptest.NewRequest("POST", "/api/control/devices/192.0.2.10/zone/add/192.0.2.10", nil) + req = withChiParams(req, map[string]string{"id": "192.0.2.10", "slaveId": "192.0.2.10"}) + w := httptest.NewRecorder() + + app.HandleZoneAdd(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "cannot be added to its own zone") { + t.Fatalf("unexpected response: %s", w.Body.String()) + } +} + +func TestHandleZoneAddRejectsSameHardwareUnderDifferentKeys(t *testing.T) { + app := NewWebApp() + app.AddDevice("speaker.local", webtypes.NewDeviceConnection( + client.NewClient(&client.Config{Host: "http://speaker.local"}), + &models.DeviceInfo{Name: "Speaker", DeviceID: "SAMEHW01"}, + )) + app.AddDevice("192.0.2.10", webtypes.NewDeviceConnection(nil, + &models.DeviceInfo{Name: "Speaker alias", DeviceID: "SAMEHW01"})) + + req := httptest.NewRequest("POST", "/api/control/devices/speaker.local/zone/add/192.0.2.10", nil) + req = withChiParams(req, map[string]string{"id": "speaker.local", "slaveId": "192.0.2.10"}) + w := httptest.NewRecorder() + app.HandleZoneAdd(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestCurrentSourceAllowsMultiroom(t *testing.T) { + sources := &models.Sources{SourceItem: []models.SourceItem{ + {Source: "SPOTIFY", SourceAccount: "first", MultiroomAllowed: true}, + {Source: "BLUETOOTH", MultiroomAllowed: false}, + }} + + for _, test := range []struct { + name string + nowPlaying *models.NowPlaying + allowed bool + }{ + {name: "matching account", nowPlaying: &models.NowPlaying{Source: "SPOTIFY", SourceAccount: "first"}, allowed: true}, + {name: "different account", nowPlaying: &models.NowPlaying{Source: "SPOTIFY", SourceAccount: "second"}}, + {name: "source disallows multiroom", nowPlaying: &models.NowPlaying{Source: "BLUETOOTH"}}, + {name: "standby", nowPlaying: &models.NowPlaying{Source: "STANDBY"}}, + {name: "missing state"}, + } { + t.Run(test.name, func(t *testing.T) { + if got := currentSourceAllowsMultiroom(test.nowPlaying, sources); got != test.allowed { + t.Fatalf("currentSourceAllowsMultiroom() = %t, want %t", got, test.allowed) + } + }) + } +} + +func TestHandleZoneAddUsesSetZoneWithoutStartingPlayback(t *testing.T) { + var paths []string + var zoneBody string + masterSpeaker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.Method+" "+r.URL.Path) + switch r.URL.Path { + case "/now_playing": + _, _ = w.Write([]byte(`PLAY_STATE`)) + case "/sources": + _, _ = w.Write([]byte(``)) + case "/getZone": + _, _ = w.Write([]byte(``)) + case "/setZone": + body, _ := io.ReadAll(r.Body) + zoneBody = string(body) + w.WriteHeader(http.StatusOK) + default: + http.NotFound(w, r) + } + })) + defer masterSpeaker.Close() + + app := NewWebApp() + master := webtypes.NewDeviceConnection( + client.NewClient(&client.Config{Host: masterSpeaker.URL}), + &models.DeviceInfo{Name: "Master", DeviceID: "MASTERHW01"}, + ) + master.SetStatus(&webtypes.DeviceStatus{IsConnected: true, LastActivity: time.Now()}) + app.AddDevice("192.0.2.10", master) + app.AddDevice("192.0.2.20", webtypes.NewDeviceConnection(nil, + &models.DeviceInfo{Name: "Slave", DeviceID: "SLAVEHW02"})) + + req := httptest.NewRequest("POST", "/api/control/devices/192.0.2.10/zone/add/192.0.2.20", nil) + req = withChiParams(req, map[string]string{"id": "192.0.2.10", "slaveId": "192.0.2.20"}) + w := httptest.NewRecorder() + app.HandleZoneAdd(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + wantPaths := []string{"GET /now_playing", "GET /sources", "GET /getZone", "POST /setZone"} + if !reflect.DeepEqual(paths, wantPaths) { + t.Fatalf("requests = %v, want %v", paths, wantPaths) + } + if !strings.Contains(zoneBody, "SLAVEHW02") { + t.Fatalf("setZone body does not contain slave: %s", zoneBody) + } +} + +func TestHandleZoneAddRejectsStandbyMaster(t *testing.T) { + var paths []string + masterSpeaker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.Method+" "+r.URL.Path) + switch r.URL.Path { + case "/now_playing": + _, _ = w.Write([]byte(``)) + case "/sources": + _, _ = w.Write([]byte(``)) + default: + http.NotFound(w, r) + } + })) + defer masterSpeaker.Close() + + app := NewWebApp() + app.AddDevice("192.0.2.10", webtypes.NewDeviceConnection( + client.NewClient(&client.Config{Host: masterSpeaker.URL}), + &models.DeviceInfo{Name: "Master", DeviceID: "MASTERHW01"}, + )) + app.AddDevice("192.0.2.20", webtypes.NewDeviceConnection(nil, + &models.DeviceInfo{Name: "Slave", DeviceID: "SLAVEHW02"})) + + req := httptest.NewRequest("POST", "/api/control/devices/192.0.2.10/zone/add/192.0.2.20", nil) + req = withChiParams(req, map[string]string{"id": "192.0.2.10", "slaveId": "192.0.2.20"}) + w := httptest.NewRecorder() + app.HandleZoneAdd(w, req) + + if w.Code != http.StatusConflict { + t.Fatalf("expected 409, got %d: %s", w.Code, w.Body.String()) + } + if want := []string{"GET /now_playing", "GET /sources"}; !reflect.DeepEqual(paths, want) { + t.Fatalf("requests = %v, want %v", paths, want) + } +} + // TestHandleZoneRemove_UsesRemoveZoneSlave is the #511 regression: removing one // member from a multi-member zone must target that member via /removeZoneSlave. // The previous implementation rebuilt the zone with /setZone and the remaining diff --git a/pkg/service/soundtouchweb/static/js/components/Zone.js b/pkg/service/soundtouchweb/static/js/components/Zone.js index 4529fb4c..371531d0 100644 --- a/pkg/service/soundtouchweb/static/js/components/Zone.js +++ b/pkg/service/soundtouchweb/static/js/components/Zone.js @@ -5,11 +5,22 @@ import { api } from '../api.js'; const html = htm.bind(h); +function currentSourceAllowsMultiroom(device) { + const nowPlaying = device?.status?.nowPlaying; + const source = nowPlaying?.Source; + if (!source || source === 'STANDBY' || source === 'INVALID_SOURCE') return false; + + return (device?.status?.sources?.SourceItem || []).some(item => + item.Source === source && item.MultiroomAllowed && + (!nowPlaying.SourceAccount || item.SourceAccount === nowPlaying.SourceAccount)); +} + export function Zone({ deviceId, devices }) { const [zone, setZone] = useState(null); const [candidates, setCandidates] = useState({}); const [loading, setLoading] = useState(true); const [showPicker, setShowPicker] = useState(false); + const canGroup = currentSourceAllowsMultiroom(devices?.[deviceId]); function refresh() { Promise.all([api.zone(deviceId), api.zoneCandidates(deviceId)]) @@ -21,8 +32,12 @@ export function Zone({ deviceId, devices }) { } useEffect(() => { refresh(); }, [deviceId]); + useEffect(() => { + if (!canGroup) setShowPicker(false); + }, [canGroup]); async function addDevice(slaveId) { + if (!canGroup) return; setShowPicker(false); await api.zoneAdd(deviceId, slaveId); refresh(); @@ -70,9 +85,13 @@ export function Zone({ deviceId, devices }) {
Standalone ${available.length > 0 && html` - + `}
+ ${available.length > 0 && !canGroup && html` +
Start a multiroom-capable source before grouping speakers.
+ `} `} ${zone.isMaster && html` @@ -91,7 +110,8 @@ export function Zone({ deviceId, devices }) { `)}
${available.length > 0 && html` - + `}
@@ -106,7 +126,7 @@ export function Zone({ deviceId, devices }) { `} - ${showPicker && html` + ${showPicker && canGroup && html`
setShowPicker(false)}>
e.stopPropagation()}>
Add to zone
@@ -126,4 +146,4 @@ export function Zone({ deviceId, devices }) { `}
`; -} \ No newline at end of file +}