From 8b37db2621822e9edffaa76c0c09bd37ca958bed Mon Sep 17 00:00:00 2001 From: Justin Terry Date: Sat, 13 Jun 2026 20:26:46 -0700 Subject: [PATCH] fix(n64): don't 422 on Controller Pak images with blank note slots extractLogicalEntries + countN64ControllerPakEntries iterated every pakfs ReadDirRoot entry and called fsys.Open on its name. Real-world mempaks (e.g. MiSTer-written .cpk) expose blank/uninitialised note slots with empty names, so fsys.Open("") returned "open : invalid argument" and the whole upload was rejected with HTTP 422 ("open controller pak entry \"\""). On a MiSTer SS1 this silently blocked ALL N64 save-sync (uploaded=0, errors=76/cycle). Skip empty-named slots in both functions (and make a single unreadable note non-fatal in extract) so valid Controller Paks sync and blank ones cleanly validate as "no save entries" instead of crashing. Count and extract now agree. Regression test uses a real blank mempak fixture that reproduced the 422. Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/cmd/server/n64_controller_pak.go | 27 ++++++++++-- .../n64_controller_pak_emptyslot_test.go | 41 ++++++++++++++++++ .../n64_controller_pak_empty_slot.cpk | Bin 0 -> 32768 bytes 3 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 backend/cmd/server/n64_controller_pak_emptyslot_test.go create mode 100644 backend/cmd/server/testdata/n64_controller_pak_empty_slot.cpk diff --git a/backend/cmd/server/n64_controller_pak.go b/backend/cmd/server/n64_controller_pak.go index fcc5848..5f28e86 100644 --- a/backend/cmd/server/n64_controller_pak.go +++ b/backend/cmd/server/n64_controller_pak.go @@ -389,10 +389,18 @@ func (s *n64ControllerPakStore) extractLogicalEntries(syncLineKey string, payloa } root := fsys.ReadDirRoot() out := make([]n64ControllerPakExtractedEntry, 0, len(root)) - for idx, dirEntry := range root { + for _, dirEntry := range root { + // Skip blank/empty note slots: some Controller Pak images expose free or + // uninitialised slots whose name is empty, which pakfs.Open rejects with + // "open : invalid argument". Ignoring them (rather than erroring) stops valid + // .cpk uploads from being rejected with HTTP 422 ("open controller pak entry"). + if strings.TrimSpace(dirEntry.Name()) == "" { + continue + } opened, err := fsys.Open(dirEntry.Name()) if err != nil { - return nil, fmt.Errorf("open controller pak entry %q: %w", dirEntry.Name(), err) + // A single unreadable note must not fail the whole Controller Pak upload. + continue } file, ok := opened.(*pakfs.File) if !ok { @@ -418,7 +426,7 @@ func (s *n64ControllerPakStore) extractLogicalEntries(syncLineKey string, payloa GameCode: strings.ToUpper(strings.TrimSpace(gameCode)), PublisherCode: strings.ToUpper(strings.TrimSpace(publisherCode)), NoteName: noteName, - EntryIndex: idx + 1, + EntryIndex: len(out) + 1, PageCount: int((file.Size() + 255) / 256), BlockUsage: int((file.Size() + 255) / 256), StructureValid: true, @@ -448,7 +456,18 @@ func countN64ControllerPakEntries(payload []byte) (int, error) { if err != nil { return 0, fmt.Errorf("parse controller pak filesystem: %w", err) } - return len(fsys.ReadDirRoot()), nil + root := fsys.ReadDirRoot() + count := 0 + for _, dirEntry := range root { + // Count only real, named notes — ignore blank/empty slots that + // extractLogicalEntries also skips, so validation and extraction agree + // (a pak with only empty slots counts as 0 → "no save entries", not a + // later HTTP 422 during extraction). + if strings.TrimSpace(dirEntry.Name()) != "" { + count++ + } + } + return count, nil } func (logical n64ControllerPakLogicalSave) latestRevision() (n64ControllerPakLogicalRevision, bool) { diff --git a/backend/cmd/server/n64_controller_pak_emptyslot_test.go b/backend/cmd/server/n64_controller_pak_emptyslot_test.go new file mode 100644 index 0000000..defbc69 --- /dev/null +++ b/backend/cmd/server/n64_controller_pak_emptyslot_test.go @@ -0,0 +1,41 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// Regression for the N64 Controller Pak HTTP 422 upload bug: real-world mempaks +// (e.g. a MiSTer-written .cpk) can contain a blank/empty-named note slot that +// pakfs surfaces in ReadDirRoot but cannot Open, which previously made both +// countN64ControllerPakEntries and extractLogicalEntries fail with +// `open controller pak entry "": open : invalid argument` — rejecting the whole +// (valid) save with HTTP 422. Fixture is a real Controller Pak exhibiting this. +func TestN64ControllerPakEmptySlotDoesNotError(t *testing.T) { + buf, err := os.ReadFile(filepath.Join("testdata", "n64_controller_pak_empty_slot.cpk")) + if err != nil { + t.Fatalf("read fixture: %v", err) + } + + count, err := countN64ControllerPakEntries(buf) + if err != nil { + t.Fatalf("countN64ControllerPakEntries errored on real mempak with empty slot: %v", err) + } + + store, err := newN64ControllerPakStore(t.TempDir()) + if err != nil { + t.Fatalf("newN64ControllerPakStore: %v", err) + } + entries, err := store.extractLogicalEntries("synctest", buf) + if err != nil { + if strings.Contains(err.Error(), "open controller pak entry") { + t.Fatalf("regression: extract still fails on empty slot: %v", err) + } + t.Fatalf("extractLogicalEntries errored: %v", err) + } + if len(entries) != count { + t.Fatalf("extract/count disagree: count=%d extracted=%d", count, len(entries)) + } +} diff --git a/backend/cmd/server/testdata/n64_controller_pak_empty_slot.cpk b/backend/cmd/server/testdata/n64_controller_pak_empty_slot.cpk new file mode 100644 index 0000000000000000000000000000000000000000..aae3b60965ae6404b6cf02d8f31d800b392681cc GIT binary patch literal 32768 zcmeIus||oK6ac_Bh-wyKAQW@pSc6G$?7(7xAcQ7ODv-Tb^}b0jgi55lpN^sH*EySv zF-+S&X0=jM+pqOYH@~&N`^$T*3oX9J|9=Vs1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+ z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF P5FkK+009C72s{E8#l@Yi literal 0 HcmV?d00001