From 208fc2c4d6e90dfdb221a96c24dc285a926d1f60 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Fri, 9 Oct 2026 14:53:48 -0400 Subject: [PATCH] =?UTF-8?q?fix(server):=20kobo=20markup=20panic=20?= =?UTF-8?q?=E2=80=94=20wire=20LibraryService=20into=20the=20test=20harness?= =?UTF-8?q?,=20guard=20typed-nil=20resolver?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestProgressWeb_KoboIntegration 500s'd on every markup with a last-read-place bookmark: the panic-recovered stack was a nil pointer dereference in LibraryService.ResolveMediaPath (s.db deref), reached from KoboHandler.convertKoboCFIToStandard during kepub CFI conversion. Two-layer bug: 1. Test harness (root cause): the router.Config built in test_helpers_test.go omitted LibraryService, so router/sync.go's SetLibraryService(cfg.LibraryService) stored a nil *services.LibraryService in the interface field. Production wiring (main.go:246) was always correct. 2. Latent trap (hardening): `h.libraryService != nil` at the convertKoboCFIToStandard call site can never catch a typed nil — an interface holding a nil *LibraryService is non-nil. ResolveMediaPath now degrades with a "library service not configured" error instead of panicking on a nil receiver; callers already handle the error by skipping CFI conversion. Harness-only fix, zero production behavior change: the endpoint now returns a clean 200 with the position saved wherever kepub conversion is unavailable instead of a panic-recovered 500. Verified: TestProgressWeb_KoboIntegration passes all three subtests (401 unauthenticated, ReadingSync→last-read-place percentage preservation, last-read-place without prior ReadingSync) and on immediate rerun — previously it failed every run. --- cmd/server/tests/test_helpers_test.go | 4 ++++ internal/services/library_service.go | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/cmd/server/tests/test_helpers_test.go b/cmd/server/tests/test_helpers_test.go index 48eb8adf..656f0b7f 100644 --- a/cmd/server/tests/test_helpers_test.go +++ b/cmd/server/tests/test_helpers_test.go @@ -572,6 +572,10 @@ func setupTestServer(t *testing.T) *TestServerSetup { // the main binary wires database.NewSettingsRegistry(queries) — // the test harness must too or route registration nil-panics. Settings: database.NewSettingsRegistry(queries), + // main.go:246 passes the shared LibraryService here; omitting it + // hands the kobo/kepub CFI converter a typed-nil (kobo markup + // panic-recovered as 500s in TestProgressWeb_KoboIntegration). + LibraryService: libraryService, } router.RegisterRoutes(routerConfig) diff --git a/internal/services/library_service.go b/internal/services/library_service.go index 9746da6f..9bd4cc65 100644 --- a/internal/services/library_service.go +++ b/internal/services/library_service.go @@ -299,6 +299,13 @@ func (s *LibraryService) SyncAllowedExtensions(ctx context.Context) { } func (s *LibraryService) ResolveMediaPath(ctx context.Context, libraryID pgtype.UUID, relativePath string) (string, error) { + // Typed-nil interface guard: a nil *LibraryService held in an + // interface passes `!= nil` checks at call sites; degrade instead + // of panicking (kobo markup 500s were exactly this path). + if s == nil || s.db == nil { + return "", fmt.Errorf("library service not configured") + } + // Get library folders for this library folders, err := s.db.GetLibraryFolders(ctx, libraryID) if err != nil || len(folders) == 0 {