Repository navigation
fix: various export bug fixes - #49
Merged
Merged
Conversation
Found running the v0.5.0 release checklist: exporting a library containing a track whose album tag carries an embedded NUL (a mangled apostrophe) failed with mkdir EINVAL. SanitizeFilename only replaced the FAT-illegal printable characters; control characters sailed through, and a NUL in a path is an immediate EINVAL on Linux before FAT even gets a say. Control characters (0x00-0x1f, 0x7f) are now dropped - they were never meant to be visible - FAT-illegal punctuation still becomes _, trailing dots/spaces are trimmed (FAT rejects those names), and a name that sanitizes away entirely becomes _ so the directory is always creatable. TestSanitizeFilenameControlChars pins it, including mkdir-ability of every sanitized result.
Second export failure found during the v0.5.0 checklist on the same stick: 'file name too long'. The 126-char total-path budget shrinks the filename and album but never the artist component, so a single oversized artist name overflows NAME_MAX on its own. This stick's pdb decodes a corrupt artist tag into a 650-byte string (many names concatenated by a string-heap over-read in the reader — tracked separately; it also mis-displays the artist in the library), which made mkdir fail with ENAMETOOLONG. Cap artist and album to 64 bytes each via a new truncateComponent: rune-boundary byte cap, no '.ext' preservation (TruncateFilename would treat a dot in an artist name as an extension and mangle it), trailing space/dot re-trimmed, never empty. 64 stays well under the 126 total-path budget and the filesystem limit, so a malformed tag can no longer produce an un-mkdir-able path regardless of the decode bug. Verified: the stick that failed now lays out cleanly. TestTruncateComponent pins the cap; TestSanitizeFilename unchanged.
Two export-surface fixes found during the v0.5.0 checklist, both in the export handler's source resolution: USB playlist export: the toolbar EXPORT button was hidden on a served rekordbox USB's playlists (the read-only rule hid ALL toolbar actions), and the export source resolver only understood PlaylistStore IDs — a namespaced USB-playlist ID 404'd. A shared exportPlaylistSource now resolves both store and USB-tree playlists (export only reads tracks, so re-exporting a stick's playlist is allowed even though mutation isn't), the preview and export handlers both use it, and the web UI shows EXPORT on read-only playlists again. Collection track count: exporting source=all reported "wrote 0 tracks" because the handler left opts.Tracks nil (export.Run defaulted the collection internally) while the response reported len(opts.Tracks). The export was correct; only the count was wrong. The 'all' case now resolves the collection into opts.Tracks too. TestExportPlaylistSourceUSB pins USB-playlist resolution; verified a 113-track collection export reports 113.
vynulldev
marked this pull request as ready for review
October 3, 2026 21:26
5 tasks done
vynulldev
added a commit
that referenced
this pull request
Oct 3, 2026
) Serving a rekordbox USB mis-displayed some artists as hundreds of bytes of concatenated names (the root cause the #49 export-sanitize fixes masked). The reader assumed a fixed name offset per row, but the DeviceSQL format stores the offset IN the row (u8 near / u16 far) and the string there. Short ASCII names sit at 10 so the hardcode worked for most; a UTF-16 name sits further along, so the hardcode hit padding, read lk=0x00, and the fallback returned the whole string heap. Read row[9]/row[10:12] for artists and row[21]/row[22:24] for albums; widen the id reads to u32. Safe for working libraries (where the hardcode was right the offset read-back matches it) — validated across five real pdbs with zero artist regressions, over-reads fixed. TestParseArtistRowReadsNameOffset pins it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Four export-path bugs, all found running the v0.5.0 export checklist against a real rekordbox stick.
Control characters in tags (ceaecf3): export failed with
mkdir .../Don t Forget My Love: invalid argument. The album tag carries an embedded NUL where the apostrophe should be (mangled encoding); a NUL in a path is EINVAL on Linux. SanitizeFilename only replaced FAT-illegal printable chars — control characters (0x00-0x1f, 0x7f) are now dropped, trailing dots/spaces trimmed (FAT rejects those), and a fully-stripped name becomes_.Oversized path components (872940e): a second failure,
file name too long. The 126-char total-path budget shrank filename and album but never the artist, so one oversized artist overflowed NAME_MAX alone. Artist and album are now capped to 64 bytes each viatruncateComponent(rune-boundary, no false ".ext" preservation, trailing dot/space re-trimmed). The oversized artist traces to a pdb string-heap over-read (below); the cap makes any malformed tag mkdir-able regardless.USB playlist export (161d848): the toolbar EXPORT button was hidden on a served USB's playlists (the read-only rule hid all toolbar actions), and the export source resolver only understood PlaylistStore IDs — a namespaced USB-playlist ID 404'd. A shared
exportPlaylistSourceresolves both; export only reads tracks, so re-exporting a stick's playlist is allowed even though mutation isn't. EXPORT shows on read-only playlists again.Correct collection count (161d848): exporting the whole collection reported "wrote 0 tracks" — the handler left opts.Tracks nil (export.Run defaulted it internally) while the response reported len(opts.Tracks). The export was correct; only the count was wrong. Now reports the real count (verified: 113-track collection → 113).
Known follow-up, NOT fixed here
The oversized artist comes from a real pdb reader bug:
readDeviceSQLStringover-reads the string heap on this stick's UTF-16 artist rows (lands 2 bytes early, hits lk=0x00, the fallback returns the rest of the heap), decoding one artist as 650 bytes of concatenated names. This also mis-displays the artist in the library and deck browse, not just export. The export fixes make the bad name a valid directory; the decode bug needs separate RE of the artist-row variant and is tracked as a follow-up.Hardware testing
Checklist
go build ./...,go vet ./..., andgo test ./...passgofmt -l .is cleanGPL-3.0-or-later)