fix(wavelog): the response reader lost contacts three more ways

An audit cloned the Wavelog server and read the QSO endpoints rather than the
documentation. The previous commit had transcribed the wrong endpoint - `success`,
`successful` and `dupe` come from create_station; the QSO path answers `created` on
success and `abort` with a 400 when a record in a batch failed. Three defects followed,
and the worst reintroduced the very failure the class was written to prevent.

Matching the bare word "duplicate" anywhere in the body classified a hard rejection as a
duplicate, which maps to success and drops the QSO from the queue. This is not
hypothetical: the server's own rejection text is "Duplicate for <call>", built in
Logbook_model::import, and Api_v2 puts strip_tags'd copies of those messages into
validation_error bodies. Probed against real response bodies, five of ten lost the
contact. Only the status field counts now, or a 409.

An HTML body was accepted. A reverse proxy, a maintenance page or a PHP fatal answers 200
with HTML and no status token, so it fell through to Accepted and a misconfigured proxy
ate contacts silently. A body starting with `<` is Unreadable, which keeps the QSO queued.

A rejection from v2 stopped the upload. v2 refuses a legacy v1 key with 401 invalid_token
- the app sends the v1 key as a Bearer token, and Api_v2::authenticate requires a wl2_
prefix - so a v1-only operator could not upload at all. That was a regression against the
pre-fix code, which fell through on any non-2xx. Both v2 and the first v1 endpoint now
always fall through; only the last one is final, and the failure message carries the most
specific reason any endpoint gave plus all three status codes. The third was previously
dropped from that message.

Also: the v2 error envelope has no status key at all, so `"error":` is now recognised on
its own.
This commit is contained in:
mckero committed 2026-08-26 02:11:55 +00:00
1 parent e8e67b74cd
commit 13c5fc9584
3 files changed
+132 -17

No files matched your search

