Artifact 8fe794f2049043c181cff0e6cde2a00eff9abc97f3490a4d06d3904a8fe0ffb2:
- File
internal/deck/cardcodec_test.go
— part of check-in
[0e579e24a4]
at
2026-07-22 23:03:29
on branch trunk
— fix: J/T card escape-encoding symmetry and T-card §4.7.16 validation (#90, #91, #93) (#116)
* fix: J card decodes value, stores name verbatim (#90)
The J-card parser had the escape direction inverted: it decoded the field name and exposed the value with its escape sequences intact. Both the spec (4.7.8) and canonical fossil (manifest.c) do the opposite -- defossilize only the value, compare the raw name.
Swap both the parse and marshal directions so the value round-trips decoded and the name stays verbatim.
* fix: T card escape-encodes the tag name on marshal (#91)
A tag/branch name containing a space was written verbatim, so 'my branch' produced 'T *sym-my branch *' -- a structurally valid but silently wrong card that re-parses as two tokens.
Align with canonical fossil: escape-encode the name on marshal and escape-decode it on parse, leaving the target token verbatim in both directions (4.7.16). TagCard.Name now holds the decoded form, so tagOrderKey reattaches the sign without re-decoding.
* fix: enforce two skipped T-card §4.7.16 checks on parse (#93)
parseTCard validated the sign and ordering key but accepted two malformed shapes from untrusted peers:
- a name that after the sign is entirely hex digits, indistinguishable from a hash target; - a target that is neither a valid artifact hash nor the literal '*'.
Reject both with errors (never a panic). hash.IsValidHash reuses the same target check the primary uuid path uses; the '*' and non-empty guards run first since it panics on an empty string.
* fix: T card decodes and encodes the tag value (#91), guard hex names on marshal (#93)
§4.7.16 applies the same escape rule to all three T-card tokens: the name and value are escape-decoded on parse and escape-encoded on marshal, while the target is stored verbatim. Only the name was handled; the value was passed through raw in both directions.
The read side was the real damage. `fossil branch new 'my branch'` writes
T *branch * my\sbranch T *sym-my\sbranch *
so importing an ordinary fossil repo with a spaced branch name produced a branch literally named "my\sbranch" through crosslink. Verified against the fossil binary: the artifact for that command now parses to value "my branch" and re-marshals byte-for-byte identical.
Marshal also gains the mirror of the hex-only tag name check parse already enforces. Without it a caller could build and store an artifact this repository could never read back; canonical fossil never emits one either, since it prefixes branch and user tags with "sym-". (user: [email protected], size: 7582) [annotate] [blame] [check-ins using]
package deck import ( "crypto/md5" "fmt" "strings" "testing" ) // withZCard appends the §4.7.19 Z checksum card to a manifest body so Parse // accepts it. The cards in body must already appear in ascending-letter // order, which Parse enforces. func withZCard(body string) []byte { sum := md5.Sum([]byte(body)) return []byte(fmt.Sprintf("%sZ %x\n", body, sum)) } // --- #90: J-card decode direction (§4.7.8) --- // TestParseJCardDecodesValueKeepsNameVerbatim pins §4.7.8: the J-card value // is escape-decoded while the field name is stored verbatim, matching // canonical manifest.c which defossilizes only zValue and compares the raw // zName. A value carrying escaped spaces must arrive decoded; a name // carrying an escape must arrive unchanged. func TestParseJCardDecodesValueKeepsNameVerbatim(t *testing.T) { d := &Deck{} if err := parseJCard(d, `a\sname a\svalue`); err != nil { t.Fatalf("parseJCard: %v", err) } if len(d.J) != 1 { t.Fatalf("J count = %d, want 1", len(d.J)) } if got := d.J[0].Name; got != `a\sname` { t.Errorf("Name = %q, want verbatim %q", got, `a\sname`) } if got := d.J[0].Value; got != "a value" { t.Errorf("Value = %q, want decoded %q", got, "a value") } } // TestMarshalJCardEncodesValueKeepsNameVerbatim is the write-side mirror of // the decode direction: the value is escape-encoded on the way out and the // name is written verbatim, so a decoded value with spaces reproduces its // wire escapes. func TestMarshalJCardEncodesValueKeepsNameVerbatim(t *testing.T) { var b strings.Builder d := &Deck{J: []TicketField{{Name: "comment", Value: "a value with spaces"}}} marshalCards(&b, d) if !strings.Contains(b.String(), "J comment a\\svalue\\swith\\sspaces\n") { t.Fatalf("J value not escape-encoded on marshal:\n%s", b.String()) } } // TestJCardWireRoundTrip pins that Marshal reproduces the original wire // bytes for an unmodified J-card whose value contains spaces (§4.7.8). func TestJCardWireRoundTrip(t *testing.T) { wire := withZCard("D 2024-01-15T10:30:00.000\nJ comment a\\svalue\\swith\\sspaces\n") d, err := Parse(wire) if err != nil { t.Fatalf("Parse: %v", err) } if got := d.J[0].Value; got != "a value with spaces" { t.Fatalf("Value = %q, want decoded", got) } out, _ := d.Marshal() if !strings.Contains(string(out), "J comment a\\svalue\\swith\\sspaces\n") { t.Fatalf("J line not reproduced on round-trip:\n%s", out) } } // --- #91: T-card marshal escape-encodes the tag name (§4.7.16) --- // TestTCardNameWithSpaceRoundTrips pins that a tag name containing a space // round-trips Marshal -> Parse unchanged. Marshal must escape-encode the // name so "sym-my branch" reaches the wire as "sym-my\sbranch" and parses // back to a single name token, not two. func TestTCardNameWithSpaceRoundTrips(t *testing.T) { d := &Deck{ T: []TagCard{{Type: TagPropagating, Name: "sym-my branch", UUID: "*"}}, } out, _ := d.Marshal() if !strings.Contains(string(out), "T *sym-my\\sbranch *\n") { t.Fatalf("tag name not escape-encoded on marshal:\n%s", out) } parsed, err := Parse(out) if err != nil { t.Fatalf("Parse: %v", err) } if len(parsed.T) != 1 { t.Fatalf("T count = %d, want 1", len(parsed.T)) } if got := parsed.T[0].Name; got != "sym-my branch" { t.Errorf("Name = %q, want %q", got, "sym-my branch") } if got := parsed.T[0].UUID; got != "*" { t.Errorf("UUID = %q, want %q", got, "*") } } // TestTCardTargetHashStaysUnencoded pins §4.7.16: the target token is never // decoded on the way in, so it must not be encoded on the way out. func TestTCardTargetHashStaysUnencoded(t *testing.T) { target := strings.Repeat("a", 40) var b strings.Builder d := &Deck{T: []TagCard{{Type: TagSingleton, Name: "sym-x", UUID: target}}} marshalCards(&b, d) if !strings.Contains(b.String(), "T +sym-x "+target+"\n") { t.Fatalf("target hash was altered on marshal:\n%s", b.String()) } } // TestTCardValueWithSpaceRoundTrips pins §4.7.16 for the third T-card token: // the value is escape-decoded on parse and escape-encoded on marshal, the // same rule the name follows. The wire bytes asserted here are the bytes // real fossil writes for `fossil branch new 'my branch'`: // // T *branch * my\sbranch // T *sym-my\sbranch * // // Leaving the value raw made an imported branch literally named // "my\sbranch" all the way through crosslink. func TestTCardValueWithSpaceRoundTrips(t *testing.T) { wire := withZCard("D 2024-01-15T10:30:00.000\nT *branch * my\\sbranch\n") d, err := Parse(wire) if err != nil { t.Fatalf("Parse: %v", err) } if got := d.T[0].Value; got != "my branch" { t.Fatalf("Value = %q, want decoded %q", got, "my branch") } out, err := d.Marshal() if err != nil { t.Fatalf("Marshal: %v", err) } if !strings.Contains(string(out), "T *branch * my\\sbranch\n") { t.Fatalf("T value not escape-encoded on marshal:\n%s", out) } } // TestTCardEmptyValueOmitted guards the optional-value case: a T-card with no // value token must not gain one, and decoding must not turn an absent value // into a present-but-empty one. func TestTCardEmptyValueOmitted(t *testing.T) { d := &Deck{T: []TagCard{{Type: TagPropagating, Name: "sym-x", UUID: "*"}}} out, err := d.Marshal() if err != nil { t.Fatalf("Marshal: %v", err) } if !strings.Contains(string(out), "T *sym-x *\n") { t.Fatalf("valueless T-card not reproduced:\n%s", out) } parsed, err := Parse(out) if err != nil { t.Fatalf("Parse: %v", err) } if got := parsed.T[0].Value; got != "" { t.Errorf("Value = %q, want empty", got) } } // --- #93: parseTCard §4.7.16 validation --- // TestParseTCardRejectsHexOnlyName pins that a T-card name which after the // sign character is entirely hex digits is rejected, because it cannot be // told apart from a hash target (§4.7.16). func TestParseTCardRejectsHexOnlyName(t *testing.T) { hexName := strings.Repeat("a", 40) wire := withZCard("D 2024-01-15T10:30:00.000\nT +" + hexName + " *\n") if _, err := Parse(wire); err == nil { t.Fatalf("Parse accepted a hex-only tag name %q, want error", hexName) } } // TestMarshalTCardRejectsHexOnlyName is the write-side mirror: Marshal must // refuse to emit a tag name the parser rejects, otherwise a caller can store // an artifact this repository can never read back. Canonical fossil never // produces one either -- it prefixes branch and user tags with "sym-". func TestMarshalTCardRejectsHexOnlyName(t *testing.T) { d := &Deck{T: []TagCard{{Type: TagSingleton, Name: "cafe", UUID: "*"}}} if _, err := d.Marshal(); err == nil { t.Fatalf("Marshal accepted a hex-only tag name, want error") } } // TestParseTCardRejectsInvalidTarget pins that a target which is neither a // valid artifact hash nor the literal '*' is rejected (§4.7.16). func TestParseTCardRejectsInvalidTarget(t *testing.T) { wire := withZCard("D 2024-01-15T10:30:00.000\nT +sym-x notahash\n") if _, err := Parse(wire); err == nil { t.Fatalf("Parse accepted an invalid T-card target, want error") } } // TestParseTCardAcceptsValidTargets guards against over-rejection: a real // 40-hex target and the literal '*' must both still parse (§4.7.16). func TestParseTCardAcceptsValidTargets(t *testing.T) { target := strings.Repeat("b", 40) wire := withZCard("D 2024-01-15T10:30:00.000\nT +sym-x " + target + "\n") if _, err := Parse(wire); err != nil { t.Fatalf("Parse rejected a valid 40-hex target: %v", err) } wireStar := withZCard("D 2024-01-15T10:30:00.000\nT +sym-x *\n") if _, err := Parse(wireStar); err != nil { t.Fatalf("Parse rejected the literal '*' target: %v", err) } }