fix(player): guard multiroom zone creation

This commit is contained in:
Lukáš Lipinský
2026-09-05 11:38:49 +02:00
committed by Tobias Gesellchen
parent aa16d1040b
commit ca2e2f9257
3 changed files with 216 additions and 4 deletions
+46
View File
@@ -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")
+146
View File
@@ -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(`<nowPlaying deviceID="MASTERHW01" source="LOCAL_INTERNET_RADIO"><playStatus>PLAY_STATE</playStatus></nowPlaying>`))
case "/sources":
_, _ = w.Write([]byte(`<sources deviceID="MASTERHW01"><sourceItem source="LOCAL_INTERNET_RADIO" status="READY" isLocal="false" multiroomallowed="true" /></sources>`))
case "/getZone":
_, _ = w.Write([]byte(`<zone master="MASTERHW01"/>`))
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(`<nowPlaying deviceID="MASTERHW01" source="STANDBY"/>`))
case "/sources":
_, _ = w.Write([]byte(`<sources deviceID="MASTERHW01"><sourceItem source="LOCAL_INTERNET_RADIO" status="READY" multiroomallowed="true" /></sources>`))
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
@@ -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 }) {
<div class="zone-row">
<span class="zone-status-label">Standalone</span>
${available.length > 0 && html`
<button class="btn-secondary zone-btn" onClick=${() => setShowPicker(true)}>+ Group with…</button>
<button class="btn-secondary zone-btn" onClick=${() => setShowPicker(true)}
disabled=${!canGroup}>+ Group with…</button>
`}
</div>
${available.length > 0 && !canGroup && html`
<div class="zone-status-label">Start a multiroom-capable source before grouping speakers.</div>
`}
`}
${zone.isMaster && html`
@@ -91,7 +110,8 @@ export function Zone({ deviceId, devices }) {
`)}
<div class="zone-actions">
${available.length > 0 && html`
<button class="btn-secondary zone-btn" onClick=${() => setShowPicker(true)}>+ Add speaker</button>
<button class="btn-secondary zone-btn" onClick=${() => setShowPicker(true)}
disabled=${!canGroup}>+ Add speaker</button>
`}
<button class="btn-secondary zone-btn" onClick=${dissolve}>Dissolve zone</button>
</div>
@@ -106,7 +126,7 @@ export function Zone({ deviceId, devices }) {
</div>
`}
${showPicker && html`
${showPicker && canGroup && html`
<div class="overlay" onClick=${() => setShowPicker(false)}>
<div class="device-picker" onClick=${e => e.stopPropagation()}>
<div class="picker-title">Add to zone</div>
@@ -126,4 +146,4 @@ export function Zone({ deviceId, devices }) {
`}
</div>
`;
}
}