From a52fe09e24dd2f53474ad66d17e8d040321ebf63 Mon Sep 17 00:00:00 2001 From: Optio Agent Date: Thu, 9 Jul 2026 01:45:41 +0000 Subject: [PATCH 1/2] Fix app crash on malformed/32-bit Xiegu CAT handle frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The HANDLE branch of X6100Radio.doReceiveLineEvent parsed the Xiegu client handle with an unguarded Integer.parseInt(head.substring(1), 16). Every sibling parse in this class (seq_number, resultCode, play_volume) and the identical handle parse in the Flex sibling (FlexRadio) wrap the same call in a try/catch; the X6100 variant moved the parse out of the XieguResponse constructor and lost the guard. Two inputs make it throw NumberFormatException: * a garbled or truncated frame whose tail is not hex — even a lone "H", whose tail is the empty string; and * a legitimate high-bit 32-bit handle such as "HFFFFFFFF": the Xiegu protocol handle is a full 32-bit value (see the HANDLE enum doc) and 0xFFFFFFFF overflows a signed int, so Integer.parseInt rejects it. That parse runs on the TCP CAT read thread (RadioTcpClient.SocketThread), whose read loop catches only SocketException/IOException. A NumberFormatException escaping the listener therefore propagates out of run(), killing the read thread and crashing the whole app via the default uncaught-exception handler. Extract the parse into a package-private static parseXieguHandle(head, currentHandle): it length-guards the input, parses via Long.parseLong so the full unsigned 32-bit range round-trips into the int handle field, and contains any remaining malformed input by keeping the current handle (matching FlexRadio). The handle field is write-only in this class, so preserving the prior value on a bad frame is behaviourally safe. Add X6100RadioHandleParseTest covering low handles, full 8-digit and high-bit handles (the 0xFFFFFFFF / 0x80000000 overflow cases that used to crash), and the lone-"H"/non-hex/empty/null frames. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../java/com/k1af/ft8af/x6100/X6100Radio.java | 47 ++++++++++++- .../x6100/X6100RadioHandleParseTest.java | 69 +++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 ft8af/app/src/test/java/com/k1af/ft8af/x6100/X6100RadioHandleParseTest.java diff --git a/ft8af/app/src/main/java/com/k1af/ft8af/x6100/X6100Radio.java b/ft8af/app/src/main/java/com/k1af/ft8af/x6100/X6100Radio.java index 943d33425..80fdbe297 100644 --- a/ft8af/app/src/main/java/com/k1af/ft8af/x6100/X6100Radio.java +++ b/ft8af/app/src/main/java/com/k1af/ft8af/x6100/X6100Radio.java @@ -630,7 +630,7 @@ private void doReceiveLineEvent(String line) { this.version = response.head.substring(1); break; case HANDLE: - this.handle = Integer.parseInt(response.head.substring(1), 16); + this.handle = parseXieguHandle(response.head, this.handle); break; case RESPONSE: if (XieguCommand.AUDIO == response.xieguCommand) {//response information for audio command @@ -675,6 +675,51 @@ private void doReceiveLineEvent(String line) { } + /** + * Parse a Xiegu client-handle response ("H" + hex handle) into its numeric + * value, returning {@code currentHandle} unchanged when the frame is empty, + * truncated, or not valid hex. + * + *

This is the crash fix for the HANDLE branch of {@link #doReceiveLineEvent}. + * The previous {@code Integer.parseInt(head.substring(1), 16)} was unguarded, + * unlike every sibling parse in this class (seq_number, resultCode, + * play_volume) and unlike the identical handle parse in + * {@link com.k1af.ft8af.flex.FlexRadio}, which wraps it in a try/catch. Two + * inputs made it throw {@link NumberFormatException}: a garbled/truncated + * frame whose tail is not hex (even a lone {@code "H"}, whose tail is empty), + * and a legitimate high-bit 32-bit handle such as {@code "HFFFFFFFF"} — the + * Xiegu protocol handle is a full 32-bit value (see the {@code HANDLE} enum + * doc) and {@code 0xFFFFFFFF} overflows a signed {@code int}, so + * {@code Integer.parseInt} rejected it. This parse runs on the TCP CAT read + * thread ({@link com.k1af.ft8af.flex.RadioTcpClient}), whose read loop catches + * only {@code SocketException}/{@code IOException}; a {@code NumberFormatException} + * escaping here therefore killed that thread and crashed the whole app. We + * parse via {@code Long.parseLong} so the full unsigned 32-bit range is + * accepted (stored back into the {@code int} handle with the same bit + * pattern), and contain any remaining malformed input. + * + *

Package-private and static so it can be unit-tested without a live + * socket or {@code Context}. + * + * @param head the response head ({@code "H"} + hex handle); may be + * {@code null} or too short to carry a handle + * @param currentHandle the handle value to keep when {@code head} can't be parsed + * @return the parsed handle, or {@code currentHandle} on any parse failure + */ + static int parseXieguHandle(String head, int currentHandle) { + if (head == null || head.length() < 2) { + return currentHandle; + } + try { + // 32-bit handle: parse as long, then narrow so 0x80000000..0xFFFFFFFF + // (which overflow a signed-int parse) round-trip into the int field. + return (int) Long.parseLong(head.substring(1), 16); + } catch (NumberFormatException e) { + Log.e(TAG, "XieguResponse parseInt handle exception: " + e.getMessage()); + return currentHandle; + } + } + /** * Get the radio's audio information * diff --git a/ft8af/app/src/test/java/com/k1af/ft8af/x6100/X6100RadioHandleParseTest.java b/ft8af/app/src/test/java/com/k1af/ft8af/x6100/X6100RadioHandleParseTest.java new file mode 100644 index 000000000..e55a1d919 --- /dev/null +++ b/ft8af/app/src/test/java/com/k1af/ft8af/x6100/X6100RadioHandleParseTest.java @@ -0,0 +1,69 @@ +package com.k1af.ft8af.x6100; + +import static com.google.common.truth.Truth.assertThat; + +import org.junit.Test; + +/** + * Pure-JVM coverage for {@link X6100Radio#parseXieguHandle(String, int)} — the + * crash fix for the Xiegu {@code HANDLE} ("H" + 32-bit hex) response. + * + *

The parse runs on the TCP CAT read thread + * ({@link com.k1af.ft8af.flex.RadioTcpClient}), whose read loop catches only + * {@code SocketException}/{@code IOException}. The old unguarded + * {@code Integer.parseInt(head.substring(1), 16)} therefore let a + * {@link NumberFormatException} from a garbled/truncated frame — or from a + * legitimate high-bit 32-bit handle that overflows a signed {@code int} — + * escape and crash the whole app. These tests pin the guarded behaviour. + * + *

{@code android.util.Log} returns defaults under plain JUnit + * (unitTests.returnDefaultValues), so no Robolectric runner is needed. + */ +public class X6100RadioHandleParseTest { + + private static final int KEEP = 0x1234; // sentinel "current handle" to preserve + + @Test + public void parsesLowHandle() { + assertThat(X6100Radio.parseXieguHandle("H1A2B", KEEP)).isEqualTo(0x1A2B); + } + + @Test + public void parsesFullEightDigitHandle() { + assertThat(X6100Radio.parseXieguHandle("H0009ABCD", KEEP)).isEqualTo(0x0009ABCD); + } + + @Test + public void parsesHighBitHandle_wouldOverflowSignedIntParse() { + // 0xFFFFFFFF is a valid 32-bit Xiegu handle but overflows Integer.parseInt, + // which threw NumberFormatException and crashed the CAT read thread. + assertThat(X6100Radio.parseXieguHandle("HFFFFFFFF", KEEP)).isEqualTo(-1); + } + + @Test + public void parsesHighBitHandle_minValue() { + assertThat(X6100Radio.parseXieguHandle("H80000000", KEEP)).isEqualTo(Integer.MIN_VALUE); + } + + @Test + public void loneHandleLetter_keepsCurrent_doesNotCrash() { + // Regression: "H" -> substring(1) == "" -> parseInt("", 16) threw. + assertThat(X6100Radio.parseXieguHandle("H", KEEP)).isEqualTo(KEEP); + } + + @Test + public void nonHexTail_keepsCurrent_doesNotCrash() { + // Regression: a truncated/garbled frame like "HELLO" -> parseInt("ELLO", 16) threw. + assertThat(X6100Radio.parseXieguHandle("HELLO", KEEP)).isEqualTo(KEEP); + } + + @Test + public void emptyString_keepsCurrent() { + assertThat(X6100Radio.parseXieguHandle("", KEEP)).isEqualTo(KEEP); + } + + @Test + public void nullHead_keepsCurrent() { + assertThat(X6100Radio.parseXieguHandle(null, KEEP)).isEqualTo(KEEP); + } +} From cecc56f9adc4d3d34613028f10dec2dec8b2d52d Mon Sep 17 00:00:00 2001 From: Optio Agent Date: Thu, 9 Jul 2026 02:05:52 +0000 Subject: [PATCH 2/2] Address review: strictly validate the Xiegu handle tail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review on PR #488: parsing via Long.parseLong alone was too permissive for an unsigned handle — it accepted a leading sign ("H-1" -> -1 instead of keeping the current handle) and accepted more than 8 hex digits and then silently truncated on the narrowing int cast ("H100000000" -> 0). The error log also copied FlexRadio's misleading "parseInt"/"XieguResponse" wording. Validate the tail as 1-8 unsigned hex digits (isUnsignedHex) before parsing; reject anything else and keep the last good handle. After validation Long.parseLong can't throw, and 0x80000000..0xFFFFFFFF still round-trip into the int field with the same 32-bit pattern. Log an accurate message naming the offending tail. Add tests for lowercase hex, a signed tail, an overlong (9-digit) tail, and a whitespace tail. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../java/com/k1af/ft8af/x6100/X6100Radio.java | 45 ++++++++++++++----- .../x6100/X6100RadioHandleParseTest.java | 26 +++++++++++ 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/ft8af/app/src/main/java/com/k1af/ft8af/x6100/X6100Radio.java b/ft8af/app/src/main/java/com/k1af/ft8af/x6100/X6100Radio.java index 80fdbe297..fe12342b9 100644 --- a/ft8af/app/src/main/java/com/k1af/ft8af/x6100/X6100Radio.java +++ b/ft8af/app/src/main/java/com/k1af/ft8af/x6100/X6100Radio.java @@ -693,10 +693,17 @@ private void doReceiveLineEvent(String line) { * {@code Integer.parseInt} rejected it. This parse runs on the TCP CAT read * thread ({@link com.k1af.ft8af.flex.RadioTcpClient}), whose read loop catches * only {@code SocketException}/{@code IOException}; a {@code NumberFormatException} - * escaping here therefore killed that thread and crashed the whole app. We - * parse via {@code Long.parseLong} so the full unsigned 32-bit range is - * accepted (stored back into the {@code int} handle with the same bit - * pattern), and contain any remaining malformed input. + * escaping here therefore killed that thread and crashed the whole app. + * + *

A valid handle is 1–8 unsigned hex digits. We validate that + * shape explicitly and only then parse via {@code Long.parseLong} (so the + * full {@code 0x00000000}–{@code 0xFFFFFFFF} range round-trips into the + * {@code int} handle field with the same bit pattern). Rejecting anything + * else keeps the last good handle rather than misinterpreting the frame: + * {@code Long.parseLong} would otherwise accept a leading sign + * (e.g. {@code "H-1"} → {@code -1}) and would accept more than 8 digits and + * then silently truncate them on the narrowing cast (e.g. {@code "H100000000"} + * → {@code 0}). * *

Package-private and static so it can be unit-tested without a live * socket or {@code Context}. @@ -710,14 +717,32 @@ static int parseXieguHandle(String head, int currentHandle) { if (head == null || head.length() < 2) { return currentHandle; } - try { - // 32-bit handle: parse as long, then narrow so 0x80000000..0xFFFFFFFF - // (which overflow a signed-int parse) round-trip into the int field. - return (int) Long.parseLong(head.substring(1), 16); - } catch (NumberFormatException e) { - Log.e(TAG, "XieguResponse parseInt handle exception: " + e.getMessage()); + String hex = head.substring(1); + if (hex.length() > 8 || !isUnsignedHex(hex)) { + Log.e(TAG, "Xiegu handle parse failed (expected 1-8 hex digits): " + hex); return currentHandle; } + // Validated to 1-8 unsigned hex digits, so this never throws; parse as + // long so 0x80000000..0xFFFFFFFF narrow into the int field with the same + // 32-bit pattern (a plain signed-int parse would reject those). + return (int) Long.parseLong(hex, 16); + } + + /** @return true iff {@code s} is non-empty and every char is {@code [0-9a-fA-F]}. */ + private static boolean isUnsignedHex(String s) { + if (s.isEmpty()) { + return false; + } + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + boolean hex = (c >= '0' && c <= '9') + || (c >= 'a' && c <= 'f') + || (c >= 'A' && c <= 'F'); + if (!hex) { + return false; + } + } + return true; } /** diff --git a/ft8af/app/src/test/java/com/k1af/ft8af/x6100/X6100RadioHandleParseTest.java b/ft8af/app/src/test/java/com/k1af/ft8af/x6100/X6100RadioHandleParseTest.java index e55a1d919..6bc617439 100644 --- a/ft8af/app/src/test/java/com/k1af/ft8af/x6100/X6100RadioHandleParseTest.java +++ b/ft8af/app/src/test/java/com/k1af/ft8af/x6100/X6100RadioHandleParseTest.java @@ -57,6 +57,32 @@ public void nonHexTail_keepsCurrent_doesNotCrash() { assertThat(X6100Radio.parseXieguHandle("HELLO", KEEP)).isEqualTo(KEEP); } + @Test + public void lowercaseHex_isParsed() { + // The handle tail keeps the frame's original case (only the header char is + // upper-cased for dispatch), so lowercase hex is legitimate. + assertThat(X6100Radio.parseXieguHandle("Hbeef", KEEP)).isEqualTo(0xBEEF); + } + + @Test + public void signedTail_keepsCurrent() { + // Long.parseLong would accept the sign ("-1" -> -1); a handle is unsigned, + // so a signed tail is malformed and must preserve the current handle. + assertThat(X6100Radio.parseXieguHandle("H-1", KEEP)).isEqualTo(KEEP); + } + + @Test + public void overlongTail_keepsCurrent_noSilentTruncation() { + // 9 hex digits exceed a 32-bit handle; parsing+narrowing would silently + // truncate ("H100000000" -> 0), so reject and keep the current handle. + assertThat(X6100Radio.parseXieguHandle("H100000000", KEEP)).isEqualTo(KEEP); + } + + @Test + public void whitespaceTail_keepsCurrent() { + assertThat(X6100Radio.parseXieguHandle("H 1A", KEEP)).isEqualTo(KEEP); + } + @Test public void emptyString_keepsCurrent() { assertThat(X6100Radio.parseXieguHandle("", KEEP)).isEqualTo(KEEP);