mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-18 08:36:13 +00:00
fix(marge): strict-match preset/recent source by type, refuse cross-type binds
GH-343: a TUNEIN preset surviving a reboot used to come back from /full re-attributed to RADIOPLAYER because mapPresetsToFullResponse's step-1 exact-ID match accepted any source with the matching numeric ID, regardless of what the preset originally claimed for its Source. The speaker trusts /full as ground truth, so the local preset got its source attribute silently rewritten. Tighten step-1: refuse the bind when the preset's claimed Source and the configured source's SourceKeyType disagree (both populated). The existing step-2 type/account fallback then finds the right source, or synthesise/skip handles the no-match case. The refusal is logged so the cross-type collision is visible in service logs. Same fix applied to findMatchingSourceForRecent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
ccdc2bd6a4
commit
5ff72f2af8
@@ -832,16 +832,28 @@ func mapPresetsToFullResponse(presets []models.ServicePreset, sources []models.C
|
||||
}
|
||||
|
||||
var matchedSource *models.ConfiguredSource
|
||||
// 1. Try exact ID match first
|
||||
// 1. Exact ID match — but only if the source's type is
|
||||
// compatible with what the preset claims. GH-343: a numeric
|
||||
// SourceID collision between e.g. a TuneIn preset and a
|
||||
// RADIOPLAYER source in the configured-sources list used to
|
||||
// silently rewrite the preset's source attribute to
|
||||
// RADIOPLAYER on the next /full sync. Strict-match refuses
|
||||
// the bind when the types disagree; the fallback below then
|
||||
// finds the right source (or synthesise/skip kicks in).
|
||||
if p.SourceID != "" {
|
||||
for j := range sources {
|
||||
if sources[j].ID == p.SourceID {
|
||||
if sources[j].ID == p.SourceID && sourceTypeCompatible(p.Source, sources[j].SourceKeyType) {
|
||||
copySource := sources[j]
|
||||
PrepareConfiguredSource(©Source)
|
||||
matchedSource = ©Source
|
||||
|
||||
break
|
||||
}
|
||||
|
||||
if sources[j].ID == p.SourceID && !sourceTypeCompatible(p.Source, sources[j].SourceKeyType) {
|
||||
log.Printf("[Marge] /full: refusing cross-type bind for preset %s — preset.Source=%q but configured source id=%s has type %q; falling through to type-based match",
|
||||
p.ButtonNumber, p.Source, sources[j].ID, sources[j].SourceKeyType)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -953,17 +965,24 @@ func synthesiseDefaultSourceByType(sourceKeyType string) (models.FullResponseSou
|
||||
}
|
||||
|
||||
// findMatchingSourceForRecent resolves the configured source a recent
|
||||
// references. Tries exact SourceID match first, then SourceKeyType+account.
|
||||
// Returns nil when nothing matches — caller falls back to synthesise/skip.
|
||||
// references. Tries exact SourceID match (but only if the matched source's
|
||||
// type is compatible with the recent's claimed Source — see GH-343), then
|
||||
// SourceKeyType+account. Returns nil when nothing matches; caller falls
|
||||
// back to synthesise/skip.
|
||||
func findMatchingSourceForRecent(r *models.ServiceRecent, sources []models.ConfiguredSource) *models.ConfiguredSource {
|
||||
if r.SourceID != "" {
|
||||
for j := range sources {
|
||||
if sources[j].ID == r.SourceID {
|
||||
if sources[j].ID == r.SourceID && sourceTypeCompatible(r.Source, sources[j].SourceKeyType) {
|
||||
copySource := sources[j]
|
||||
PrepareConfiguredSource(©Source)
|
||||
|
||||
return ©Source
|
||||
}
|
||||
|
||||
if sources[j].ID == r.SourceID && !sourceTypeCompatible(r.Source, sources[j].SourceKeyType) {
|
||||
log.Printf("[Marge] /full: refusing cross-type bind for recent id=%s — recent.Source=%q but configured source id=%s has type %q; falling through to type-based match",
|
||||
r.ID, r.Source, sources[j].ID, sources[j].SourceKeyType)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -980,6 +999,20 @@ func findMatchingSourceForRecent(r *models.ServiceRecent, sources []models.Confi
|
||||
return nil
|
||||
}
|
||||
|
||||
// sourceTypeCompatible decides whether a preset/recent whose original
|
||||
// source is `claimed` can be bound to a configured source of type
|
||||
// `configured`. Strict-match policy: identical types compatible; either
|
||||
// side empty is treated as "no info, allow" (matches pre-fix behaviour for
|
||||
// records that lost their Source attribute). Refuse when both are set and
|
||||
// disagree — that's the GH-343 cross-type rewrite the speaker reflects.
|
||||
func sourceTypeCompatible(claimed, configured string) bool {
|
||||
if claimed == "" || configured == "" {
|
||||
return true
|
||||
}
|
||||
|
||||
return claimed == configured
|
||||
}
|
||||
|
||||
func mapRecentsToFullResponse(recents []models.ServiceRecent, sources []models.ConfiguredSource) []models.FullResponseRecent {
|
||||
var fullRecents []models.FullResponseRecent
|
||||
|
||||
|
||||
@@ -258,6 +258,85 @@ func TestMapRecentsToFullResponse_UnresolvedSource(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestMapPresetsToFullResponse_StrictTypeMatch_GH343 is the regression for
|
||||
// GH-343: a preset stored with Source=TUNEIN used to come back from /full
|
||||
// rebound to RADIOPLAYER because the numeric SourceID collided with a
|
||||
// RADIOPLAYER source in the configured-sources list. The speaker, trusting
|
||||
// /full as ground truth, then locally re-attributed the preset's source.
|
||||
//
|
||||
// Strict-match refuses to bind a TUNEIN preset to a RADIOPLAYER source,
|
||||
// even when the IDs match. The fallback type-search then finds the right
|
||||
// TUNEIN source.
|
||||
func TestMapPresetsToFullResponse_StrictTypeMatch_GH343(t *testing.T) {
|
||||
// Two configured sources with the same numeric ID. In real life they'd
|
||||
// be from different devices/accounts; the bug surfaced when a stale
|
||||
// migration left a RADIOPLAYER source with the same ID a TUNEIN preset
|
||||
// referenced. Order matters here — RADIOPLAYER comes first so the
|
||||
// strict-match has to actively skip it to reach the TUNEIN entry.
|
||||
configured := []models.ConfiguredSource{
|
||||
{
|
||||
ID: "10004",
|
||||
Type: "Audio",
|
||||
SourceKeyType: "RADIOPLAYER",
|
||||
SourceProviderID: "35",
|
||||
},
|
||||
{
|
||||
ID: "10004",
|
||||
Type: "Audio",
|
||||
SourceKeyType: constants.ProviderTunein,
|
||||
SourceProviderID: "25",
|
||||
},
|
||||
}
|
||||
|
||||
presets := []models.ServicePreset{
|
||||
{
|
||||
ServiceContentItem: models.ServiceContentItem{
|
||||
Name: "MDR JUMP",
|
||||
Source: constants.ProviderTunein,
|
||||
SourceID: "10004",
|
||||
Location: "/v1/playback/station/s6634",
|
||||
ContentItemType: "stationurl",
|
||||
},
|
||||
ButtonNumber: "1",
|
||||
},
|
||||
}
|
||||
|
||||
got := mapPresetsToFullResponse(presets, configured)
|
||||
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("expected 1 emitted preset, got %d", len(got))
|
||||
}
|
||||
|
||||
if got[0].Source.SourceProviderID != "25" {
|
||||
t.Errorf("strict-match should have bound to TuneIn (sourceproviderid=25), got %q — cross-type bind not refused?", got[0].Source.SourceProviderID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSourceTypeCompatible pins the strict-match policy:
|
||||
// - identical types compatible
|
||||
// - either side empty treated as "no info, allow"
|
||||
// - both set and different — refused
|
||||
func TestSourceTypeCompatible(t *testing.T) {
|
||||
cases := []struct {
|
||||
claimed, configured string
|
||||
want bool
|
||||
}{
|
||||
{"TUNEIN", "TUNEIN", true},
|
||||
{"TUNEIN", "RADIOPLAYER", false},
|
||||
{"", "TUNEIN", true},
|
||||
{"TUNEIN", "", true},
|
||||
{"", "", true},
|
||||
{"SPOTIFY", "SPOTIFY", true},
|
||||
{"SPOTIFY", "AMAZON", false},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
if got := sourceTypeCompatible(tc.claimed, tc.configured); got != tc.want {
|
||||
t.Errorf("sourceTypeCompatible(%q, %q) = %v, want %v", tc.claimed, tc.configured, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestCanonicalDefaultsByType pins the type → (id, providerid) mapping
|
||||
// against the canonical IDs the speaker firmware ships with.
|
||||
// canonicalProviderIDByID (the inverse) is already tested implicitly via
|
||||
|
||||
Reference in New Issue
Block a user