Skip to content

fix(adif): declare export field lengths as UTF-8 byte count - #228

Merged
patrickrb merged 1 commit into
mainfrom
optio/task-0e7bc752-d1fa-42c8-a65f-a2ad148b7329
Jul 23, 2026
Merged

fix(adif): declare export field lengths as UTF-8 byte count#228
patrickrb merged 1 commit into
mainfrom
optio/task-0e7bc752-d1fa-42c8-a65f-a2ad148b7329

Conversation

@patrickrb

Copy link
Copy Markdown
Owner

Problem

ADIF field headers (<name:length>value) must declare length as the number of UTF-8 octets in the data, per the ADIF spec. The exporter (generateADIF in src/app/api/adif/export/route.ts) computed length with JavaScript's String.length, which counts UTF-16 code units.

For plain ASCII the two agree, so this went unnoticed. But any operator whose data contains multi-byte characters got the wrong length declared:

Data Bytes (correct) .length (emitted)
José 5 <name:4>
München 8 <qth:7>
東京 6 <qth:2>
RN3Ø (slashed-zero callsign) 5 <call:4>

Strict ADIF readers — LoTW/TQSL, Cloudlog/Wavelog, N1MM — read exactly the declared number of bytes, so the under-count truncates the field or mis-aligns the parse of everything after it. This silently corrupts exports for non-English names, QTHs, and notes.

Solution

  • Extract the ADIF generator out of the route into src/lib/adif.ts as pure, testable generateAdif() and adifField() functions — mirroring the existing parseAdifRecords() there and its tests/adif-parse.spec.ts.
  • adifField() declares length via Buffer.byteLength(value, 'utf8') so it is byte-correct. It also centralizes the "omit empty/null/zero optional fields" gating the route did inline, so field selection and value transforms (callsign/mode/band/grid uppercasing, freq in MHz, etc.) are unchanged.
  • The export route now imports generateAdif / AdifExportContact and delegates. No API request/response shape changes; ASCII exports are byte-for-byte identical apart from the header's created_timestamp length, which was previously off-by-one (:15 for a 16-byte value) and is now correct.

Testing

  • Added tests/adif-generate.spec.ts (12 assertions across adifField and generateAdif), including a round-trip through parseAdifRecords for Jürgen / München to prove no truncation.
  • npm run typecheck — clean
  • npm run lint — clean
  • npm run build — compiled successfully
  • npx playwright test adif-generate adif-parse — 12 passed

Backwards compatibility

Fully preserved. Pure ASCII logs (the common case) export identically; only multi-byte fields change, and they change from broken to correct.

Future follow-up

The ADIF parser (parseSingleRecord) reads field data with [^<]* and clips by declared length using JS string length rather than reading exactly N UTF-8 bytes. It round-trips our own output fine, but a length-driven byte reader would be more robust against third-party files whose data contains a literal <. Out of scope here.

🤖 Generated with Claude Code

@vercel

vercel Bot commented Jul 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nodelog Ready Ready Preview, Comment Jul 23, 2026 7:34pm

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes ADIF export correctness for non-ASCII data by declaring field lengths as UTF-8 byte counts (per ADIF <name:length>value requirements) and refactors the exporter into a shared, testable library module.

Changes:

  • Extracts ADIF export generation into src/lib/adif.ts with adifField() (UTF-8 byte length) and generateAdif().
  • Updates the ADIF export API route to delegate to the shared generator without changing the API contract.
  • Adds unit-style Playwright tests validating UTF-8 byte-length headers and a parser round-trip for multi-byte operator/QTH data.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/adif-generate.spec.ts Adds coverage for UTF-8 byte-length field headers and generateAdif() output/round-trip behavior.
src/lib/adif.ts Introduces AdifExportContact, adifField() using UTF-8 byte length, and generateAdif() to build ADIF documents.
src/app/api/adif/export/route.ts Replaces inline exporter with generateAdif() from the shared ADIF library.

@patrickrb

Copy link
Copy Markdown
Owner Author

Review of PR #228 (approval blocked by GitHub: can't approve one's own PR under this account — posting findings as a comment instead)

Verified against the PR head commit directly (not just the diff).

Correctness

  • The genuine bug fix is real and correctly implemented: adifField() uses Buffer.byteLength(str, 'utf8') instead of String.length, fixing under-counted lengths for multi-byte data (accents, CJK, slashed-zero callsigns). Confirmed the math for the test cases (José → 5 bytes, 東京 → 6 bytes, RN3Ø → 5 bytes).
  • Also fixes a real off-by-one in the header's created_timestamp (previously declared :15 for a 16-byte value including the trailing Z) — now derived from the actual string via the same adifField helper.
  • Compared the SQL SELECT column list in the route against the new AdifExportContact interface field-by-field — they match exactly, no drift.
  • Verified the old generateADIF/Contact interface has no other callers anywhere in the codebase, so the extraction is safe.
  • The value === 0 omission in adifField intentionally preserves the original truthy-check gating behavior (e.g. if (contact.dxcc)), so this isn't a regression, just a preserved quirk.
  • No client components import @/lib/adif, so the added Buffer usage (Node-only) is safe — only used from API routes and tests.

Tests: New tests/adif-generate.spec.ts directly exercises the byte-length bug (ASCII vs multi-byte), header/required-field emission, optional-field omission, and a round-trip through parseAdifRecords. Good coverage, follows the existing adif-parse.spec.ts pattern.

Style/conventions: snake_case fields preserved per repo convention, no any introduced, no console usage, error handling untouched. Scope is appropriately tight — a real reliability bug fix with tests, no unrelated refactoring.

No issues found. This PR looks good to merge.

…ength

ADIF field headers must declare data length as the number of UTF-8 octets,
not UTF-16 code units. The exporter used String.length, so a value like
"José" (5 bytes) was written as <name:5>... only when ASCII — multi-byte
characters (accents, ø, CJK, emoji) under-counted, e.g. <name:4>José. Strict
ADIF readers (LoTW/TQSL, Cloudlog, N1MM) then truncate or mis-align the field.

Extract the generator into src/lib/adif.ts as pure, tested generateAdif() /
adifField() (mirrors the existing parseAdifRecords test pattern) and count
bytes via Buffer.byteLength. The export route now delegates to it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@patrickrb
patrickrb force-pushed the optio/task-0e7bc752-d1fa-42c8-a65f-a2ad148b7329 branch from bc3ae7d to 06d8bad Compare July 23, 2026 19:33
@patrickrb
patrickrb merged commit fa679e4 into main Jul 23, 2026
5 checks passed
@patrickrb
patrickrb deleted the optio/task-0e7bc752-d1fa-42c8-a65f-a2ad148b7329 branch July 23, 2026 19:59
patrickrb added a commit that referenced this pull request Jul 23, 2026
…rator (#230)

The /api/contacts/search?export=true path hand-rolled its own ADIF
serializer instead of reusing generateAdif(). That copy carried three
defects the main export had already shed:

- Declared field lengths with JS String.length (UTF-16 code units)
  rather than the UTF-8 byte count ADIF requires — the interop bug fixed
  for the main export in #228. Accented names, ø-callsigns and CJK QTHs
  came out with wrong lengths and got truncated/mis-aligned by strict
  readers (LoTW/TQSL, Cloudlog, N1MM).
- Called .toString()/.length on frequency/mode/band unconditionally, so
  a single contact imported without one of those fields 500'd the whole
  export.
- Emitted only ~11 fields, silently dropping DXCC, QSL status, country,
  zones and station info.

Delegating to generateAdif() fixes all three and keeps the two export
paths in lockstep.

Co-authored-by: Optio Agent <optio-agent@noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants