diff --git a/pkg/service/handlers/handlers_marge.go b/pkg/service/handlers/handlers_marge.go index e6927a6..927ca58 100644 --- a/pkg/service/handlers/handlers_marge.go +++ b/pkg/service/handlers/handlers_marge.go @@ -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) diff --git a/pkg/service/handlers/issue285_regression_test.go b/pkg/service/handlers/issue285_regression_test.go index 7330a6b..4e63d5f 100644 --- a/pkg/service/handlers/issue285_regression_test.go +++ b/pkg/service/handlers/issue285_regression_test.go @@ -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) + } }