Artifact 44e4dcee2496c425d7a50ac422593e4c7cb0fe6337438ba14cc6508d06ffb4b3:
- File
internal/sync/clone.go
— part of check-in
[89535a6c42]
at
2026-07-19 18:28:45
on branch trunk
— fix: store delta content against a phantomized base instead of discarding it (#83)
* fix: store delta content against a phantomized base instead of discarding it
A delta artifact arriving before its base is normal steady-state behavior during a Fossil transfer, not a fault. The receive path used to discard such deltas outright and phantomize the target instead, so a phantom only ever cleared when its base happened to already be present -- resolution advanced one delta-chain hop per round, making clones of deep-history repos provably unable to converge within MaxRounds.
Match canonical Fossil's content_put_ex/rid_from_uuid model: resolve the delta's source via a create-phantom-if-missing lookup, persist the delta bytes unconditionally (reading the target size straight out of the delta header, so the base never needs to be readable to store it), and record the delta-to-source link immediately. Availability resolves lazily via the existing content.IsAvailable chain walk and manifest.Crosslink's deferred retry -- both already correct and untouched here.
storeReceivedFile is the single call site shared by clone, sync-pull, and server-push, so fixing it here also closes out the delta-before-base sentinel handling on the pull and push paths (issue #57, folded into this fix): neither path needs to special-case the condition anymore because it stops being an error.
Verified against the pikchr corpus via a loopback fossil server: rounds dropped from 100 (exhausted) to 3, files received from 63,155 (22.2x) to 2,980 (1.05x), and the phantom table empties out completely.
* fix: converge delta storage on canonical, close integrity gap, harden wire parsing
Three defects from review, all in the raw-delta path added for the delta-before-base fix, all reachable from an untrusted peer:
1. delta.getInt accumulated v = v*64 + digit with no overflow check. A twelve-byte header of maximum-value digits wraps to exactly math.MaxUint64, which casts to int64(-1) -- the same sentinel blob.size uses for "phantom". That silently corrupted the row: real content reporting as a phantom, with no phantom-table entry to make it re-requestable. getInt now rejects overflow instead of wrapping, and parseTargetLen (shared by Apply and OutputSize) bounds the parsed target length to a sane maximum before it ever reaches an allocation or a size column.
2. StoreDeltaRaw panicked on a zero-length delta payload. storeReceivedFile guarded payload == nil but not len(payload) == 0, and nothing between wire decode and storage checked for it -- a peer sending an empty delta file card crashed the process. Wire data gets an error return, not an assertion; OutputSize and StoreDeltaRaw are now consistent with delta.Apply's existing "empty delta is an error" contract.
3. The receive-time split (store raw only when the base happens to be unavailable, otherwise eagerly expand and verify) left a gap an attacker chooses the timing of: withhold the base, and a delta claiming any UUID goes down the unverified raw path. Nothing downstream checked it either -- content.Expand applied deltas with no hash check, and content.Verify was reachable only from repo rebuild and one integration test.
Closed by converging fully on canonical's model: every delta is now stored raw and linked via a create-phantom-if-missing lookup, regardless of whether its base is currently available -- receive time no longer branches on availability at all. Verification moves to the one choke point every expansion path already goes through: content.Expand now hashes its result against the row's claimed UUID before returning it, catching a mismatch regardless of which caller or how the chain assembled. content.Verify becomes a thin wrapper over the same check.
Storage-layer and end-to-end regression tests added for all three, including the exact claim-one-UUID/ship-a-delta-for-another/withhold-the- base reproduction, guarded against degenerating into a no-op (delta must actually compress, base must be genuinely absent, rejection must be a real error rather than merely different bytes).
Re-verified against pikchr (2,842 artifacts) and wapp (1,039): pikchr converges in 3 rounds, 2,980 files, 0 phantoms; wapp converges in 3 rounds, 1,115 files, 1 phantom outstanding (pre-existing, unrelated -- clone.go has no DB-phantom rescan the way the pull path's loadDBPhantoms does; separately tracked). On-disk repository size is close to but slightly smaller than storing everything as raw delta bytes should produce (pikchr 6.81MB -> 6.73MB, wapp 5.64MB -> 5.46MB): zlib on full text already captures most of the redundancy an explicit delta would, so the win here is mainly integrity and convergence, not size.
* fix: bound delta allocation to actual work, validate delta source hex
Two defects from round-two review, plus a stray comment.
1. delta.Apply preallocated its output buffer directly from the delta header's claimed target length (make([]byte, 0, targetLen)). That claim only had to clear maxDeltaTargetSize (4 GiB) to be trusted as an allocation size, so a handful of wire bytes declaring a near-4 GiB target triggered an allocation sized to the claim, not to any work actually done. Harmless as a one-off parse spike; durable once a delta can be stored unverified (see blob.StoreDeltaRaw, this branch) -- a hostile peer plants one such delta and every later Expand of that row repeats the attempt. The initial capacity is now a small fixed bound; append grows it normally for legitimately large content instead of trusting the header's own claim about itself.
2. storeReceivedFile validated the target UUID's hex-ness (hash.IsValidHash) but not the delta source's. The blob table's CHECK constraint only bounds uuid length, not hex-ness, so a non-hex string of valid length passed straight through to blob.StorePhantom and became a permanent, unfillable row -- one the pull path's loadDBPhantoms would then re-request every sync round, forever. deltaSrc now gets the same check as the target uuid, at the same place.
Also deleted a stray doc-comment line left over from an earlier refactor.
Regression tests for both, RED-verified against reverted code before being fixed: the allocation test had to be corrected mid-way through -- delta headers encode their integer in Fossil's own base-64 alphabet, not decimal, so an initial fmt.Sprintf("%d", ...) fixture parsed as a different (still large but differently-bounded) value and passed vacuously; appendInt is the real encoder and is used directly now, with a round-trip sanity check against OutputSize guarding the fixture itself. (user: [email protected], size: 13081) [annotate] [blame] [check-ins using]
package sync import ( "bytes" "context" "fmt" "os" "github.com/danmestas/libfossil/internal/content" "github.com/danmestas/libfossil/internal/manifest" "github.com/danmestas/libfossil/internal/repo" "github.com/danmestas/libfossil/simio" "github.com/danmestas/libfossil/internal/xfer" ) // cloneSession holds the mutable state of a running clone. type cloneSession struct { repo *repo.Repo env *simio.Env opts CloneOpts result CloneResult phantoms map[string]bool seqno int projectCode string serverCode string cookie string // server-issued snapshot bound; echoed every round so the // server can scope its blob query to the rid range that existed when the // clone session opened. See cloneCapPrefix in handler.go. obs Observer } // Clone performs a full repository clone from a remote Fossil server. // It creates a new repository at path, runs the clone protocol until // convergence, and returns the opened repo and a result summary. // On error, the partially-created repo file is removed. func Clone(ctx context.Context, path string, t Transport, opts CloneOpts) (r *repo.Repo, result *CloneResult, err error) { if path == "" { panic("sync.Clone: path must not be empty") } if t == nil { panic("sync.Clone: t must not be nil") } env := opts.Env if env == nil { env = simio.RealEnv() } storage := env.Storage if storage == nil { storage = simio.OSStorage{} } // Path must not already exist. if _, statErr := storage.Stat(path); statErr == nil { return nil, nil, fmt.Errorf("sync.Clone: file already exists: %s", path) } user := opts.User if user == "" { user = "setup" } // Create the repository. r, err = repo.CreateWithEnv(path, user, env, "") if err != nil { return nil, nil, fmt.Errorf("sync.Clone: create repo: %w", err) } // Cleanup on error: close repo and remove the file. defer func() { if err != nil { r.Close() if rmErr := storage.Remove(path); rmErr != nil { fmt.Fprintf(os.Stderr, "sync.Clone: cleanup failed: %v\n", rmErr) } r = nil } }() // Clear project-code — the server will provide its own. if _, execErr := r.DB().Exec("DELETE FROM config WHERE name='project-code'"); execErr != nil { err = fmt.Errorf("sync.Clone: clear project-code: %w", execErr) return } cs := &cloneSession{ repo: r, env: env, opts: opts, seqno: 1, phantoms: make(map[string]bool), } cs.obs = resolveObserver(opts.Observer) cloneResult, cloneErr := cs.run(ctx, t) if cloneErr != nil { err = cloneErr return } // Crosslink: parse received manifests into event/plink/leaf/mlink tables. linked, xlinkErr := manifest.Crosslink(r) if xlinkErr != nil { err = fmt.Errorf("sync.Clone: crosslink: %w", xlinkErr) return } cloneResult.ArtifactsLinked = linked return r, cloneResult, nil } // run executes the clone loop. func (cs *cloneSession) run(ctx context.Context, t Transport) (*CloneResult, error) { ctx = cs.obs.Started(ctx, SessionStart{ Operation: "clone", Pull: true, }) prevPhantomCount := -1 for cycle := 0; ; cycle++ { select { case <-ctx.Done(): cs.obs.Completed(ctx, sessionEndFromClone(&cs.result), ctx.Err()) return &cs.result, ctx.Err() default: } if cycle >= MaxRounds { err := fmt.Errorf("sync.Clone: exceeded %d rounds", MaxRounds) cs.obs.Completed(ctx, sessionEndFromClone(&cs.result), err) return &cs.result, err } roundCtx := cs.obs.RoundStarted(ctx, cycle) req, err := cs.buildRequest(cycle) if err != nil { cs.obs.RoundCompleted(roundCtx, cycle, RoundStats{}) cs.obs.Completed(ctx, sessionEndFromClone(&cs.result), err) return &cs.result, fmt.Errorf("sync.Clone: buildRequest round %d: %w", cycle, err) } resp, err := t.Exchange(ctx, req) if err != nil { cs.obs.RoundCompleted(roundCtx, cycle, RoundStats{}) cs.obs.Completed(ctx, sessionEndFromClone(&cs.result), err) return &cs.result, fmt.Errorf("sync.Clone: exchange round %d: %w", cycle, err) } recvdBefore := cs.result.BlobsRecvd done, err := cs.processResponse(resp) if err != nil { cs.obs.RoundCompleted(roundCtx, cycle, RoundStats{FilesReceived: cs.result.BlobsRecvd - recvdBefore}) cs.obs.Completed(ctx, sessionEndFromClone(&cs.result), err) return &cs.result, fmt.Errorf("sync.Clone: process round %d: %w", cycle, err) } cs.result.Rounds = cycle + 1 cs.obs.RoundCompleted(roundCtx, cycle, RoundStats{FilesReceived: cs.result.BlobsRecvd - recvdBefore}) // Convergence: need at least 2 rounds. // Continue while seqno > 0 or phantoms remain with progress. if cycle >= 1 && done { if cs.seqno <= 0 && len(cs.phantoms) == 0 { break } // If phantoms remain but no progress, stop. phantomCount := len(cs.phantoms) if cs.seqno <= 0 && phantomCount > 0 && phantomCount >= prevPhantomCount { break } prevPhantomCount = phantomCount if cs.seqno <= 0 { // Only phantoms remain, keep going if making progress. continue } } if cycle >= 1 { prevPhantomCount = len(cs.phantoms) } } cs.result.ProjectCode = cs.projectCode cs.result.ServerCode = cs.serverCode cs.obs.Completed(ctx, sessionEndFromClone(&cs.result), nil) return &cs.result, nil } // sessionEndFromClone builds a SessionEnd from a CloneResult. func sessionEndFromClone(r *CloneResult) SessionEnd { return SessionEnd{ Operation: "clone", Rounds: r.Rounds, FilesRecvd: r.BlobsRecvd, ProjectCode: r.ProjectCode, Errors: r.Messages, } } // buildRequest assembles one outbound xfer message for a clone round. func (cs *cloneSession) buildRequest(cycle int) (*xfer.Message, error) { var cards []xfer.Card // Pragma: client-version (every round) cards = append(cards, &xfer.PragmaCard{ Name: "client-version", Values: []string{"22800", "20260315", "120000"}, }) // Cookie carries the server-captured clone snapshot bound. Echoing it back // every round lets the stateless handler enforce a stable upper rid bound // on its batch query, so a hub that is being written to during a clone // session can't keep extending the queue and prevent convergence. if cs.cookie != "" { cards = append(cards, &xfer.CookieCard{Value: cs.cookie}) } // Clone card — only when seqno > 0 (sequential delivery in progress). // When seqno reaches 0, the server has sent all blobs and the client // switches to gimme-based phantom resolution (matching Fossil xfer.c:2706). if cs.seqno > 0 { version := cs.opts.Version if version <= 0 { version = 3 } cards = append(cards, &xfer.CloneCard{ Version: version, SeqNo: cs.seqno, }) // CloneSeqNoCard carries the actual pagination cursor server-side // (see TestHandleClonePagination). The CloneCard alone only signals // clone mode; without this card the server's cloneSeq stays 0 every // round and the same blob prefix is re-emitted forever — fine when // a repo fits in one batch, fatal under writing-hub or smallBatch // conditions. Skip on round 0 so the server treats it as a fresh // session (any starting cursor would skip rid <= cs.seqno). if cycle > 0 { cards = append(cards, &xfer.CloneSeqNoCard{SeqNo: cs.seqno}) } } else { // Pull mode for phantom resolution after sequential delivery completes. if cs.projectCode != "" && cs.serverCode != "" { cards = append(cards, &xfer.PullCard{ ServerCode: cs.serverCode, ProjectCode: cs.projectCode, }) } } // Gimme cards for phantoms — only when seqno <= 1 (main transfer done or finishing). if cs.seqno <= 1 { gimmes := make([]string, 0, len(cs.phantoms)) for uuid := range cs.phantoms { gimmes = append(gimmes, uuid) } // BUGGIFY: 5% chance drop the last gimme card. if len(gimmes) > 1 && cs.opts.Buggify != nil && cs.opts.Buggify.Check("clone.buildRequest.dropGimme", 0.05) { gimmes = gimmes[:len(gimmes)-1] } for _, uuid := range gimmes { cards = append(cards, &xfer.GimmeCard{UUID: uuid}) } } // Login card: skip round 0. On round 1+, only if User is set AND projectCode received. if cycle > 0 && cs.opts.User != "" && cs.projectCode != "" { loginCard, err := cs.buildLoginCard(cards) if err != nil { return nil, fmt.Errorf("clone buildLoginCard: %w", err) } cards = append([]xfer.Card{loginCard}, cards...) } return &xfer.Message{Cards: cards}, nil } // buildLoginCard encodes the non-login cards, appends a random comment, // then computes the login card. func (cs *cloneSession) buildLoginCard(cards []xfer.Card) (*xfer.LoginCard, error) { var buf bytes.Buffer for _, c := range cards { if err := xfer.EncodeCard(&buf, c); err != nil { return nil, err } } payload := appendRandomComment(buf.Bytes(), cs.env.Rand) login := computeLogin(cs.opts.User, cs.opts.Password, cs.projectCode, payload) // BUGGIFY: 5% chance corrupt login nonce to test auth failure recovery. if cs.opts.Buggify != nil && cs.opts.Buggify.Check("clone.buildRequest.badLogin", 0.05) { login.Nonce = "corrupted-nonce" } return login, nil } // processResponse handles all cards in a server response for a clone round. // Returns true when the round produced no new file content. func (cs *cloneSession) processResponse(msg *xfer.Message) (bool, error) { if msg == nil { panic("sync.Clone.processResponse: msg must not be nil") } filesRecvd := 0 for _, card := range msg.Cards { switch c := card.(type) { case *xfer.PushCard: // Server sends push card with project-code and server-code. if c.ProjectCode != "" && cs.projectCode == "" { cs.projectCode = c.ProjectCode if _, err := cs.repo.DB().Exec( "REPLACE INTO config(name, value) VALUES('project-code', ?)", c.ProjectCode, ); err != nil { return false, fmt.Errorf("sync.Clone: store project-code: %w", err) } } if c.ServerCode != "" && cs.serverCode == "" { cs.serverCode = c.ServerCode if _, err := cs.repo.DB().Exec( "REPLACE INTO config(name, value) VALUES('server-code', ?)", c.ServerCode, ); err != nil { return false, fmt.Errorf("sync.Clone: store server-code: %w", err) } } case *xfer.FileCard: content := c.Content // BUGGIFY: 2% chance corrupt file content to test hash verification. // Relies on blob.Store verify-before-commit to catch corruption. if cs.opts.Buggify != nil && cs.opts.Buggify.Check("clone.processResponse.corruptHash", 0.02) { corrupted := make([]byte, len(content)) copy(corrupted, content) if len(corrupted) > 0 { corrupted[0] ^= 0xff } content = corrupted } // BUGGIFY: 5% chance skip storing a received file, creating a phantom. if cs.opts.Buggify != nil && cs.opts.Buggify.Check("clone.processResponse.dropFile", 0.05) { filesRecvd++ continue } if err := cs.handleFile(c.UUID, c.DeltaSrc, content); err != nil { return false, err } filesRecvd++ case *xfer.CFileCard: content := c.Content // BUGGIFY: 2% chance corrupt file content to test hash verification. // Relies on blob.Store verify-before-commit to catch corruption. if cs.opts.Buggify != nil && cs.opts.Buggify.Check("clone.processResponse.corruptHash", 0.02) { corrupted := make([]byte, len(content)) copy(corrupted, content) if len(corrupted) > 0 { corrupted[0] ^= 0xff } content = corrupted } // BUGGIFY: 5% chance skip storing a received file, creating a phantom. if cs.opts.Buggify != nil && cs.opts.Buggify.Check("clone.processResponse.dropFile", 0.05) { filesRecvd++ continue } if err := cs.handleFile(c.UUID, c.DeltaSrc, content); err != nil { return false, err } filesRecvd++ case *xfer.CloneSeqNoCard: // BUGGIFY: 5% chance ignore completion signal, forcing extra round. if c.SeqNo == 0 && cs.opts.Buggify != nil && cs.opts.Buggify.Check("clone.processResponse.dropSeqNo", 0.05) { continue } cs.seqno = c.SeqNo case *xfer.ErrorCard: return false, fmt.Errorf("sync.Clone: server error: %s", c.Message) case *xfer.CookieCard: cs.cookie = c.Value case *xfer.MessageCard: cs.result.Messages = append(cs.result.Messages, c.Message) } } cs.result.BlobsRecvd += filesRecvd return filesRecvd == 0, nil } // handleFile stores a received file. storeReceivedFile now persists a // delta whose base hasn't arrived yet rather than discarding it (see // storeDeltaAgainstPhantomBase), so uuid itself is never re-requested here // once stored — only a still-missing base might need another round. func (cs *cloneSession) handleFile(uuid, deltaSrc string, payload []byte) error { if err := storeReceivedFile(cs.repo, uuid, deltaSrc, payload); err != nil { return fmt.Errorf("sync.Clone: handleFile %s: %w", uuid, err) } delete(cs.phantoms, uuid) if deltaSrc != "" { // Availability, not existence: a phantom row for deltaSrc still // needs requesting, and so does a delta whose own base is a // phantom. if _, available := content.AvailableByUUID(cs.repo.DB(), deltaSrc); !available { cs.phantoms[deltaSrc] = true } } return nil }