@@ -242,13 +242,14 @@ object WaveLogApi {
// QSO arrives as HTTP 200 with {"status":"failed"}. Trusting the code marked it uploaded
// and dropped it from the queue.
val (code, resp) = httpRequest("$base/index.php/api/v2/qso", "POST", apiKey, v2Body.toString())
when (val verdict = WavelogResponse.verdict(code, resp)) {
val v2Verdict = WavelogResponse.verdict(code, resp)
when (v2Verdict) {
is WavelogResponse.Verdict.Accepted -> return@withContext WavelogResult.Success("v2")
WavelogResponse.Verdict.Duplicate -> return@withContext WavelogResult.Success("duplicate")
is WavelogResponse.Verdict.Rejected ->
return@withContext WavelogResult.Failure(verdict.reason)
// Unreadable falls through to v1: an older server may not have the v2 endpoint at all.
is WavelogResponse.Verdict.Unreadable -> Unit
// Anything else falls through to v1. A rejection here is NOT final: v2 refuses a legacy
// v1 key with 401 invalid_token, and returning at that point stopped a v1-only operator
// from uploading at all. The v1 attempt below is the one that can speak for them.
else -> Unit
}
// v1: POST /index.php/api/qso (key in body + ADIF)
@@ -259,12 +260,13 @@ object WaveLogApi {
put("string", toAdif(qso, gridsquare, satName))
}
val (code1, resp1) = httpRequest("$base/index.php/api/qso", "POST", apiKey, v1Body.toString())
when (val verdict = WavelogResponse.verdict(code1, resp1)) {
val v1Verdict = WavelogResponse.verdict(code1, resp1)
when (v1Verdict) {
is WavelogResponse.Verdict.Accepted -> return@withContext WavelogResult.Success("v1")
WavelogResponse.Verdict.Duplicate -> return@withContext WavelogResult.Success("duplicate")
is WavelogResponse.Verdict.Rejected ->
return@withContext WavelogResult.Failure(verdict.reason)
is WavelogResponse.Verdict.Unreadable -> Unit
// Also falls through: a server with different rewrite rules answers this path with a
// 404 page, which is a rejection but says nothing about whether the QSO can be stored.
else -> Unit
}
// v1 without index.php, for a server whose rewrite rules differ
@@ -279,8 +281,17 @@ object WaveLogApi {
// Every endpoint answered something we could not read. Keeping the QSO queued is the only
// honest outcome: it may have been stored, and dropping it would lose the contact.
// No endpoint accepted it. The v2 reason is preferred when it explained itself, since a 401
// invalid_token is the most actionable thing an operator can be told; otherwise all three
// status codes go out, because the third was previously dropped from this message.
val reasons = listOfNotNull(
(v1Verdict as? WavelogResponse.Verdict.Rejected)?.reason,
(v2Verdict as? WavelogResponse.Verdict.Rejected)?.reason
).filter { it.isNotBlank() }
WavelogResult.Failure(
"unreadable response: v2 HTTP $code, v1 HTTP $code1 - ${shortError(resp1.ifBlank { resp1b })}"
reasons.firstOrNull()
?: ("no endpoint accepted it: v2 HTTP $code, v1 HTTP $code1, v1-alt HTTP $code1b" +
" - " + shortError(resp1.ifBlank { resp1b }))
)
}
@@ -61,17 +61,27 @@ object WavelogResponse {
val lower = text.lowercase().replace(AROUND_SEPARATORS, "")
// Duplicate reported three ways: a 409, the word in a message, or Wavelog's own
// `{"status":"dupe"}` - which arrives with HTTP 200 and is documented, not guessed.
if (statusCode == DUPLICATE_CODE ||
DUPLICATE_MARKERS.any { lower.contains(it) }
) {
return Verdict.Duplicate
}
// An HTML body is a proxy or a PHP fatal, never a verdict. Checked first because such a
// page carries no status token and would otherwise read as an acceptance, so a
// misconfigured reverse proxy answering 200 would eat contacts.
if (lower.startsWith("<")) return Verdict.Unreadable(text.take(MAX_DETAIL))
// Duplicate only when the STATUS says so, or on a 409. The word alone is not enough: the
// server's own rejection text is "Duplicate for <call>", and Api_v2 surfaces that inside
// validation_error bodies - so matching the bare substring classified a hard rejection as
// a duplicate, which maps to success and drops the QSO from the queue. That is the very
// defect this class was written to prevent, reached through a different door.
if (statusCode == DUPLICATE_CODE || lower.contains(DUPE_STATUS)) return Verdict.Duplicate
if (statusCode !in SUCCESS_CODES) {
return Verdict.Rejected(reasonFrom(text).ifBlank { "HTTP $statusCode" })
}
if (FAILURE_MARKERS.any { lower.contains(it) }) {
return Verdict.Rejected(reasonFrom(text).ifBlank { "server reported failure" })
}
// The v2 endpoint reports errors in an envelope with no status key at all.
if (lower.contains("\"error\":")) {
return Verdict.Rejected(reasonFrom(text).ifBlank { "server reported an error" })
}
// A status field we cannot read is not a success. Saying so keeps the QSO queued.
if (lower.contains("\"status\"") && SUCCESS_MARKERS.none { lower.contains(it) }) {
return Verdict.Unreadable(text.take(MAX_DETAIL))
@@ -115,15 +125,23 @@ object WavelogResponse {
* `successful` matters as much as `success`: the API reference uses both, and treating one as
* unrecognised would leave a stored QSO queued forever.
*/
/**
* Success wordings, taken from the Wavelog server source rather than its documentation.
*
* The QSO endpoint answers `created`; `success` and `successful` come from other endpoints and
* are kept because a future version may use them. `abort` is NOT here - v1 uses it when any
* record in a batch failed, and it arrives with a 400.
*/
private val SUCCESS_MARKERS = listOf(
"\"status\":\"created\"",
"\"status\":\"success\"",
"\"status\":\"successful\"",
"\"status\":\"created\"",
"\"status\":\"ok\""
)
/** `dupe` is Wavelog's own wording and arrives with a 200. */
private val DUPLICATE_MARKERS = listOf("\"status\":\"dupe\"", "duplicate")
/** Wavelog's own duplicate status. The bare word "duplicate" is deliberately NOT a marker. */
private const val DUPE_STATUS = "\"status\":\"dupe\""
/** Whitespace next to a colon or comma, which JSON allows and servers use inconsistently. */
private val AROUND_SEPARATORS = Regex("""\s*(?=[:,])|(?<=[:,])\s*""")
@@ -134,6 +134,92 @@ class WavelogResponseTest {
)
}
/**
* The defect that lost contacts. Wavelog's own rejection text is "Duplicate for <call>", and
* Api_v2 surfaces that inside validation_error bodies - so matching the bare word classified a
* hard rejection as a duplicate, which maps to success and drops the QSO from the queue.
*
* Bodies transcribed from the Wavelog server source, not from its documentation.
*/
@Test
fun `the word duplicate in a rejection does not make it a duplicate`() {
val bodies = listOf(
400 to """{"error":{"code":"validation_error","message":"duplicate submode is invalid"}}""",
400 to """{"status":"failed","reason":"duplicate key violation; QSO NOT stored"}""",
200 to """{"status":"failed","reason":"Duplicate for BG7NTA"}"""
)
for ((code, body) in bodies) {
val verdict = WavelogResponse.verdict(code, body)
assertTrue(
"must be a rejection, got " + verdict + " for " + body,
verdict is WavelogResponse.Verdict.Rejected
)
}
}
/** A success that mentions the word must still be a success. */
@Test
fun `the word duplicate in a success does not make it a failure`() {
val verdict = WavelogResponse.verdict(
201,
"""{"status":"created","adif_errors":0,"messages":["Removed duplicate mode entry"]}"""
)
assertTrue("got " + verdict, verdict is WavelogResponse.Verdict.Accepted)
}
/**
* A reverse proxy or a PHP fatal answers 200 with HTML. There is no status token, so it used to
* read as a stored QSO and a misconfigured proxy would eat contacts silently.
*/
@Test
fun `an html body is never an acceptance`() {
val pages = listOf(
"<!DOCTYPE html><html><head><title>502 Bad Gateway</title></head></html>",
"<html><body><h1>Maintenance</h1></body></html>",
"<br /><b>Fatal error</b>: Uncaught Error in /var/www/index.php"
)
for (page in pages) {
val verdict = WavelogResponse.verdict(200, page)
assertTrue(
"html must not be accepted, got " + verdict,
verdict is WavelogResponse.Verdict.Unreadable
)
}
}
/** The v2 endpoint reports errors in an envelope with no status key at all. */
@Test
fun `a v2 error envelope is a rejection`() {
val verdict = WavelogResponse.verdict(
401,
"""{"error":{"code":"invalid_token","message":"Bearer token must start with wl2_"}}"""
)
assertEquals(
WavelogResponse.Verdict.Rejected("Bearer token must start with wl2_"),
verdict
)
}
/** The QSO endpoint answers `created`, which is what the v1 source actually emits. */
@Test
fun `the created status the qso endpoint returns is accepted`() {
val verdict = WavelogResponse.verdict(
201,
"""{"status":"created","adif_count":1,"adif_errors":0,"messages":[""]}"""
)
assertTrue("got " + verdict, verdict is WavelogResponse.Verdict.Accepted)
}
/** v1 uses `abort` with a 400 when any record in a batch failed. Not a success. */
@Test
fun `an abort status is a rejection`() {
val verdict = WavelogResponse.verdict(
400,
"""{"status":"abort","messages":["Bad ADIF field CALL"]}"""
)
assertTrue("got " + verdict, verdict is WavelogResponse.Verdict.Rejected)
}
/** A failure with no explanation still has to say something usable. */
@Test
fun `a failure without a reason still reports one`() {