Skip to content

Fix ADIF import truncating field values that contain '>'#648

Merged
patrickrb merged 2 commits into
devfrom
optio/task-49b586b6-fc8d-47e2-9a0c-e450e5e52057
Jul 23, 2026
Merged

Fix ADIF import truncating field values that contain '>'#648
patrickrb merged 2 commits into
devfrom
optio/task-49b586b6-fc8d-47e2-9a0c-e450e5e52057

Conversation

@patrickrb

Copy link
Copy Markdown
Owner

Summary

Importing an ADIF log silently truncated any field value that contains a > character, corrupting logged QSO data. A COMMENT of hello>world was imported as hello.

Both ADIF import parsers were affected (same root cause):

  • LogFileImport.getLogRecords — the built-in web-logger HTTP-upload path (LogHttpServernew LogFileImport).
  • ImportSharedLogs.parseLogRecords — the VIEW/SEND intent path that lets other apps hand a .adi file to FT8AF.

Root cause

An ADIF field is <NAME:LEN[:TYPE]>VALUE, where LEN is the authoritative byte length of the value, and the value may legally contain > (free-text fields such as COMMENT/NOTES/QSLMSG).

Both parsers split each field on every >:

String[] values = field.split(">");
...
String value = extractFieldValue(values[1], valueLen);

so values[1] is only the text up to the second > — everything from the first interior > onward was discarded, even though the declared length said to keep it. This also contradicted extractFieldValue's own documented contract ("everything after the first >").

Fix

Split each field only at the first > (via indexOf) into header + raw value, then slice the raw value to the already-parsed declared length:

int gt = field.indexOf('>');
String header   = field.substring(0, gt);   // NAME:LEN[:TYPE]
String rawValue = field.substring(gt + 1);  // may contain '>'
...
String value = extractFieldValue(rawValue, valueLen);

Values without > decode byte-identically, so well-formed logs are unaffected. Field-length clamping, the MAX_ADIF_FIELD_LEN cap (shared parser), the type-qualified <NAME:LEN:TYPE> form, and the per-record error tracking (web-logger) all keep their existing behavior.

To make the web-logger parser unit-testable (its constructor reads a file), the per-record field parsing was extracted into a static, Android-free helper LogFileImport.parseRecord, per the project's testing convention.

Testing

./gradlew :app:testDebugUnitTest — full suite green. ./gradlew :app:assembleDebug — builds all ABIs.

New / updated tests:

  • ImportSharedLogsTest: fieldValueContainingGreaterThan_isPreserved, fieldValueWithMultipleGreaterThan_isClampedToDeclaredLength (both fail before the fix, returning the truncated hello / a); updated the stray-> edge case (<call:3>>X>X) to the corrected length-based slice.
  • LogFileImportTest: fieldValueContainingGreaterThan_isPreservedEndToEnd (end-to-end through the file-reading constructor) plus pure-JVM parseRecord_* cases covering the fix, declared-length clamping, type-qualified headers, and the non-numeric-length error path.

Risk

Low. Pure Java in the ADIF import path only; no native/DSP, decoder, waterfall, CAT, or protocol code touched. Behavior changes only for malformed/>-containing field values, which were previously silently corrupted.

The web-logger upload parser (LogFileImport.getLogRecords) and the shared-log
import parser (ImportSharedLogs.parseLogRecords) split each ADIF field on
EVERY '>' and kept the token before the second one. An ADIF field is
<NAME:LEN[:TYPE]>VALUE where LEN is the authoritative byte length, so a value
may legally contain '>' (free-text COMMENT/NOTES/QSLMSG and similar). Such a
value was silently truncated at its first interior '>' on import -- e.g. a
comment "hello>world" became "hello", corrupting logged QSO data.

Root cause: the value slice used field.split(">")[1] instead of honouring the
already-parsed declared length. Fix both parsers to split only at the FIRST '>'
(indexOf) into header + raw value, then slice the raw value to the declared
length -- matching extractFieldValue's documented contract ("everything after
the first '>'"). Valid values without '>' decode identically.

Extracted LogFileImport.parseRecord as a static, Android-free helper so the
field parsing is unit-testable per the project's testing convention. Added
regression tests to both ImportSharedLogsTest and LogFileImportTest (pure-JVM
helpers plus an end-to-end file import); updated the stray-'>' edge case to the
corrected length-based slice.

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 (55de7f4) to head (972c2e8).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #648      +/-   ##
============================================
+ Coverage     36.73%   36.79%   +0.06%     
- Complexity      197      207      +10     
============================================
  Files           216      218       +2     
  Lines         26888    26994     +106     
  Branches       3287     3311      +24     
============================================
+ Hits           9877     9933      +56     
- Misses        16785    16829      +44     
- Partials        226      232       +6     
Flag Coverage Δ
android 15.29% <ø> (+0.25%) ⬆️
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

Fixes ADIF import truncation when a field value legitimately contains > by splitting each field only at the first > and then slicing the value to the declared length. This targets both ADIF import entry points (web-logger upload and shared-file import) and adds regression/unit tests, including a new Android-free per-record parsing helper for LogFileImport.

Changes:

  • Updated both ADIF import parsers to split field header/value at the first > to preserve > inside values.
  • Extracted LogFileImport.parseRecord(...) for unit-testing record parsing without the file-reading constructor.
  • Added/updated unit tests covering >-containing values and multi-> clamping behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
ft8af/app/src/main/java/com/k1af/ft8af/log/ImportSharedLogs.java Fixes field parsing to split at the first > so > can exist inside values.
ft8af/app/src/main/java/com/k1af/ft8af/log/LogFileImport.java Refactors field parsing into a static helper and applies the same first-> split fix in the web-upload import path.
ft8af/app/src/test/java/com/k1af/ft8af/log/ImportSharedLogsTest.java Adds regression tests ensuring > inside values is preserved and later fields aren’t corrupted.
ft8af/app/src/test/java/com/k1af/ft8af/log/LogFileImportTest.java Adds end-to-end and helper-level tests for the > regression and record parsing behavior.

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

Comment thread ft8af/app/src/main/java/com/k1af/ft8af/log/LogFileImport.java
Comment thread ft8af/app/src/main/java/com/k1af/ft8af/log/ImportSharedLogs.java
…Locale.US

Copilot review: ADIF <field:len> declares a UTF-8 byte count (what this app's
own export writes via AdifFormat.utf8Length), but both importers sliced by
UTF-16 chars — over-reading past non-ASCII values into the following
inter-field whitespace. Add AdifFormat.sliceByUtf8Length (never splits a code
point, clamps to available text) and use it in LogFileImport.parseRecord and
ImportSharedLogs.extractFieldValue. Field-name keys now uppercase with
Locale.US so Turkish-style locales can't turn GRIDSQUARE into GRİDSQUARE.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@patrickrb
patrickrb merged commit ac169cc into dev Jul 23, 2026
17 checks passed
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