From d7c3976684bffde91e8432d122e27c124f67919c Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Thu, 4 Jun 2026 19:15:20 +0200 Subject: [PATCH] =?UTF-8?q?fix(datastore):=20treat=20empty/0-byte/unparsea?= =?UTF-8?q?ble=20XML=20as=20missing=20=E2=86=92=20serve=20defaults=20(#458?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A power-cut on the speaker's NAND can leave a datastore file present but 0-byte (a not-yet-flushed atomicWriteFile write). The read paths now treat empty/0-byte/ unparseable Presets/Recents/Sources the same as missing: GetConfiguredSources serves the managed defaults (so /full self-heals instead of wiping the speaker), GetPresets/GetRecents return an empty list (no more HTTP 500 on the device-level endpoints), and HasConfiguredSources reports a 0-byte file as absent (so the create_default_sources health quick fix is offered again). Read-side resilience only; the write-side durability fix (fsync in atomicWriteFile) follows in a separate PR. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/service/datastore/datastore.go | 72 ++++++++-- pkg/service/datastore/empty_datastore_test.go | 135 ++++++++++++++++++ 2 files changed, 197 insertions(+), 10 deletions(-) create mode 100644 pkg/service/datastore/empty_datastore_test.go diff --git a/pkg/service/datastore/datastore.go b/pkg/service/datastore/datastore.go index 38b06fd..d2cbe8e 100644 --- a/pkg/service/datastore/datastore.go +++ b/pkg/service/datastore/datastore.go @@ -962,6 +962,16 @@ func (ds *DataStore) readPresetsLocked(account, device string) ([]models.Service return nil, false, err } + // An empty / 0-byte Presets.xml (e.g. truncated by an unclean power-cut on + // the speaker's NAND) is treated as "no presets" rather than a hard parse + // error, so the device-level /presets endpoint returns an empty list + // instead of HTTP 500. See #458. + if len(bytes.TrimSpace(data)) == 0 { + log.Printf("[Datastore] readPresetsLocked: empty/0-byte Presets.xml at %s — treating as no presets (#458)", sanitizeLog(path)) + + return []models.ServicePreset{}, false, nil + } + var presetsWrap struct { Presets []struct { ID string `xml:"id,attr"` @@ -989,7 +999,9 @@ func (ds *DataStore) readPresetsLocked(account, device string) ([]models.Service needsRewrite := !bytes.Equal(normalized, data) if err := xml.Unmarshal(normalized, &presetsWrap); err != nil { - return nil, false, fmt.Errorf("malformed presets XML at %s: %w", path, err) + log.Printf("[Datastore] readPresetsLocked: malformed Presets.xml at %s (%v) — treating as no presets (#458)", sanitizeLog(path), err) + + return []models.ServicePreset{}, false, nil } presets := []models.ServicePreset{} @@ -1225,6 +1237,16 @@ func (ds *DataStore) GetRecents(account, device string) ([]models.ServiceRecent, return nil, err } + // An empty / 0-byte Recents.xml (e.g. truncated by an unclean power-cut) is + // treated as "no recents" rather than a hard parse error, so the + // device-level /recents endpoint returns an empty list instead of HTTP 500. + // See #458. + if len(bytes.TrimSpace(data)) == 0 { + log.Printf("[Datastore] GetRecents: empty/0-byte Recents.xml at %s — treating as no recents (#458)", sanitizeLog(path)) + + return []models.ServiceRecent{}, nil + } + type RecentXML struct { DeviceID string `xml:"deviceID,attr,omitempty"` UtcTime string `xml:"utcTime,attr,omitempty"` @@ -1252,7 +1274,9 @@ func (ds *DataStore) GetRecents(account, device string) ([]models.ServiceRecent, var wrap RecentsXML if err := xml.Unmarshal(data, &wrap); err != nil { - return nil, fmt.Errorf("malformed recents XML at %s: %w", path, err) + log.Printf("[Datastore] GetRecents: malformed Recents.xml at %s (%v) — treating as no recents (#458)", sanitizeLog(path), err) + + return []models.ServiceRecent{}, nil } recents := make([]models.ServiceRecent, 0, len(wrap.Recents)) @@ -1794,18 +1818,34 @@ func (ds *DataStore) GetConfiguredSources(account, device string) ([]models.Conf path := filepath.Join(ds.AccountDeviceDir(account, device), constants.SourcesFile) + // defaultSources is the fallback used whenever there is no usable + // Sources.xml: file missing (normal for a fresh device) or present but + // empty / 0-byte / unparseable. The latter happens when an unclean + // power-cut truncates a not-yet-flushed datastore write on the speaker's + // NAND; treating it like "missing" lets /full re-serve the managed defaults + // so the speaker self-heals instead of dropping all its sources. See #458. + defaultSources := func() []models.ConfiguredSource { + sources := ds.getInitialSources() + ds.DeduceSourceIDs(account, device, sources) + + return sources + } + data, err := ds.rootReadFile(path) if err != nil { if os.IsNotExist(err) { - sources := ds.getInitialSources() - ds.DeduceSourceIDs(account, device, sources) - - return sources, nil + return defaultSources(), nil } return nil, err } + if len(bytes.TrimSpace(data)) == 0 { + log.Printf("[Datastore] GetConfiguredSources: empty/0-byte Sources.xml at %s — treating as missing, serving defaults (#458)", sanitizeLog(path)) + + return defaultSources(), nil + } + type persistentSource struct { DisplayName string `xml:"displayName,attr,omitempty"` ID string `xml:"id,attr,omitempty"` @@ -1830,7 +1870,9 @@ func (ds *DataStore) GetConfiguredSources(account, device string) ([]models.Conf } if err := xml.Unmarshal(data, &sourcesWrap); err != nil { - return nil, fmt.Errorf("malformed sources XML at %s: %w", path, err) + log.Printf("[Datastore] GetConfiguredSources: malformed Sources.xml at %s (%v) — treating as missing, serving defaults (#458)", sanitizeLog(path), err) + + return defaultSources(), nil } sources := make([]models.ConfiguredSource, len(sourcesWrap.Sources)) @@ -2359,12 +2401,22 @@ func (ds *DataStore) GetETagForPresets(account, device string) int64 { return info.ModTime().UnixNano() / int64(time.Millisecond) } -// HasConfiguredSources reports whether a Sources.xml file exists for the given account and device. +// HasConfiguredSources reports whether a non-empty Sources.xml file exists for +// the given account and device. A present-but-0-byte file (truncated by an +// unclean power-cut) counts as absent. See #458. func (ds *DataStore) HasConfiguredSources(account, device string) bool { path := filepath.Join(ds.AccountDeviceDir(account, device), constants.SourcesFile) - _, err := ds.rootStat(path) - return err == nil + data, err := ds.rootReadFile(path) + if err != nil { + return false + } + + // An existing but empty / 0-byte Sources.xml (e.g. truncated by an unclean + // power-cut on the speaker's NAND) must not count as "present": otherwise it + // hides the sources_xml_present health check and its create_default_sources + // quick fix, leaving the device with no managed sources. See #458. + return len(bytes.TrimSpace(data)) > 0 } // GetETagForSources returns the ETag (modification time) for the sources file for a specific device. diff --git a/pkg/service/datastore/empty_datastore_test.go b/pkg/service/datastore/empty_datastore_test.go new file mode 100644 index 0000000..7a51b81 --- /dev/null +++ b/pkg/service/datastore/empty_datastore_test.go @@ -0,0 +1,135 @@ +package datastore + +import ( + "os" + "path/filepath" + "testing" + + "github.com/gesellix/bose-soundtouch/pkg/service/constants" +) + +// These tests cover #458: an unclean power-cut on the speaker's NAND can leave +// a datastore file present but 0-byte (truncated, not-yet-flushed write). The +// read paths must treat empty / 0-byte / unparseable files the same as +// "missing" — serve defaults for sources, return empty lists for presets/recents +// — instead of advertising nothing on /full (which wipes the speaker) or +// returning HTTP 500 on the device-level endpoints. + +func newTestStore(t *testing.T) (*DataStore, string, string) { + t.Helper() + + tempDir, err := os.MkdirTemp("", "st-empty-test-*") + if err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { _ = os.RemoveAll(tempDir) }) + + ds := NewDataStore(tempDir) + account := "1234567" + device := "001122334455" + + dir := ds.AccountDeviceDir(account, device) + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + + return ds, account, device +} + +func writeDeviceFile(t *testing.T, ds *DataStore, account, device, name string, content []byte) { + t.Helper() + + path := filepath.Join(ds.AccountDeviceDir(account, device), name) + if err := os.WriteFile(path, content, 0644); err != nil { + t.Fatal(err) + } +} + +func TestGetConfiguredSources_EmptyFile_ServesDefaults(t *testing.T) { + ds, account, device := newTestStore(t) + writeDeviceFile(t, ds, account, device, constants.SourcesFile, []byte{}) + + sources, err := ds.GetConfiguredSources(account, device) + if err != nil { + t.Fatalf("GetConfiguredSources returned error for 0-byte file: %v", err) + } + + if len(sources) == 0 { + t.Fatal("expected default sources for a 0-byte Sources.xml, got none") + } + + types := map[string]bool{} + for i := range sources { + types[sources[i].SourceKeyType] = true + } + + for _, want := range []string{constants.ProviderTunein, constants.ProviderLocalInternetRadio} { + if !types[want] { + t.Errorf("expected default sources to include %q (ding/radio need it); got %v", want, types) + } + } +} + +func TestGetConfiguredSources_MalformedFile_ServesDefaults(t *testing.T) { + ds, account, device := newTestStore(t) + writeDeviceFile(t, ds, account, device, constants.SourcesFile, []byte("`)) + + if !ds.HasConfiguredSources(account, device) { + t.Error("HasConfiguredSources returned false for a populated Sources.xml; want true") + } +}