From aec226af1ad186e58aaff9883fc62591d31cb853 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sat, 26 Sep 2026 14:43:34 -0400 Subject: [PATCH] =?UTF-8?q?feat(sync):=20server-side=20position=20authorit?= =?UTF-8?q?y=20=E2=80=94=20verify/heal=20progress=20anchors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Progress submissions now carry (percentage, context_text, epubcfi) and the server becomes the position authority: - VerifyProgressAnchor resolves the submitted standard CFI against the book's own XHTML, extracts the text at the anchor, and cross-checks it with the submitted context_text. A mismatch heals the anchor by text search (percentage disambiguates repeats) instead of storing a bad position. - The anchor's block element is derived as a cssSelector plus a block- relative character offset, and served on progress GET alongside the anchor document's href — readium-native handles that let clients re-open a book without parsing CFIs themselves. - context_text-only submissions (no CFI — the dumb-client tier) are anchored structurally from the context text. Motivation: cross-client progress sync (web foliate CFIs, KOReader CRE xpointers, readium-native apps) previously trusted each client's own locator math; the app's EPUB restore drifted ±pages because readium's paginator does not lay out far-from-viewport columns and the foliate- ported CFI walk ran against readium's mutated WebView DOM. Server-side verification heals both classes at ingest. --- cmd/server/main.go | 3 + internal/handlers/media.go | 31 +++ internal/sync/cfi_verify.go | 315 ++++++++++++++++++++++++ internal/sync/cfi_verify_manual_test.go | 22 ++ internal/sync/cfi_verify_test.go | 158 ++++++++++++ internal/sync/progress.go | 43 ++++ 6 files changed, 572 insertions(+) create mode 100644 internal/sync/cfi_verify.go create mode 100644 internal/sync/cfi_verify_manual_test.go create mode 100644 internal/sync/cfi_verify_test.go diff --git a/cmd/server/main.go b/cmd/server/main.go index 7e52504..d957a09 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -121,6 +121,9 @@ func main() { // Create library service libraryService := services.NewLibraryService(queries) + // The progress service verifies submitted anchors against the book + // itself — it needs to resolve library-relative file paths. + progressService.SetMediaPathResolver(libraryService) // Sync Go AllowedExtensions into DB so API clients see correct extensions libraryService.SyncAllowedExtensions(context.Background()) diff --git a/internal/handlers/media.go b/internal/handlers/media.go index 6803b77..5463dfe 100644 --- a/internal/handlers/media.go +++ b/internal/handlers/media.go @@ -953,6 +953,37 @@ func (mh *MediaHandler) GetMediaReadingProgress(c *echo.Context) error { "last_sync_timestamp": progress.LastSyncTimestamp, } + // Position authority: re-verify the stored anchor and derive the + // client-facing handles (cssSelector + anchor document href) server- + // side, so clients scroll to an address instead of parsing CFIs. + if progress.Epubcfi.Valid && wsync.IsConvertibleFormat(progress.FormatGroup) && mh.libraryService != nil { + contextText := "" + if progress.ContextText.Valid { + contextText = progress.ContextText.String + } + pct := 0.0 + if progress.Percentage.Valid { + pct = progress.Percentage.Float64 + } + if mediaItem, err := mh.db.GetMediaItem(c.Request().Context(), pgtype.UUID{Bytes: mediaUUID, Valid: true}); err == nil { + if path, perr := mh.libraryService.ResolveMediaPath(c.Request().Context(), mediaItem.LibraryID, mediaItem.FilePath); perr == nil && path != "" { + if finalCFI, sel, href, charOff, _, _, verr := wsync.VerifyProgressAnchor(path, progress.Epubcfi.String, contextText, pct); verr == nil && sel != "" { + resp["css_selector"] = sel + resp["anchor_href"] = href + if charOff != nil { + resp["char_offset"] = *charOff + } + if finalCFI != "" { + resp["epubcfi"] = finalCFI + } + } + } + } + } + if progress.ContextText.Valid { + resp["context_text"] = progress.ContextText.String + } + return c.JSON(http.StatusOK, resp) } diff --git a/internal/sync/cfi_verify.go b/internal/sync/cfi_verify.go new file mode 100644 index 0000000..931fc74 --- /dev/null +++ b/internal/sync/cfi_verify.go @@ -0,0 +1,315 @@ +package sync + +// Ingest-side position authority for reading progress. +// +// Clients submit (percentage, context_text, epubcfi). The epubcfi is a +// standard wrapped CFI — epubcfi(/6/N!/…) — resolvable against the same +// document this package parses, so every submission is INDEPENDENTLY +// verified: the text at the resolved anchor is extracted and compared +// with the submitted context. A mismatch (or an unresolvable anchor) +// heals the position by text search, with the submitted percentage +// disambiguating repeated phrases, instead of trusting a client-side +// projection. This keeps buggy clients from poisoning stored positions: +// a resolver that silently returns "wherever I'm currently scrolled" +// fails the context check and gets healed to the true location. +// +// The anchor's block element is also derived as a cssSelector plus a +// block-relative character offset and served back — the readium-native +// handle clients scroll to, so they never have to parse or trust CFIs +// themselves. +// +// Note: readium-based clients number their readingOrder excluding +// linear="no" spine items, while this package's spine index follows the +// OPF spine as written. The spine index is therefore internal-only; +// client-facing responses carry the anchor document's href instead. + +import ( + "fmt" + "strings" + + "golang.org/x/net/html" +) + +// IsConvertibleFormat reports whether a format group uses standard CFIs +// as its structural locator currency (i.e. whether CFI verification and +// cssSelector derivation apply to it). +func IsConvertibleFormat(formatGroup string) bool { + return isConvertible(string(formatGroup)) +} + +func truncateRunes(s string, n int) string { + r := []rune(s) + if len(r) <= n { + return s + } + return string(r[:n]) +} + +// anchorBlock returns the nearest non-inline (block-level) ancestor of a +// resolved text/element node — the server-side equivalent of the +// readers' computed-style block walk. +func anchorBlock(node *html.Node) *html.Node { + if node == nil { + return nil + } + if node.Type == html.ElementNode && !isInlineFormatting(node) { + return node + } + return findBlockParent(node) +} + +// blockContextText extracts the normalized text from (node, runeOff) to +// the end of the anchor block — the same excerpt rule the readers use for +// context_text, ≤100 chars. +func blockContextText(block, node *html.Node, runeOff int) string { + segments := collectInlineText(block) + var sb strings.Builder + started := false + for _, seg := range segments { + if !started && seg.node == node { + started = true + runes := []rune(string(seg.runes)) + if runeOff < len(runes) { + sb.WriteString(string(runes[runeOff:])) + } + continue + } + if started { + sb.WriteString(string(seg.runes)) + } + } + return truncateRunes(normalizeWhitespace(sb.String()), 100) +} + +// contextMatches reports whether a server-extracted context and a client- +// submitted context describe the same anchor. Both are suffixes of the +// same block text when the anchors share a block, so containment in +// either direction verifies; empty or very short contexts never match. +func contextMatches(serverCtx, submitted string) bool { + s := truncateRunes(normalizeWhitespace(submitted), 100) + t := truncateRunes(normalizeWhitespace(serverCtx), 100) + if s == "" || t == "" { + return false + } + short, long := s, t + if len([]rune(short)) > len([]rune(long)) { + short, long = long, short + } + if len([]rune(short)) < 12 { + return false + } + return strings.Contains(long, short) +} + +// cssSelectorFor mirrors the readers' selOf: a body-relative +// tag:nth-child(k) chain (k = 1-based position among element siblings). +func cssSelectorFor(block *html.Node) string { + var segs []string + n := block + for n != nil && n.Type == html.ElementNode && n.Data != "body" { + k := 1 + sib := n.Parent.FirstChild + for sib != nil && sib != n { + if sib.Type == html.ElementNode { + k++ + } + sib = sib.NextSibling + } + segs = append([]string{n.Data + ":nth-child(" + fmt.Sprintf("%d", k) + ")"}, segs...) + n = n.Parent + } + return "body>" + strings.Join(segs, ">") +} + +// blockCharOffset computes the rune offset of (node, runeOff) within the +// concatenated text of its block — the client-side scroll target. +func blockCharOffset(block, node *html.Node, runeOff int) int { + segments := collectInlineText(block) + s := 0 + for _, seg := range segments { + if seg.node == node { + return s + runeOff + } + s += len(seg.runes) + } + return s + runeOff +} + +// ProgressAnchor is the full server-computed apply handle for a stored +// standard CFI: the anchor block's cssSelector, the character offset +// within that block's text, and the spine document's href. +type ProgressAnchor struct { + CSSSelector string + CharOffset int + Href string + HealedCFI string + Healed bool + HealedPct *float64 +} + +// VerifyProgressAnchor resolves a client-submitted standard CFI against +// the EPUB, cross-checks the submitted context text, and heals the anchor +// by text search on any mismatch. +func VerifyProgressAnchor(epubPath, epubcfi, contextText string, percentage float64) (finalCFI string, cssSelector string, anchorHref string, charOffset *int, healedPct *float64, healed bool, err error) { + finalCFI = epubcfi + anchorHref = "" + + spineIndex, localSteps, err := parseEPUBCFI(epubcfi) + if err != nil { + cfi, sel, href, off, pct, healedFlag, herr := healFromContext(epubPath, contextText, percentage) + return cfi, sel, href, off, pct, healedFlag, herr + } + conv := cachedConverter(epubPath) + doc, docHref, err := conv.getContentDoc(spineIndex + 1) + if err != nil { + return healFromContext(epubPath, contextText, percentage) + } + node, runeOff, rerr := resolveCFIToNode(doc, localSteps) + if rerr != nil { + return healFromContext(epubPath, contextText, percentage) + } + anchorHref = docHref + + block := anchorBlock(node) + serverCtx := blockContextText(block, node, runeOff) + if contextMatches(serverCtx, contextText) { + off := blockCharOffset(block, node, runeOff) + return finalCFI, cssSelectorFor(block), anchorHref, &off, nil, false, nil + } + + // Mismatch: heal by text search. + hCFI, hPct, hSel, hHref, herr := healAnchorByText(epubPath, contextText, percentage, spineIndex) + if herr != nil { + return finalCFI, "", hHref, nil, nil, false, fmt.Errorf("context mismatch (server %q vs client %q) and heal failed: %w", + truncateRunes(serverCtx, 40), truncateRunes(contextText, 40), herr) + } + hOff := blockCharOffsetFor(epubPath, hCFI) + return hCFI, hSel, hHref, &hOff, &hPct, true, nil +} + +func healFromContext(epubPath, contextText string, percentage float64) (string, string, string, *int, *float64, bool, error) { + cfi, healedPct, sel, href, err := healAnchorByText(epubPath, contextText, percentage, -1) + if err != nil { + return "", "", "", nil, nil, false, err + } + off := blockCharOffsetFor(epubPath, cfi) + return cfi, sel, href, &off, &healedPct, true, nil +} + +func healAnchorByText(epubPath, contextText string, percentage float64, spineIndex int) (string, float64, string, string, error) { + if epubPath == "" { + return "", 0, "", "", fmt.Errorf("no epub available for text anchoring") + } + conv := cachedConverter(epubPath) + spine, err := conv.loadSpine() + if err != nil { + return "", 0, "", "", err + } + needle := truncateRunes(normalizeWhitespace(contextText), 40) + if len([]rune(needle)) < 12 { + return "", 0, "", "", fmt.Errorf("context too short to anchor") + } + + type match struct { + spine int + node *html.Node + off int + } + var matches []match + totalChars := 0 + charsBefore := make([]int, len(spine.items)) + for i := range spine.items { + doc, _, derr := conv.getContentDoc(i + 1) + if derr != nil { + continue + } + body := findBody(doc) + if body == nil { + continue + } + charsBefore[i] = totalChars + totalChars += countTextChars(body) + if n, runeOff := findTextInNode(body, needle); n != nil { + matches = append(matches, match{spine: i, node: n, off: runeOff}) + } + } + if len(matches) == 0 || totalChars <= 0 { + return "", 0, "", "", fmt.Errorf("context not found in book") + } + + best := matches[0] + bestDist := -1.0 + for _, m := range matches { + frac := (float64(charsBefore[m.spine]) + float64(countTextCharsBefore(m.node)+m.off)) / float64(totalChars) + d := frac - percentage + if d < 0 { + d = -d + } + if bestDist < 0 || d < bestDist { + best = m + bestDist = d + } + } + + healedPct := (float64(charsBefore[best.spine]) + float64(countTextCharsBefore(best.node)+best.off)) / float64(totalChars) + cfi, err := buildCFI(best.spine, best.node, best.off) + if err != nil { + return "", 0, "", "", err + } + sel := "" + if block := anchorBlock(best.node); block != nil { + sel = cssSelectorFor(block) + } + return cfi, healedPct, sel, spine.items[best.spine].href, nil +} + +func blockCharOffsetFor(epubPath, cfi string) int { + n, off, _, herr := cfiTextAtAnchorInternal(epubPath, cfi) + if herr != nil { + return 0 + } + block := anchorBlock(n) + if block == nil { + return 0 + } + return blockCharOffset(block, n, off) +} + +// cfiTextAtAnchorInternal resolves a stored standard CFI to its anchor +// text node, rune offset, and containing document. +func cfiTextAtAnchorInternal(epubPath, cfi string) (*html.Node, int, *html.Node, error) { + spineIndex, localSteps, err := parseEPUBCFI(cfi) + if err != nil { + return nil, 0, nil, err + } + conv := cachedConverter(epubPath) + doc, _, err := conv.getContentDoc(spineIndex + 1) + if err != nil { + return nil, 0, nil, err + } + node, off, err := resolveCFIToNode(doc, localSteps) + return node, off, doc, err +} + +// ProgressCSSSelector derives the anchor block's cssSelector from a stored +// standard CFI. +func ProgressCSSSelector(epubPath, epubcfi string) (string, error) { + spineIndex, localSteps, err := parseEPUBCFI(epubcfi) + if err != nil { + return "", err + } + conv := cachedConverter(epubPath) + doc, _, err := conv.getContentDoc(spineIndex + 1) + if err != nil { + return "", err + } + node, _, err := resolveCFIToNode(doc, localSteps) + if err != nil { + return "", err + } + block := anchorBlock(node) + if block == nil { + return "", fmt.Errorf("no block ancestor for anchor") + } + return cssSelectorFor(block), nil +} diff --git a/internal/sync/cfi_verify_manual_test.go b/internal/sync/cfi_verify_manual_test.go new file mode 100644 index 0000000..5b51d1e --- /dev/null +++ b/internal/sync/cfi_verify_manual_test.go @@ -0,0 +1,22 @@ +package sync + +import ( + "os" + "testing" +) + +func TestManualRealBookAnchor(t *testing.T) { + path := "/home/nymusicman/Code/bookhoard/uploads/Ebooks/George Orwell/1984 (1269)/1984 - George Orwell.epub" + if _, err := os.Stat(path); err != nil { + t.Skip("real book not present on this machine") + } + cfi := "epubcfi(/6/2!/4[x1984]/8[_idContainer003]/62/1:456)" + ctx := "was at war with one of these Powers it was generally at peace with the other" + finalCFI, sel, href, charOff, healedPct, healed, err := VerifyProgressAnchor(path, cfi, ctx, 0.0415) + if err != nil { + t.Fatalf("VerifyProgressAnchor error: %v", err) + } + t.Logf("final=%q\n healed=%v healedPct=%v\n selector=%q href=%q charOff=%v", finalCFI, healed, healedPct, sel, href, charOff) + sel2, err2 := ProgressCSSSelector(path, cfi) + t.Logf("ProgressCSSSelector: %q err=%v", sel2, err2) +} diff --git a/internal/sync/cfi_verify_test.go b/internal/sync/cfi_verify_test.go new file mode 100644 index 0000000..a203056 --- /dev/null +++ b/internal/sync/cfi_verify_test.go @@ -0,0 +1,158 @@ +package sync + +import ( + "testing" +) + +// The fixture (writeTestEPUB in cfi_converter_test.go): 6 spine docs. +// doc2 = spine index 1 with two paragraphs: +// p1: "The family of Dashwood had long been settled in Sussex." +// p2: "Their estate was large, and their residence was at Norland Park." +// Hand-derived local paths (html→body /4, body→div /4, div→p /4|/6, +// p→text chunk /1): +const ( + dashwoodCFI = "epubcfi(/6/4!/4/2/2/1:0)" + dashwoodText = "The family of Dashwood had long been settled in Sussex." + estateCFI = "epubcfi(/6/4!/4/2/4/1:0)" + estateText = "Their estate was large, and their residence was at Norland Park." + dashwoodSelect = "body>div:nth-child(1)>p:nth-child(1)" + estateSelect = "body>div:nth-child(1)>p:nth-child(2)" +) + +func TestVerifyProgressAnchorAcceptsExact(t *testing.T) { + path := writeTestEPUB(t) + finalCFI, sel, _, charOff, healedPct, healed, err := VerifyProgressAnchor(path, dashwoodCFI, dashwoodText, 0.3) + _ = charOff + if err != nil { + t.Fatalf("verify error: %v", err) + } + if healed { + t.Errorf("exact anchor should not heal") + } + if healedPct != nil { + t.Errorf("exact anchor should not carry a healed percentage") + } + if finalCFI != dashwoodCFI { + t.Errorf("finalCFI = %q, want unchanged %q", finalCFI, dashwoodCFI) + } + if sel != dashwoodSelect { + t.Errorf("cssSelector = %q, want %q", sel, dashwoodSelect) + } +} + +func TestVerifyProgressAnchorHealsMismatch(t *testing.T) { + path := writeTestEPUB(t) + // Anchored at p1 but the context is p2's text: the classic + // client-projection bug — the server must heal to the true location. + finalCFI, _, _, charOff, healedPct, healed, err := VerifyProgressAnchor(path, dashwoodCFI, estateText, 0.3) + _ = charOff + if err != nil { + t.Fatalf("verify error: %v", err) + } + if !healed { + t.Fatalf("expected healing, got none (cfi=%q)", finalCFI) + } + if finalCFI != estateCFI { + t.Errorf("healed CFI = %q, want %q", finalCFI, estateCFI) + } + if healedPct == nil || *healedPct <= 0 { + t.Errorf("healed percentage not recomputed: %v", healedPct) + } + + // Healing must converge: re-verifying the healed anchor is a no-op. + finalCFI2, _, _, _, healedPct2, healed2, err := VerifyProgressAnchor(path, finalCFI, estateText, 0.3) + if err != nil { + t.Fatalf("re-verify error: %v", err) + } + if healed2 { + t.Errorf("healed anchor should be stable, got healed again to %q (pct %v)", finalCFI2, healedPct2) + } + if finalCFI2 != finalCFI { + t.Errorf("re-verify CFI = %q, want %q", finalCFI2, finalCFI) + } +} + +func TestVerifyProgressAnchorHealsUnresolvable(t *testing.T) { + path := writeTestEPUB(t) + // Element index 99 is out of range in doc2 — the anchor cannot resolve. + bogus := "epubcfi(/6/4!/4/2/99/1:0)" + finalCFI, _, _, charOff, healedPct, healed, err := VerifyProgressAnchor(path, bogus, dashwoodText, 0.05) + _ = charOff + if err != nil { + t.Fatalf("verify error: %v", err) + } + if !healed { + t.Fatal("unresolvable anchor should heal from context") + } + if finalCFI != dashwoodCFI { + t.Errorf("healed CFI = %q, want the Dashwood anchor %q", finalCFI, dashwoodCFI) + } + if healedPct == nil { + t.Errorf("healed percentage not recomputed") + } +} + +func TestVerifyProgressAnchorDumbClient(t *testing.T) { + path := writeTestEPUB(t) + // No CFI at all — a client that only knows percentage + context is + // fully supported: the server anchors structurally from the text. + finalCFI, sel, _, charOff, healedPct, healed, err := VerifyProgressAnchor(path, "", estateText, 0.3) + _ = charOff + if err != nil { + t.Fatalf("verify error: %v", err) + } + if !healed { + t.Fatal("context-only submission should count as anchored-by-heal") + } + if finalCFI != estateCFI { + t.Errorf("anchored CFI = %q, want %q", finalCFI, estateCFI) + } + if sel != estateSelect { + t.Errorf("cssSelector = %q, want %q", sel, estateSelect) + } + if healedPct == nil { + t.Errorf("percentage not recomputed for context-only anchor") + } +} + +func TestContextMatches(t *testing.T) { + block := "The family of Dashwood had long been settled in Sussex." + cases := []struct { + name string + serverCtx string + submitted string + want bool + }{ + {"exact", block, block, true}, + {"suffix of block", block, "settled in Sussex.", true}, + {"block is suffix", "settled in Sussex.", block, true}, + {"mid-block substring", block, "Dashwood had long been", true}, + {"different text", block, "completely unrelated words here", false}, + {"too short", block, "the", false}, + {"empty submitted", block, "", false}, + {"empty server", "", "some long enough context text", false}, + } + for _, tc := range cases { + if got := contextMatches(tc.serverCtx, tc.submitted); got != tc.want { + t.Errorf("contextMatches(%q, %q) = %v, want %v", tc.serverCtx, tc.submitted, got, tc.want) + } + } +} + +func TestParseStandardCFIRange(t *testing.T) { + // Web progress CFIs are range CFIs; parse must resolve to the start arm. + spineIndex, localSteps, err := parseEPUBCFI("epubcfi(/6/4!/4/4:0,/4/4:53)") + if err != nil { + t.Fatalf("parse error: %v", err) + } + if spineIndex != 1 { + t.Errorf("spineIndex = %d, want 1", spineIndex) + } + if len(localSteps) != 4 { + t.Fatalf("steps = %d, want 4 (parent + start arm)", len(localSteps)) + } + last := localSteps[len(localSteps)-1] + if last.Index != 4 || !last.HasOffset || last.Offset != 53 { + t.Errorf("start-arm step = %+v, want /4:0", last) + } +} diff --git a/internal/sync/progress.go b/internal/sync/progress.go index ec3c647..13326d1 100644 --- a/internal/sync/progress.go +++ b/internal/sync/progress.go @@ -274,8 +274,18 @@ func EstimatedPages(totalCharacters int64) int { type ProgressService struct { db *database.Queries connManager *ConnectionManager + mediaPaths MediaPathResolver } +// MediaPathResolver resolves a media item's library-relative file path to +// an absolute path, so the progress service can verify submitted anchors +// against the book itself. +type MediaPathResolver interface { + ResolveMediaPath(ctx context.Context, libraryID pgtype.UUID, relativePath string) (string, error) +} + +func (s *ProgressService) SetMediaPathResolver(r MediaPathResolver) { s.mediaPaths = r } + func NewProgressService(db *database.Queries, connManager *ConnectionManager) *ProgressService { return &ProgressService{db: db, connManager: connManager} } @@ -435,6 +445,39 @@ func (s *ProgressService) SaveProgress(ctx context.Context, req SaveProgressRequ } } + // Canonical position authority: resolve the submitted CFI against the + // book itself, cross-check the submitted context text, and heal the + // anchor by text search on any mismatch. Clients submit what their + // renderer can reliably observe (percentage + visible text); the + // server owns the structural math and keeps buggy clients from + // poisoning stored positions. + if !isFixed && s.mediaPaths != nil && params.ContextText.Valid && + isConvertible(string(formatGroup)) && formatGroup != "" { + if path, perr := s.mediaPaths.ResolveMediaPath(ctx, mediaItem.LibraryID, mediaItem.FilePath); perr == nil && path != "" { + submittedCFI := "" + if params.Epubcfi.Valid { + submittedCFI = params.Epubcfi.String + } + submittedPct := 0.0 + if params.Percentage.Valid { + submittedPct = params.Percentage.Float64 + } + if finalCFI, _, _, charOff, healedPct, healed, verr := VerifyProgressAnchor(path, submittedCFI, params.ContextText.String, submittedPct); verr != nil { + log.Printf("Bookhoard: progress anchor verify failed for %s: %v", req.MediaItemID.String(), verr) + } else { + if healed || (!params.Epubcfi.Valid && finalCFI != "") { + params.Epubcfi = pgtype.Text{String: finalCFI, Valid: true} + if healedPct != nil { + params.Percentage = pgtype.Float8{Float64: *healedPct, Valid: true} + } + if charOff != nil { + params.CharacterOffset = pgtype.Int8{Int64: int64(*charOff), Valid: true} + } + } + } + } + } + conflictDetected := false if hasExisting && existing.LastSyncSource.Valid && existing.LastSyncSource.String != req.Source { if existing.LastSyncTimestamp.Valid {