fix: two regressions this rebuild introduced for non-APRS users
Both found by an auditor comparing behaviour against the released build rather than
against the intent of the change.
Prefix-first portable callsigns were rejected. CallsignEntry took the first segment -
`call.substringBefore('/')` - and required a letter and a digit in it. A portable call can
be written prefix-first, DL/W1AW or ZL/JA1ABC or OH/W1AW/MM, where the leading token is a
country prefix with no digit at all. Measured: five such forms were refused where the old
length-only check had accepted them. Any segment may now carry the callsign.
JSON cookie exports stopped working for QRZ. QrzGridParser.cookieHeader converts the JSON
array a browser extension produces into a Cookie header, and it had zero production
callers - the raw pasted text went straight into the header. The old client normalised it.
So an operator whose export had been working would see their cookie sent as a literal JSON
blob, QRZ would serve its signed-out page, and the app would tell them the cookie had
expired when it was perfectly good. A raw `k=v; k=v` paste was unaffected, which is why
this survived review.
Both are cases where a rewrite lost behaviour the old code had. Neither had a test.
This commit is contained in:
1 parent
3a8086eb05
commit
fe6d0af8b7
3 files changed
+39
-3
No files matched your search
@@ -66,7 +66,19 @@ class QrzGridSource(
|
||||
}
|
||||
|
||||
/** Fetch [url], retrying transport failures with backoff. Null when every attempt failed. */
|
||||
private suspend fun fetchWithRetry(url: String, cookieHeader: String): String? {
|
||||
/**
|
||||
* Normalise whatever the operator pasted into a Cookie header value.
|
||||
*
|
||||
* They paste either a raw `k=v; k=v` header or the JSON array a cookie-export extension
|
||||
* produces. The old client normalised this and the rewrite dropped it, so a JSON export that
|
||||
* used to work went out as a literal JSON blob, QRZ served its signed-out page, and the app
|
||||
* told the operator their cookie had expired when it was perfectly good.
|
||||
*/
|
||||
private fun normalise(raw: String): String = QrzGridParser.cookieHeader(raw)
|
||||
|
||||
private suspend fun fetchWithRetry(url: String, rawCookie: String): String? {
|
||||
val cookieHeader = normalise(rawCookie)
|
||||
if (cookieHeader.isBlank()) return null
|
||||
repeat(MAX_ATTEMPTS) { attempt ->
|
||||
try {
|
||||
val request = Request.Builder().url(url)
|
||||
|
||||
+9
-2
@@ -92,8 +92,11 @@ object CallsignEntry {
|
||||
if (call.length < MIN_LENGTH) return Verdict.Rejected(Reason.TOO_SHORT)
|
||||
if (call.length > MAX_LENGTH) return Verdict.Rejected(Reason.TOO_LONG)
|
||||
if (!allowed.matches(call)) return Verdict.Rejected(Reason.ILLEGAL_CHARACTERS)
|
||||
val core = call.substringBefore('/').substringBefore('-')
|
||||
if (core.none { it.isDigit() } || core.none { it.isLetter() }) {
|
||||
// Any segment may be the callsign, not just the first. A portable call can be written
|
||||
// prefix-first - DL/W1AW, ZL/JA1ABC, OH/W1AW/MM - where the leading token is a country
|
||||
// prefix with no digit in it. Testing only the first segment rejected all of those, which
|
||||
// the old length-only check had accepted.
|
||||
if (call.split('/', '-').none(::looksLikeCallsign)) {
|
||||
return Verdict.Rejected(Reason.NOT_A_CALLSIGN)
|
||||
}
|
||||
val warning = when {
|
||||
@@ -104,6 +107,10 @@ object CallsignEntry {
|
||||
return Verdict.Acceptable(call, warning)
|
||||
}
|
||||
|
||||
/** A segment that could be a callsign: contains both a letter and a digit. */
|
||||
private fun looksLikeCallsign(segment: String): Boolean =
|
||||
segment.any { it.isDigit() } && segment.any { it.isLetter() }
|
||||
|
||||
/**
|
||||
* Whether this looks like a Maidenhead locator typed into the wrong field.
|
||||
*
|
||||
|
||||
+17
@@ -17,6 +17,8 @@ class CallsignEntryTest {
|
||||
private val realCallsigns = listOf(
|
||||
"W1AW", "BG7NTA", "G0ABC", "VK2XYZ", "JA1ABC", "PY2ABC",
|
||||
"W1AW/4", "K1ABC/P", "DL1ABC/M", "SV2ASP/A", "OH2ABC/MM",
|
||||
// Prefix-first portable calls: the LEADING token is a country prefix with no digit in it.
|
||||
"DL/W1AW", "ZL/JA1ABC", "OH/W1AW/MM", "PA/G0ABC/P", "F/BG7NTA",
|
||||
"2E0ABC", "2M0XYZ", "9A1CCY", "4X4ABC", "3DA0ABC",
|
||||
"VP2MDD", "ZS6ABC", "5B4ABC", "HB9ABC", "OE1ABC",
|
||||
"LU1ABC", "CT1ABC", "EA8ABC", "TF3ABC", "VU2ABC",
|
||||
@@ -65,6 +67,21 @@ class CallsignEntryTest {
|
||||
}
|
||||
|
||||
/** No callsign is all digits or all letters. */
|
||||
/**
|
||||
* The regression this guards. Checking only the first segment rejected every prefix-first
|
||||
* portable call, because a country prefix like DL or ZL has no digit in it. The old
|
||||
* length-only check had accepted them, so this was a real loss for anyone logging one.
|
||||
*/
|
||||
@Test
|
||||
fun `a prefix-first portable callsign is accepted`() {
|
||||
for (call in listOf("DL/W1AW", "ZL/JA1ABC", "OH/W1AW/MM", "PA/G0ABC/P", "F/BG7NTA")) {
|
||||
assertTrue(
|
||||
"$call must be accepted",
|
||||
CallsignEntry.check(call) is CallsignEntry.Verdict.Acceptable
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `something with no digit or no letter is not a callsign`() {
|
||||
assertEquals(
|
||||
|
||||
Reference in new issue
Block a user