From e27d692e8feaab248f8fbba2c063b15ef836c71a Mon Sep 17 00:00:00 2001 From: QIU Date: Wed, 26 Aug 2026 12:54:51 +0000 Subject: [PATCH] fix(log): the grid check let non-ASCII digits through and refused a legal length Checked against ADIF 3.1.7 (2026-03-22, the current release) rather than my own reading. Two of my rules were wrong. Char.isDigit() is Unicode-aware and covers the whole Nd category - some 600 characters. So Arabic-Indic, Devanagari, Persian and fullwidth digits all passed as a square pair, which a localised keypad produces without the operator seeing any difference. The spec is explicit: "Digit - an ASCII character whose code lies in the range of 48 through 57, inclusive." Wavelog stores GRIDSQUARE verbatim, so such a value would never match a real grid in any statistics or VUCC query - the exact failure this validation exists to prevent. Two-character locators are legal. The GridSquare type is "a case-insensitive 2-character, 4-character, 6-character, or 8-character Maidenhead locator" and the GRIDSQUARE field description repeats all four. My comment claimed Maidenhead had no other lengths, and a test name asserted there was no two-character form. Both were wrong. It is accepted now with a note that a field is accurate to about 1000km - the same treatment four characters already had. That also uncovered a latent crash: the square-pair check read index 2 of a string that may only have two characters. What survived the check: the A-R field range is right, verified by replicating qthToPosition's arithmetic - SS12AA decodes to 92N 182E, past both the pole and the antimeridian, while RR99 is the last cell inside the world. Wavelog's own Qra.php validates with the same range. The subsquare A-X range and digits in positions 7-8 are also correct. On 10 and 12 character locators the spec says store the first 8 in GRIDSQUARE and the rest in GRIDSQUARE_EXT. Neither WavelogQso nor Wavelog's field list carries GRIDSQUARE_EXT, so the extra pair has nowhere to go; the field clips at 8, which produces the spec-correct GRIDSQUARE value. Recorded in a comment rather than pretended to be deliberate. 17 tests now, including the four non-ASCII digit families and the two-character boundary. --- .../look4sat/core/domain/wavelog/GridEntry.kt | 38 +++++++++++-- .../core/domain/wavelog/GridEntryTest.kt | 53 +++++++++++++++++-- .../src/main/res/values-zh/strings.xml | 1 + .../src/main/res/values/strings.xml | 1 + .../rtbishop/look4sat/feature/radar/LogTab.kt | 11 ++-- 5 files changed, 91 insertions(+), 13 deletions(-) diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/GridEntry.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/GridEntry.kt index a16b6853..67c66bff 100644 --- a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/GridEntry.kt +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/GridEntry.kt @@ -46,6 +46,12 @@ object GridEntry { /** Worth mentioning but not worth refusing. */ enum class Warning { + /** + * Two characters. Legal per ADIF, but a field is 20 by 10 degrees - close to useless for a + * satellite contact, so it is worth saying rather than refusing. + */ + FIELD_ONLY, + /** Four characters, so the location is only accurate to about 100km. */ SQUARE_ONLY } @@ -81,7 +87,14 @@ object GridEntry { if (upper[0] !in FIELD_RANGE || upper[1] !in FIELD_RANGE) { return Verdict.Unusable(Reason.FIELD_OUT_OF_RANGE) } - if (!upper[2].isDigit() || !upper[3].isDigit()) { + // ASCII digits only. Char.isDigit() is Unicode-aware and covers the whole Nd category, so + // it accepted Arabic-Indic, Devanagari and fullwidth digits - which a localised keypad can + // produce without the operator seeing any difference. ADIF 3.1.7 defines Digit as "an ASCII + // character whose code lies in the range of 48 through 57", and Wavelog stores GRIDSQUARE + // verbatim, so such a value would never match a real grid in any statistics query. + // Guarded on length: a 2-character locator has no square pair, and reading index 2 of it + // would throw. + if (text.length >= SQUARE_LENGTH && (!upper[2].isAsciiDigit() || !upper[3].isAsciiDigit())) { return Verdict.Unusable(Reason.SQUARE_NOT_DIGITS) } if (text.length >= SUBSQUARE_LENGTH) { @@ -89,27 +102,42 @@ object GridEntry { return Verdict.Unusable(Reason.SUBSQUARE_OUT_OF_RANGE) } } - if (text.length == EXTENDED_LENGTH && (!upper[6].isDigit() || !upper[7].isDigit())) { + if (text.length == EXTENDED_LENGTH && (!upper[6].isAsciiDigit() || !upper[7].isAsciiDigit())) { return Verdict.Unusable(Reason.SQUARE_NOT_DIGITS) } return Verdict.Acceptable( normalised = normalise(upper), - warning = if (text.length == SQUARE_LENGTH) Warning.SQUARE_ONLY else null + warning = when (text.length) { + FIELD_LENGTH -> Warning.FIELD_ONLY + SQUARE_LENGTH -> Warning.SQUARE_ONLY + else -> null + } ) } /** `OL72ap` - upper case field, digits, lower case subsquare, as the convention renders it. */ private fun normalise(upper: String): String = buildString { - append(upper.take(SQUARE_LENGTH)) + append(upper.take(minOf(upper.length, SQUARE_LENGTH))) if (upper.length >= SUBSQUARE_LENGTH) append(upper.substring(4, 6).lowercase()) if (upper.length == EXTENDED_LENGTH) append(upper.substring(6, 8)) } + /** ADIF 3.1.7 defines Digit as ASCII 48-57. Kotlin's isDigit() is far wider. */ + private fun Char.isAsciiDigit(): Boolean = this in '0'..'9' + + private const val FIELD_LENGTH = 2 private const val SQUARE_LENGTH = 4 private const val SUBSQUARE_LENGTH = 6 private const val EXTENDED_LENGTH = 8 - private val VALID_LENGTHS = setOf(SQUARE_LENGTH, SUBSQUARE_LENGTH, EXTENDED_LENGTH) + + /** + * ADIF 3.1.7: GRIDSQUARE takes "2-character, 4-character, 6-character, or 8-character" locators. + * A 10 or 12 character locator stores its first 8 here and the rest in GRIDSQUARE_EXT, which + * neither WavelogQso nor Wavelog's own field list carries - so the extra pair has nowhere to go + * and the UI clips at 8, which produces the spec-correct GRIDSQUARE value. + */ + private val VALID_LENGTHS = setOf(FIELD_LENGTH, SQUARE_LENGTH, SUBSQUARE_LENGTH, EXTENDED_LENGTH) private val FIELD_RANGE = 'A'..'R' private val SUBSQUARE_RANGE = 'A'..'X' } diff --git a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/GridEntryTest.kt b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/GridEntryTest.kt index 96f1530e..679dc583 100644 --- a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/GridEntryTest.kt +++ b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/GridEntryTest.kt @@ -80,6 +80,49 @@ class GridEntryTest { assertEquals("OL72ap", (GridEntry.check(" OL72AP ") as GridEntry.Verdict.Acceptable).normalised) } + /** + * Two characters is legal per ADIF, so it is accepted - but a field is 20 by 10 degrees, close + * to useless for a satellite contact, so it says so rather than refusing. + */ + @Test + fun `a two character field is acceptable with a warning`() { + assertEquals( + GridEntry.Verdict.Acceptable("OL", GridEntry.Warning.FIELD_ONLY), + GridEntry.check("ol") + ) + } + + /** A 2-character entry must not read past its own end. */ + @Test + fun `a two character field does not crash`() { + for (grid in listOf("AA", "RR", "ol", "FN")) { + assertTrue(GridEntry.check(grid) is GridEntry.Verdict.Acceptable) + } + } + + /** + * Kotlin's Char.isDigit() is Unicode-aware and accepted Arabic-Indic, Devanagari, Persian and + * fullwidth digits, which a localised keypad produces without the operator seeing a difference. + * ADIF 3.1.7 defines Digit as ASCII 48-57, and Wavelog stores GRIDSQUARE verbatim, so such a + * value would never match a real grid in any statistics query. + */ + @Test + fun `non-ascii digits are not digits`() { + val nonAscii = listOf( + "OL\u0661\u0662ap", + "OL\u0968\u0968ap", + "OL\uff11\uff12ap", + "OL\u06f1\u06f2ap", + "IO91vl\u0663\u0669" + ) + for (grid in nonAscii) { + assertTrue( + "must be refused: " + grid, + GridEntry.check(grid) is GridEntry.Verdict.Unusable + ) + } + } + /** Nothing typed means the QRZ lookup should run, which is a different outcome from bad input. */ @Test fun `an empty entry is neither acceptable nor unusable`() { @@ -87,10 +130,14 @@ class GridEntryTest { assertEquals(GridEntry.Verdict.Empty, GridEntry.check(" ")) } - /** Maidenhead has no odd lengths and no two-character form in this context. */ + /** + * ADIF 3.1.7 permits 2, 4, 6 and 8 characters and nothing between them. A 10 or 12 character + * locator stores its first 8 here with the rest in GRIDSQUARE_EXT, which nothing in this app + * carries - the field clips at 8, which produces the spec-correct value. + */ @Test - fun `only 4 6 or 8 characters can be a grid`() { - for (bad in listOf("OL", "OL7", "OL72A", "OL72APX", "OL72AP123")) { + fun `only 2 4 6 or 8 characters can be a grid`() { + for (bad in listOf("O", "OL7", "OL72A", "OL72APX", "OL72AP123")) { val verdict = GridEntry.check(bad) assertEquals( "$bad must be refused for length, got $verdict", diff --git a/core/presentation/src/main/res/values-zh/strings.xml b/core/presentation/src/main/res/values-zh/strings.xml index 5d313408..d508969f 100644 --- a/core/presentation/src/main/res/values-zh/strings.xml +++ b/core/presentation/src/main/res/values-zh/strings.xml @@ -280,6 +280,7 @@ 对方报给你的网格。留空则从 QRZ 查询。 不是有效网格 仅方格 - 精度约 100 公里 + 仅大方格 - 精度约 1000 公里 模式 修改模式 时间 diff --git a/core/presentation/src/main/res/values/strings.xml b/core/presentation/src/main/res/values/strings.xml index 7272880f..85b48983 100644 --- a/core/presentation/src/main/res/values/strings.xml +++ b/core/presentation/src/main/res/values/strings.xml @@ -311,6 +311,7 @@ What they sent you. Left empty, it is looked up on QRZ. Not a grid square Square only - accurate to about 100 km + Field only - accurate to about 1000 km Mode Edit the mode Time diff --git a/feature/radar/src/main/java/com/rtbishop/look4sat/feature/radar/LogTab.kt b/feature/radar/src/main/java/com/rtbishop/look4sat/feature/radar/LogTab.kt index 1173aeb4..c8033f4b 100644 --- a/feature/radar/src/main/java/com/rtbishop/look4sat/feature/radar/LogTab.kt +++ b/feature/radar/src/main/java/com/rtbishop/look4sat/feature/radar/LogTab.kt @@ -473,12 +473,13 @@ private fun ExpandedLogInput( // where a wrong square is worse than a missing one. text = when (gridVerdict) { is GridEntry.Verdict.Unusable -> stringResource(id = R.string.wavelog_grid_bad) - is GridEntry.Verdict.Acceptable -> - if (gridVerdict.warning == GridEntry.Warning.SQUARE_ONLY) { + is GridEntry.Verdict.Acceptable -> when (gridVerdict.warning) { + GridEntry.Warning.FIELD_ONLY -> + stringResource(id = R.string.wavelog_grid_field_only) + GridEntry.Warning.SQUARE_ONLY -> stringResource(id = R.string.wavelog_grid_square_only) - } else { - stringResource(id = R.string.wavelog_grid_help) - } + null -> stringResource(id = R.string.wavelog_grid_help) + } GridEntry.Verdict.Empty -> stringResource(id = R.string.wavelog_grid_help) }, style = MaterialTheme.typography.bodySmall