Skip to content

Fix web-logbook ADIF export using UTF-16 char length instead of UTF-8 byte length#642

Merged
patrickrb merged 2 commits into
devfrom
optio/task-7f1aa191-26bd-42dc-bf05-53be5620a9bb
Jul 23, 2026
Merged

Fix web-logbook ADIF export using UTF-16 char length instead of UTF-8 byte length#642
patrickrb merged 2 commits into
devfrom
optio/task-7f1aa191-26bd-42dc-bf05-53be5620a9bb

Conversation

@patrickrb

Copy link
Copy Markdown
Owner

Summary

DatabaseOpr.downQSLTable — the ADIF generator behind the built-in web-logbook ADIF download (LogHttpServer, 4 call sites) — declared every <field:len> length using Java String.length() (a UTF-16 char count) instead of the UTF-8 byte count the ADIF grammar requires.

For any non-ASCII field value the declared length is too short, so a byte-correct ADIF consumer (LoTW, QRZ, Cloudlog, WSJT-X) reads fewer bytes than were actually written — truncating the field and mis-aligning everything after it, including the <eor> record boundary. The record is rejected or silently mangled.

Root cause

This method is the un-migrated twin of the canonical ADIF writer. The repo already diagnosed and fixed this exact class of bug: AdifRecord.build() and the ThirdPartyService upload paths route every field length through AdifFormat.utf8Length(), whose javadoc spells out the failure mode ("declaring the shorter char count makes the receiving parser read fewer bytes than were written, truncating the field and mis-aligning everything after it, so LoTW/QRZ/Cloudlog reject or mangle the record"). downQSLTable was never converted, so the web-logbook download regressed the very bug fixed elsewhere.

The file-export UI path (ExportLogSheetShareLogsAdifRecord) is already correct; only this web-download path was affected.

Fix

  • Route downQSLTable and its appendPotaField helper through the shared AdifFormat.utf8Length(). It has an ASCII fast-path, so pure-ASCII rows stay byte-identical and there is no measurable cost.
  • Null-guard the comment field (mirroring AdifRecord.build()'s comment == null ? ""). The old comment.length() dereference threw an NPE on a NULL comment, aborting the entire web-logbook download and leaking the cursor.

Reachability

A foreign ADIF containing a non-ASCII COMMENT (e.g. Café QSO) uploaded to the built-in web logbook is stored verbatim (LogHttpServer.doImportADILogFileImportQSLRecorddoInsertQSLData). Downloading the logbook as ADIF then emits <comment:8>Café QSO <eor> (should be :9), truncating the trailing O and corrupting the record boundary.

Concrete example

comment = "Café QSO" → 8 UTF-16 chars, 9 UTF-8 bytes (é = 2 bytes).

  • Before: <comment:8>Café QSO <eor> → consumer reads Café QS, drops O, <eor> desynced.
  • After: <comment:9>Café QSO <eor> → correct.

Testing

  • New DatabaseOprAdifLengthTest (Robolectric + MatrixCursor, mirroring the existing DatabaseOprAdifModeTest): non-ASCII comment / operator / POTA SIG_INFO each declare the UTF-8 byte length; ASCII stays char-length (byte-identical); a NULL comment no longer throws. 4 of 5 cases fail before the fix (3 assertion failures + 1 NPE), all pass after.
  • Full :app:testDebugUnitTest suite green.

Risk

Low. The change only affects declared field lengths and is byte-identical for ASCII content (the common case); it aligns this path with the already-shipped AdifFormat.utf8Length convention used by every other ADIF writer. No protocol/DSP behavior changes.

…TF-8 byte length

DatabaseOpr.downQSLTable — the ADIF generator behind the built-in web
logbook download (LogHttpServer) — declared every <field:len> length with
Java String.length() (a UTF-16 char count) instead of the UTF-8 byte count
the ADIF grammar requires. For any non-ASCII field (an accented comment, an
international POTA park name), the declared length is short, so a byte-correct
consumer (LoTW/QRZ/Cloudlog/WSJT-X) reads fewer bytes than were written,
truncating the field and mis-aligning the record boundary — the record is
rejected or mangled.

This is the un-migrated twin of the canonical writer: AdifRecord.build() and
the ThirdPartyService upload paths already route every length through
AdifFormat.utf8Length() (whose javadoc documents exactly this defect). Route
downQSLTable and its appendPotaField helper through the same shared helper.
utf8Length has an ASCII fast-path, so pure-ASCII rows stay byte-identical and
there is no measurable cost.

Also null-guard the comment field (mirroring AdifRecord.build's
`comment == null ? ""`): the old comment.length() dereference NPE'd on a NULL
comment, aborting the entire web-logbook download and leaking the cursor.

Added DatabaseOprAdifLengthTest (Robolectric + MatrixCursor, mirroring
DatabaseOprAdifModeTest): non-ASCII comment/operator/POTA sig_info each declare
the UTF-8 byte length, ASCII stays char-length (byte-identical), and a NULL
comment no longer throws. 4 of the 5 cases fail before the fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 36.79%. Comparing base (5f02472) to head (44c96e1).
⚠️ Report is 8 commits behind head on dev.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #642      +/-   ##
============================================
+ Coverage     36.71%   36.79%   +0.08%     
- Complexity      197      207      +10     
============================================
  Files           216      218       +2     
  Lines         26902    26994      +92     
  Branches       3289     3311      +22     
============================================
+ Hits           9877     9933      +56     
- Misses        16799    16829      +30     
- Partials        226      232       +6     
Flag Coverage Δ
android 15.29% <ø> (+0.27%) ⬆️
native 9.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes ADIF export correctness for the built-in web logbook download path (DatabaseOpr.downQSLTable) by declaring ADIF <field:len> values as UTF-8 byte lengths (per ADIF grammar) rather than UTF-16 String.length() character counts, and by null-guarding comment to avoid aborting exports.

Changes:

  • Update DatabaseOpr.downQSLTable and its POTA helper to compute <field:len> via AdifFormat.utf8Length(...) (UTF-8 byte count) and avoid comment NPEs.
  • Add Robolectric unit tests covering non-ASCII field values (and NULL comment) for the web-logbook ADIF export.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
ft8af/app/src/main/java/com/k1af/ft8af/database/DatabaseOpr.java Switch ADIF length declarations to UTF-8 byte lengths and null-guard comment during web-logbook ADIF generation.
ft8af/app/src/test/java/com/k1af/ft8af/database/DatabaseOprAdifLengthTest.java Add tests verifying UTF-8 byte-length declarations for non-ASCII fields and confirming NULL comment no longer throws.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ft8af/app/src/main/java/com/k1af/ft8af/database/DatabaseOpr.java Outdated
…aders ASCII

Copilot review: %d under a default locale with Arabic-Indic numerals emitted
non-ASCII digits in the length headers. Align with AdifRecord.build()'s
Locale.US usage; drop the now-unneeded DefaultLocale lint suppression.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@patrickrb
patrickrb merged commit 7d266d6 into dev Jul 23, 2026
17 checks passed
@patrickrb
patrickrb deleted the optio/task-7f1aa191-26bd-42dc-bf05-53be5620a9bb branch July 23, 2026 13:12
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