fix(marge): reject rename PUT mismatch before persisting

HandleMargeUpdateDevice used to call AddDeviceToAccount (an upsert)
and only check body-vs-URL deviceID after the row was already
written. A speaker sending a malformed PUT with the wrong deviceid
attribute would still leave a spurious record before getting 400.

Now we parse just the deviceid attribute, compare against the URL
segment, and only call into the upsert when they match. The
existing regression test gains two GetDeviceInfo assertions to lock
the no-spurious-row guarantee in.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tobias Gesellchen
2026-05-15 19:04:41 +02:00
co-authored by Claude Opus 4.7
parent 49904635f2
commit 6bc4ee1e73
2 changed files with 28 additions and 5 deletions
+17 -5
View File
@@ -637,20 +637,32 @@ func (s *Server) HandleMargeUpdateDevice(w http.ResponseWriter, r *http.Request)
return
}
bodyDeviceID, data, err := marge.AddDeviceToAccount(s.ds, account, body, r.RemoteAddr)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
// Validate body deviceID against the URL segment *before* the
// upsert in AddDeviceToAccount runs — otherwise a mismatched PUT
// would still persist a row for the body's deviceID before the
// 400 response, leaving spurious state in the datastore.
var probe struct {
DeviceID string `xml:"deviceid,attr"`
}
if xmlErr := xml.Unmarshal(body, &probe); xmlErr != nil {
http.Error(w, xmlErr.Error(), http.StatusBadRequest)
return
}
if bodyDeviceID != device {
if probe.DeviceID != device {
http.Error(w,
fmt.Sprintf("device ID in body (%q) does not match URL (%q)", bodyDeviceID, device),
fmt.Sprintf("device ID in body (%q) does not match URL (%q)", probe.DeviceID, device),
http.StatusBadRequest)
return
}
_, data, err := marge.AddDeviceToAccount(s.ds, account, body, r.RemoteAddr)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "application/vnd.bose.streaming-v1.2+xml")
w.WriteHeader(http.StatusOK)
_, _ = w.Write(data)
@@ -339,4 +339,15 @@ func TestIssue285_RenamePutRejectsMismatchedDeviceID(t *testing.T) {
respBody, _ := io.ReadAll(resp.Body)
t.Fatalf("PUT status = %d, want 400; body:\n%s", resp.StatusCode, respBody)
}
// Mismatched body must be rejected *before* the upsert runs —
// otherwise the datastore ends up with a row keyed on the body's
// deviceID even though we return 400. Verify by reading both keys.
if got, _ := ds.GetDeviceInfo("3981561", "DEADBEEFCAFE"); got != nil {
t.Fatalf("body deviceID DEADBEEFCAFE was persisted despite 400 response: %+v", got)
}
if got, _ := ds.GetDeviceInfo("3981561", urlDeviceID); got != nil {
t.Fatalf("URL deviceID %s was persisted despite 400 response: %+v", urlDeviceID, got)
}
}