Artifact a1d98c71babb9f8b6af7abc0713657771a4ded1f84cad74c4787bd0072d92d2b:
- File
internal/deck/parse.go
— part of check-in
[5036cae392]
at
2026-07-19 22:27:35
on branch trunk
— Enforce card occurrence classes and intra-run ordering (#94)
* Enforce card occurrence classes and intra-run ordering
The deck parser enforced card-type ordering and nothing else. Each of the 14 single-occurrence card types was handled by a case that assigned, so a second occurrence silently overwrote the first, and the repeatable types were appended with no sort check at all. draft-fossil-artifact-format-00 §4.5.1 and §4.5.2 define both rules; implement them.
Duplicate rejection covers A, B, C, D, E, G, H, I, K, L, N, R, U and W. The guard is separate from the card-type check because that check permits equal consecutive letters. F, J and M gain a strictly-ascending check against the preceding entry, T the two-level (name, target hash) rule with the full sign-prefixed name as primary key. P and Q deliberately get neither: P order is semantic — primary parent first — and Q is unordered-repeatable. P also now appends across cards instead of replacing, which is what §4.5.1 means by repeatable. Z belongs to neither class, so it is accepted with no duplicate or last-card rule; the authoritative checksum is the trailing 35 bytes VerifyZ already verified.
All four ordering checks route through one comparator, deck.Compare, which is §4.5.3: unsigned octets, locale-independent, no normalization, no case folding. §4.5.3 exists because the same ordering underlies the delta-manifest merge walk and baseline binary search of §8.2, so a second copy would resolve a delta manifest wrongly rather than merely fail validation. The F check runs on the decoded filename, per §4.5.2.
Malformed input is reported through the error return, never a panic -- these bytes arrive from peers.
Two writer-side defects surfaced, both of which let this package emit artifacts its own parser now rejects. Marshal emitted J cards in caller-supplied order while already sorting F, M and T; it now sorts them. Marshal also sorted T by the concatenation name+target, which lets the target's leading hex digit compete with the tail of a longer name and reorders tags whose names are prefixes of one another; it now uses the two-level key. The F and M sorts and the R-card file enumeration route through deck.Compare as well, the latter per §6.3.
Closes #70
* Order T cards by decoded name and type the Z payload
Review found two ways the first pass diverged from canonical fossil in the accept/reject direction.
The T ordering key used the raw wire name. §4.7.16 says the name is escape-decoded and that the primary ordering key is the full name token including its leading sign character; manifest.c defossilizes the name before the ordering compare. Because "\s" begins 0x5C and a space is 0x20, the two forms order oppositely, so a run like
T *sym-a\sb aaaa... T *sym-a-c bbbb...
is valid, fossil accepts it, and we rejected it. Branch names may contain spaces, so this is an ordinary tag, not a corner case -- the same post-decode trap already handled for F. tagOrderKey now builds the decoded key and both the parser check and the marshaller's sort call it, so the two cannot drift apart.
The Z card accepted any payload. Accepting a Z line inside the body is right -- §4.5.1 puts Z in neither occurrence class and treats an earlier Z as content covered by the final checksum -- but §4.7.19 still fixes the payload at exactly 32 hexadecimal characters. Validating nothing left an arbitrary-byte channel in an otherwise fully typed grammar and would have stored and forwarded artifacts canonical refuses. Upper-case digits are accepted, matching fossil's validate16.
Also: the T sign prefix is now checked against {+,-,*} per §4.7.16, both because canonical checks it and because an unvalidated sign byte >= 0x80 would UTF-8-expand to two bytes when the ordering key is assembled and silently corrupt the comparison. Two stale §4.7.20 citations corrected to §4.7.16, and the two-level T test gained the prefix-name case that actually distinguishes a two-level comparison from a concatenated one.
* Cite the normative grammar for the case-insensitive Z token
The Z payload's case-insensitivity was justified by inferring it from the word "hexadecimal" in §4.7.19. §6.1 states it normatively: md5-token = 32hexdig-ci, a parse-accept grammar admitting both 0-9a-f and A-F. Cite that instead, and record the split it implies -- the structural check is case-insensitive while §6.2 keeps verification byte-exact against the lowercase form.
Also soften deck.Compare's claim that the delta-manifest merge walk relies on F ordering to the future tense parse.go already uses: the merge walk is a map overlay today with no comparator to share. (user: [email protected], size: 9741) [annotate] [blame] [check-ins using]
package deck import ( "bytes" "fmt" "strconv" "strings" "time" ) func Parse(data []byte) (*Deck, error) { if data == nil { panic("deck.Parse: data must not be nil") } if err := VerifyZ(data); err != nil { return nil, fmt.Errorf("deck.Parse: %w", err) } // body is safe: VerifyZ above guarantees len(data) >= 35. body := data[:len(data)-35] d := &Deck{} var lastCard byte var seen seenCards reader := bytes.NewReader(body) for reader.Len() > 0 { line, err := readLine(reader) if err != nil || len(line) == 0 { continue } card := line[0] // §4.5.1 occurrence classes. The card-type ordering check below // permits equal consecutive letters, so the duplicate guard for // single-occurrence types has to be separate from it. W is // single-occurrence too, so the claim precedes the W branch. if err := seen.claim(card); err != nil { return nil, fmt.Errorf("deck.Parse: %w", err) } if card == 'W' { if card < lastCard { return nil, fmt.Errorf("deck.Parse: card 'W' out of order (after '%c')", lastCard) } lastCard = card sizeStr := strings.TrimSpace(line[2:]) size, err := strconv.Atoi(sizeStr) if err != nil { return nil, fmt.Errorf("deck.Parse: bad W size: %w", err) } content := make([]byte, size) n, readErr := reader.Read(content) if readErr != nil && n != size { return nil, fmt.Errorf("deck.Parse: W content read: %w", readErr) } if n != size { return nil, fmt.Errorf("deck.Parse: W content: got %d, want %d", n, size) } reader.ReadByte() // trailing newline d.W = content continue } if card < lastCard { return nil, fmt.Errorf("deck.Parse: card '%c' out of order (after '%c')", card, lastCard) } lastCard = card if len(line) < 2 || line[1] != ' ' { return nil, fmt.Errorf("deck.Parse: malformed: %q", line) } args := line[2:] if err := parseCard(d, card, args); err != nil { return nil, fmt.Errorf("deck.Parse: %w", err) } } d.Type = inferType(d) return d, nil } func readLine(r *bytes.Reader) (string, error) { var b strings.Builder for { c, err := r.ReadByte() if err != nil { return b.String(), nil } if c == '\n' { return b.String(), nil } b.WriteByte(c) } } func parseCard(d *Deck, card byte, args string) error { if d == nil { panic("deck.parseCard: d must not be nil") } switch card { case 'A': return parseACard(d, args) case 'D': return parseDCard(d, args) case 'E': return parseECard(d, args) case 'F': return parseFCard(d, args) case 'J': return parseJCard(d, args) case 'Q': return parseQCard(d, args) case 'T': return parseTCard(d, args) // Simple cards stay inline: case 'B': d.B = strings.TrimSpace(args) return nil case 'C': d.C = FossilDecode(args) return nil case 'G': d.G = strings.TrimSpace(args) return nil case 'H': d.H = FossilDecode(args) return nil case 'I': d.I = strings.TrimSpace(args) return nil case 'K': d.K = strings.TrimSpace(args) return nil case 'L': d.L = FossilDecode(args) return nil case 'M': return parseMCard(d, args) case 'N': d.N = strings.TrimSpace(args) return nil case 'P': // Repeatable, and neither sorted nor deduplicated (§4.5.2): P // order is semantic, primary parent first. Appending — not // assigning — is what makes a second P card add parents rather // than replace them (§4.5.1). d.P = append(d.P, strings.Fields(args)...) return nil case 'R': d.R = strings.TrimSpace(args) return nil case 'U': return parseUCard(d, args) case 'Z': return parseZCard(args) default: return fmt.Errorf("unknown card '%c'", card) } } func parseACard(d *Deck, args string) error { parts := strings.SplitN(args, " ", 3) if len(parts) < 2 { return fmt.Errorf("A-card needs 2+ fields") } ac := &AttachmentCard{Filename: FossilDecode(parts[0]), Target: parts[1]} if len(parts) == 3 { ac.Source = parts[2] } d.A = ac return nil } func parseDCard(d *Deck, args string) error { t, err := parseTimestamp(args) if err != nil { return fmt.Errorf("D-card: %w", err) } d.D = t return nil } func parseECard(d *Deck, args string) error { parts := strings.SplitN(args, " ", 2) if len(parts) != 2 { return fmt.Errorf("E-card needs datetime and uuid") } t, err := parseTimestamp(parts[0]) if err != nil { return fmt.Errorf("E-card: %w", err) } d.E = &EventCard{Date: t, UUID: parts[1]} return nil } func parseFCard(d *Deck, args string) error { parts := strings.Fields(args) if len(parts) == 0 { return fmt.Errorf("empty F-card") } fc := FileCard{Name: FossilDecode(parts[0])} if len(parts) >= 2 { fc.UUID = parts[1] } if len(parts) >= 3 { fc.Perm = parts[2] } if len(parts) >= 4 { fc.OldName = FossilDecode(parts[3]) } // §4.5.2: strictly ascending by decoded filename. Decoding has already // happened above, so the check sees the post-decode name the merge walk // and baseline binary search of §8.2 will later rely on. if n := len(d.F); n > 0 { if err := requireAscending('F', d.F[n-1].Name, fc.Name); err != nil { return err } } d.F = append(d.F, fc) return nil } func parseMCard(d *Deck, args string) error { hash := strings.TrimSpace(args) // §4.5.2: strictly ascending by hash. if n := len(d.M); n > 0 { if err := requireAscending('M', d.M[n-1], hash); err != nil { return err } } d.M = append(d.M, hash) return nil } func parseJCard(d *Deck, args string) error { parts := strings.SplitN(args, " ", 2) jf := TicketField{Name: FossilDecode(parts[0])} if len(parts) == 2 { jf.Value = parts[1] } // §4.5.2: strictly ascending by field name. if n := len(d.J); n > 0 { if err := requireAscending('J', d.J[n-1].Name, jf.Name); err != nil { return err } } d.J = append(d.J, jf) return nil } func parseQCard(d *Deck, args string) error { if len(args) < 2 { return fmt.Errorf("Q-card too short") } cp := CherryPick{IsBackout: args[0] == '-'} rest := args[1:] parts := strings.SplitN(rest, " ", 2) cp.Target = parts[0] if len(parts) == 2 { cp.Baseline = parts[1] } d.Q = append(d.Q, cp) return nil } func parseTCard(d *Deck, args string) error { if len(args) < 2 { return fmt.Errorf("T-card too short") } // §4.7.16: the first character of the name token MUST be +, - or *. // Rejecting anything else also keeps the sign a single byte, so the // ordering key assembled from it cannot silently gain a second byte // through UTF-8 expansion. switch TagType(args[0]) { case TagSingleton, TagPropagating, TagCancel: default: return fmt.Errorf("T-card name must begin with '+', '-' or '*', got %q", args[:1]) } tc := TagCard{Type: TagType(args[0])} parts := strings.SplitN(args[1:], " ", 3) if len(parts) < 2 { return fmt.Errorf("T-card needs name and uuid") } tc.Name = parts[0] tc.UUID = parts[1] if len(parts) == 3 { tc.Value = parts[2] } if n := len(d.T); n > 0 { if err := requireTagAscending(d.T[n-1], tc); err != nil { return err } } d.T = append(d.T, tc) return nil } // requireTagAscending enforces the two-level T rule of §4.5.2: ascending by // tag name, then by target hash, with equal names requiring a strictly // increasing hash. Per §4.7.16 the primary key is the decoded full name // token including its leading sign character; tagOrderKey builds it. func requireTagAscending(previous, current TagCard) error { previousName := tagOrderKey(previous) currentName := tagOrderKey(current) switch cmp := Compare(currentName, previousName); { case cmp > 0: return nil case cmp < 0: return fmt.Errorf("T-card %q out of order after %q", currentName, previousName) default: if Compare(current.UUID, previous.UUID) > 0 { return nil } return fmt.Errorf("T-card %q target %q out of order after %q", currentName, current.UUID, previous.UUID) } } // parseUCard resolves the U-card's decoded value the same way fossil's // own parser does (src/manifest.c:1008-1016): a present-but-empty U-card // becomes the literal "anonymous". A wholly absent U-card never calls // this function at all, so d.U stays nil — the third state a bare string // field cannot represent. Resolving here, once, means every downstream // crosslink call site can bind d.U directly and get the right SQL value // (NULL, "anonymous", or the login) without repeating this logic. // // args is trimmed before the emptiness check, matching canonical fossil's // next_token() (which skips leading whitespace) and every sibling card // parser in this file (B/G/I/K/M/N/R all TrimSpace); otherwise a // whitespace-only U-card ("U \n") would store literal whitespace instead // of resolving to "anonymous". func parseUCard(d *Deck, args string) error { user := FossilDecode(strings.TrimSpace(args)) if user == "" { user = "anonymous" } d.U = &user return nil } func parseTimestamp(s string) (time.Time, error) { s = strings.TrimSpace(s) for _, layout := range []string{ "2006-01-02T15:04:05.000", "2006-01-02T15:04:05", } { if t, err := time.Parse(layout, s); err == nil { return t, nil } } s = strings.Replace(s, "t", "T", 1) for _, layout := range []string{ "2006-01-02T15:04:05.000", "2006-01-02T15:04:05", } { if t, err := time.Parse(layout, s); err == nil { return t, nil } } return time.Time{}, fmt.Errorf("cannot parse timestamp %q", s) } func inferType(d *Deck) ArtifactType { switch { case len(d.M) > 0: return Cluster case d.G != "" || d.H != "" || d.I != "": return ForumPost case d.A != nil: return Attachment case d.K != "": return Ticket case d.L != "": return Wiki case d.E != nil: return Event case len(d.F) > 0 || d.R != "": return Checkin case len(d.T) > 0: return Control default: return Checkin } }