Skip to content

Add lossless registry-backed vCard codec - #533

Open
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:vcard-registry-codec
Open

Add lossless registry-backed vCard codec#533
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:vcard-registry-codec

Conversation

@salmonumbrella

@salmonumbrella salmonumbrella commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Vendors the current IANA vCard property and parameter registries with explicit handling declarations and drift/update commands.
  • Adds bounded, ordered decode and render support for vCard 2.1, 3.0, and 4.0 while preserving groups, source spelling, parameter order and quoting, raw values, repeated properties, and unknown extensions.
  • Adds reusable text, structured/list, RFC 6868, quoted-printable, partial-date, registry lookup, and semantic validation helpers.
  • Keeps the existing Contact and ParseFile API as a compatible projection over the lossless codec.

Why

Richer person profiles and CardDAV need a standards boundary that can preserve source syntax and data Msgvault does not yet understand. The previous contact projection discarded that information, making future native mapping and round trips lossy.

This is the first focused step in #534. Native profile mapping, resource persistence, and CardDAV serving remain separate follow-up boundaries.

Usage

Run make vcard-registry-check to report upstream registry drift. Run make vcard-registry-update to fetch and atomically write a validated snapshot for review.

@roborev-ci

roborev-ci Bot commented Jul 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (2aca272)

Medium-severity vCard compatibility issues should be addressed before merging.

Medium

  • internal/vcard/legacy.go:58 — Parameterized tel: URIs are normalized incorrectly

    Valid URI parameters are included in digit-only normalization. For example, tel:+1-202-555-0123;ext=456 becomes +12025550123456, while phone-context values may be rejected entirely.

    Fix: Parse TEL URIs and normalize only the telephone-number component, handling extensions and phone-context separately. Add coverage for parameterized TEL URIs.

  • internal/vcard/decode.go:105 — Valid legacy-character-set vCards are rejected or decoded incorrectly

    Requiring every physical line to be UTF-8 rejects valid vCard 2.1 files declaring character sets such as ISO-8859-1. Quoted-printable values can pass this check but are decoded without applying CHARSET, potentially yielding invalid UTF-8 contact names.

    Fix: Honor CHARSET when decoding legacy values and defer validation/transcoding until after transfer decoding. Add literal and quoted-printable non-UTF-8 fixtures.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 8m57s

@salmonumbrella

Copy link
Copy Markdown
Contributor Author

Fixed in cb3df06.

  • TEL URI projection now excludes ext from E.164 identity keys and resolves local subscribers with numeric phone-context separately.
  • Legacy text decoding now applies quoted-printable transfer decoding before the declared CHARSET conversion; literal and quoted-printable ISO-8859-1 vCard 2.1 values are covered.
  • Undeclared invalid UTF-8 remains rejected.

The PR remains one commit.

@roborev-ci

roborev-ci Bot commented Jul 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (cb3df06)

Medium-severity issues remain in the vCard codec and parser.

Medium

  • internal/vcard/encode.go:215 — Quoted-printable folding can split an =XX escape immediately after =. During decoding, unfoldLines treats the trailing = as a soft break and removes it, corrupting marshal/decode round trips. Make folding quoted-printable-aware and add a boundary regression test.

  • internal/vcard/encode.go:201, internal/vcard/decode.go:165 — The decoder accepts non-UTF-8 values with a legacy CHARSET, but the encoder unconditionally rejects the resulting document. Valid vCard 2.1 data such as raw ISO-8859-1 cannot round-trip through the lossless codec. Preserve charset-tagged bytes safely or normalize the value and charset metadata before encoding, with legacy-charset round-trip coverage.

  • internal/vcard/contentline.go:84 — Parameter-value parsing permits severe memory amplification. A content line near the 16 MiB limit containing millions of comma-separated values can cause splitOutsideQuotes and ParameterValue allocations to consume roughly a gigabyte of memory. Enforce parameter and value-count limits before allocating, preferably with a capped splitter, and consider a total decoded-property/input budget.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 8m26s

@salmonumbrella

Copy link
Copy Markdown
Contributor Author

Fixed in 600646e.

  • Quoted-printable folding now chooses boundaries outside =XX escapes, preventing trailing = from being consumed as a soft break.
  • Charset-declared legacy bytes accepted by the decoder are preserved by the lossless encoder and can round-trip through folding.
  • Quote-aware parsing now caps each property at 100 parameters and each parameter at 100 values before allocation amplification; boundary tests cover 100 and 101.

Focused and repository-wide tests, lint, vet, and testify checks pass. The PR remains one commit.

@roborev-ci

roborev-ci Bot commented Jul 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (600646e)

Medium-severity performance and resource-exhaustion risk found in the vCard decoder.

Medium

  • internal/vcard/decode.go:40 — The decoder buffers the entire input before enforcing MaxCards, allowing oversized documents to consume unbounded memory before rejection. Additionally, repeated string concatenation for folded lines at line 144 can cause quadratic processing time on heavily folded input.
    • Suggested fix: Stream physical lines through unfolding and card parsing, use a string builder for continuations, and enforce card or total-input limits while reading.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 12m54s

@salmonumbrella

Copy link
Copy Markdown
Contributor Author

Fixed in 6e218d6.

  • Decoding now streams physical lines through unfolding and card parsing, so card and logical-line limits stop reads before trailing input is consumed.
  • Folded continuations use amortized byte-buffer growth, and quoted-printable header state is cached instead of repeatedly rescanning a growing line.
  • Added early-stop regressions for card/logical limits and allocation coverage for heavily folded input.

The PR remains one commit.

@roborev-ci

roborev-ci Bot commented Jul 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (6e218d6)

Medium-severity version handling issue found; no material security vulnerabilities identified.

Medium

  • RFC 6868 decoding applied to pre-4.0 vCardsinternal/vcard/contentline.go:79,139
    Caret decoding is unconditional, although RFC 6868 applies only to vCard 4.0. In vCard 2.1/3.0, literal sequences such as ^n therefore produce an incorrect Decoded value. Decode and encode parameter values according to the containing card’s version, and add tests covering literal carets in vCard 2.1/3.0 parameters.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 10m47s

@salmonumbrella

Copy link
Copy Markdown
Contributor Author

Fixed in c50f6e9. RFC 6868 parameter escaping now follows the containing card version: vCard 2.1/3.0 retain literal caret sequences, while 4.0 continues to decode and encode RFC 6868. Added 2.1/3.0 decode and constructed-parameter regression coverage.

@roborev-ci

roborev-ci Bot commented Jul 30, 2026

Copy link
Copy Markdown

roborev: Combined Review (c50f6e9)

Code review passed: no Medium, High, or Critical findings.


Reviewers: 2 done | Synthesis: codex, 6s | Total: 9m39s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant