fix(server/m7-382): strip NUL bytes from MBIDs before persist
Follow-up to 947f944. With the dhowden/tag/mbz extractor now correctly
matching ID3v2 TXXX frames, mbid backfill started rejecting every row
with "invalid byte sequence for encoding UTF8: 0x00 (SQLSTATE 22021)".
Cause: dhowden's readTextWithDescrFrame (id3v2frames.go:454) — the path
that decodes TXXX text — does NOT strip the trailing/embedded NUL bytes
that ID3v2.4 uses as frame terminator and multi-value separator. Unlike
readTFrame, which does (line 313). So a TXXX:MusicBrainz Album Id of
"abc-123\x00" comes back to us verbatim, and Postgres refuses to store
NULs in text columns.
cleanMBID splits on NUL and returns the first non-empty segment, which
also handles the multi-value case (collaboration artist IDs are
NUL-separated; Minstrel uses the primary for MBCAA, so first wins).
Tests cover the trailing-NUL, multi-value, and all-NUL paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
package library
|
||||
|
||||
import (
|
||||
"strings"
|
||||
|
||||
"github.com/dhowden/tag"
|
||||
"github.com/dhowden/tag/mbz"
|
||||
)
|
||||
@@ -13,11 +15,26 @@ import (
|
||||
// a *tag.Comm value whose Description holds the Picard tag name.
|
||||
// - MP4 (M4A) freeform iTunes atoms are stored under their bare sub-name.
|
||||
//
|
||||
// We previously hand-rolled this lookup against guessed keys
|
||||
// ("MUSICBRAINZ_ALBUMID", "TXXX:MusicBrainz Album Id", ...), none of which
|
||||
// dhowden actually produces — so every extraction returned ("",""), which
|
||||
// fed an empty MBID into both the scanner and the boot-time backfill.
|
||||
// Then sanitizes: dhowden's readTextWithDescrFrame (used for ID3v2 TXXX)
|
||||
// does NOT strip the trailing/embedded NUL bytes that ID3v2.4 uses as
|
||||
// frame terminator and multi-value separator — so the Text often comes
|
||||
// out as "uuid\x00" or "uuid1\x00uuid2". Postgres rejects NULs in text
|
||||
// columns (SQLSTATE 22021). We split on NUL and take the first non-empty
|
||||
// segment — Minstrel uses the primary release artist for MBCAA lookup,
|
||||
// so first-wins matches the original semantics.
|
||||
func extractMBIDs(m tag.Metadata) (albumMBID, artistMBID string) {
|
||||
info := mbz.Extract(m)
|
||||
return info.Get(mbz.Album), info.Get(mbz.Artist)
|
||||
return cleanMBID(info.Get(mbz.Album)), cleanMBID(info.Get(mbz.Artist))
|
||||
}
|
||||
|
||||
func cleanMBID(s string) string {
|
||||
if s == "" {
|
||||
return ""
|
||||
}
|
||||
for _, part := range strings.Split(s, "\x00") {
|
||||
if t := strings.TrimSpace(part); t != "" {
|
||||
return t
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
@@ -109,3 +109,57 @@ func TestExtractMBIDs_VorbisUppercaseKeysMiss(t *testing.T) {
|
||||
t.Errorf("uppercase Vorbis keys must not match dhowden output; got (%q, %q)", a, ar)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractMBIDs_ID3v2StripsTrailingNUL(t *testing.T) {
|
||||
// dhowden's readTextWithDescrFrame leaves the ID3v2.4 frame-
|
||||
// terminator NUL on TXXX values. Postgres rejects NULs in text
|
||||
// columns (SQLSTATE 22021), so we must strip them.
|
||||
m := stubMeta{
|
||||
format: tag.ID3v2_4,
|
||||
raw: map[string]interface{}{
|
||||
"TXXX": &tag.Comm{
|
||||
Description: "MusicBrainz Album Id",
|
||||
Text: "abc-123\x00",
|
||||
},
|
||||
"TXXX_0": &tag.Comm{
|
||||
Description: "MusicBrainz Artist Id",
|
||||
Text: "def-456\x00",
|
||||
},
|
||||
},
|
||||
}
|
||||
a, ar := extractMBIDs(m)
|
||||
if a != "abc-123" || ar != "def-456" {
|
||||
t.Errorf("got (%q, %q), want trailing NUL stripped", a, ar)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractMBIDs_ID3v2MultiValueTakesFirst(t *testing.T) {
|
||||
// Some Picard configs emit multi-value TXXX:MusicBrainz Artist Id
|
||||
// for collaborations, NUL-separated. Minstrel uses the primary
|
||||
// release artist for MBCAA, so first wins.
|
||||
m := stubMeta{
|
||||
format: tag.ID3v2_4,
|
||||
raw: map[string]interface{}{
|
||||
"TXXX": &tag.Comm{
|
||||
Description: "MusicBrainz Artist Id",
|
||||
Text: "primary-mbid\x00collab-mbid",
|
||||
},
|
||||
},
|
||||
}
|
||||
_, ar := extractMBIDs(m)
|
||||
if ar != "primary-mbid" {
|
||||
t.Errorf("got %q, want %q", ar, "primary-mbid")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCleanMBID_OnlyNULReturnsEmpty(t *testing.T) {
|
||||
// Defensive: a TXXX value that is purely NULs (or empty after
|
||||
// trimming) must round-trip to empty so the caller skips the row
|
||||
// instead of writing a NUL-only string to Postgres.
|
||||
if got := cleanMBID("\x00\x00"); got != "" {
|
||||
t.Errorf("got %q, want empty", got)
|
||||
}
|
||||
if got := cleanMBID(""); got != "" {
|
||||
t.Errorf("got %q, want empty", got)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user