Read bundled data assets fully so a short read can't corrupt the callsign/DXCC/zone databases - #621
Merged
Conversation
The bundled reference-data assets were read with `new byte[inputStream.available()]` followed by a single `inputStream.read(bytes)` whose return value was ignored. Neither is reliable: `available()` is only a hint, and one `read(byte[])` is explicitly permitted to return fewer bytes than requested. Android's `AssetInputStream` decompresses on the fly, so a large compressed asset is delivered in chunks and the lone read keeps only the first chunk, leaving the rest of the buffer as NUL bytes. Impact: - cty.dat (~280 KB) — the callsign->country/CQ-zone/ITU-zone map — is silently truncated, so callsigns past the first chunk resolve to the wrong (or no) country/zone in the decode list, on the map, and in ADIF export. - ituzone.json / cqzone.json / dxcc_list.json (450-740 KB) are parsed as JSON right after the read; a truncated buffer makes `new JSONObject(...)` throw, wiping the entire zone/DXCC table. This is the same class of bug the team already fixed for log import in `LogFileImport.readFully`; these were the remaining copies of the pattern. Added `Streams.readAllBytes(InputStream)`, which drains the stream to EOF in a loop, and routed all eight sites through it (CallsignFileOperation, RigNameList, OperationBand, DatabaseOpr x3, HelpDialog, ClearCacheDataDialog). Behavior is otherwise unchanged (same default-charset decoding). Tests: new StreamsTest covers full-drain of a stream that short-reads one byte per call and one that under-reports available(); a new CallsignFileOperationTest case feeds a chunked stream and asserts every record survives (fails against the old single-read, passes now). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a long-standing asset-loading bug where bundled reference-data files could be silently truncated due to unsafe available() sizing and single-call read(byte[]) usage, leading to corrupted callsign/DXCC/zone data at runtime.
Changes:
- Added
com.k1af.ft8af.util.Streams.readAllBytes(InputStream)to drain streams to EOF reliably. - Routed multiple asset-loading call sites to use
Streams.readAllBytes. - Added/extended pure-JVM unit tests to regress short-read / under-reported-available scenarios.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ft8af/app/src/main/java/com/k1af/ft8af/util/Streams.java | New helper to fully read InputStream contents to EOF. |
| ft8af/app/src/main/java/com/k1af/ft8af/callsign/CallsignFileOperation.java | Uses Streams.readAllBytes when splitting cty.dat content. |
| ft8af/app/src/main/java/com/k1af/ft8af/database/DatabaseOpr.java | Uses Streams.readAllBytes for DXCC/CQ/ITU JSON asset reads. |
| ft8af/app/src/main/java/com/k1af/ft8af/database/RigNameList.java | Uses Streams.readAllBytes for rigaddress.txt parsing helper. |
| ft8af/app/src/main/java/com/k1af/ft8af/database/OperationBand.java | Uses Streams.readAllBytes for bands.txt parsing helper. |
| ft8af/app/src/main/java/com/k1af/ft8af/ui/HelpDialog.java | Uses Streams.readAllBytes when reading help text assets. |
| ft8af/app/src/main/java/com/k1af/ft8af/ui/ClearCacheDataDialog.java | Uses Streams.readAllBytes when reading dialog/help assets. |
| ft8af/app/src/test/java/com/k1af/ft8af/util/StreamsTest.java | New unit tests covering short reads and lying available(). |
| ft8af/app/src/test/java/com/k1af/ft8af/callsign/CallsignFileOperationTest.java | New regression test ensuring chunked reads don’t truncate semicolon records. |
Comments suppressed due to low confidence (3)
ft8af/app/src/main/java/com/k1af/ft8af/database/RigNameList.java:106
- getLinesFromInputStream returns null on IOException, but callers (e.g., getRigNamesFromFile) assume a non-null array and will crash with NullPointerException. Prefer returning an empty array (and logging) or throwing a checked/unchecked exception with the cause so failures don’t turn into NPEs.
byte[] bytes = Streams.readAllBytes(inputStream);
return (new String(bytes)).split(deLimited);
}catch (IOException e){
return null;
}
ft8af/app/src/main/java/com/k1af/ft8af/database/OperationBand.java:373
- getLinesFromInputStream returns null on IOException, but getBandsFromFile passes the result to parseBandLines without a null check, which will crash with NullPointerException. Prefer returning an empty array (and logging) or throwing an exception with the cause.
byte[] bytes = Streams.readAllBytes(inputStream);
return (new String(bytes)).split(deLimited);
}catch (IOException e){
return null;
}
ft8af/app/src/main/java/com/k1af/ft8af/callsign/CallsignFileOperation.java:62
- getLinesFromInputStream returns null on IOException, but getCallSingInfoFromFile assumes a non-null array (uses st.length) and will crash with NullPointerException if an asset read fails. Prefer returning an empty array (and logging) or throwing an exception with the original cause.
byte[] bytes = Streams.readAllBytes(inputStream);
return (new String(bytes)).split(deLimited);
}catch (IOException e){
return null;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Streams.readAllBytes can throw part-way through a read, and the manual close() after it was skipped on that path, leaking the AssetInputStream. Same fix in both help/clear-cache dialogs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
The bundled reference-data assets are read with:
Both halves are unsafe:
InputStream.available()is only a hint, not the length.read(byte[])is explicitly permitted to return fewer bytes than requested.Android's
AssetInputStreamdecompresses assets on the fly, so a large compressed asset is handed back in chunks. The lonereadkeeps only the first chunk; the tail of the buffer stays as NUL bytes, silently truncating the file.Impact
cty.dat(~280 KB) — the callsign→country / CQ-zone / ITU-zone map (CallsignFileOperation). Truncation drops whole countries, so callsigns past the first chunk resolve to the wrong (or no) country/zone in the decode list, on the map, and in ADIF export.ituzone.json/cqzone.json/dxcc_list.json(450–740 KB) — loaded inDatabaseOprand parsed as JSON immediately after the read. A truncated buffer makesnew JSONObject(new String(bytes))throwJSONException, which is caught and logged — wiping the entire zone/DXCC table.bands.txt/rigaddress.txt/ help texts — smaller, but the same latent defect (OperationBand,RigNameList,HelpDialog,ClearCacheDataDialog).This is the same class of bug the team already fixed for log import in
LogFileImport.readFully(which carries a detailed comment on exactly this hazard). These eight sites were the remaining copies of the pattern.Fix
Added
com.k1af.ft8af.util.Streams.readAllBytes(InputStream), which drains the stream to EOF in a loop (8 KB working buffer,available()used only as an initial sizing hint). Routed all eight call sites through it. Decoding is otherwise unchanged — stillnew String(bytes)with the platform default charset — so this is strictly a "read the whole file" fix with no behavioral change on inputs that already fit in one read.Files:
CallsignFileOperation,RigNameList,OperationBand,DatabaseOpr(×3),HelpDialog,ClearCacheDataDialog, + newStreams.Testing
StreamsTest(new, pure JVM):readAllBytesfully drains a stream that yields one byte perread, and one that under-reportsavailable(); plus exact-content and empty-stream cases.CallsignFileOperationTest(new case): feedsgetLinesFromInputStreama chunked (one-byte-per-read) stream of 2000;-separated records and asserts every record survives. Verified this fails against the old single-read code and passes with the fix../gradlew :app:testDebugUnitTest— full suite green../gradlew :app:assembleDebug— full APK (all 4 ABIs, native + hamlib) builds successfully.Risk
Low. No DSP/native/protocol changes. The only behavioral change is that these bundled files are now read in full instead of a truncated prefix; on any input small enough that the old single read already succeeded, output is byte-identical.