fix(aprs): report a failed send as failed, and a refused login as refused
Two defects made every APRS failure invisible. sendPacket ended with
`.getOrElse { Pair(true, "OK") }`, so a read that threw - including on a dead
socket - was reported as a successful send. And the login check threw inside a
runCatching whose result was discarded, so a server that refused to verify the
passcode could not propagate: aprsc keeps such a client connected and its writes
succeed while silently discarding every packet, which the app reported as success.
An audit put it plainly - twelve commits are all fix(aprs), none added a
socket-level test, so "it worked" was never evidence a packet had landed.
sendPacket now separates the cases. A read timeout stays a success, because
APRS-IS does not acknowledge position reports and silence is the normal outcome.
A closed stream or an IOException is a failure. The catch order matters and is
load-bearing: SocketTimeoutException extends IOException, so reversing them would
mark every normal report as failed.
The login handshake follows the spec: read the server's identification line first,
then log in, then read until a verdict arrives. AprsLogin holds that as pure logic
in core:domain with the parsing that decides it, including one trap worth naming -
"unverified" contains "verified", so the negative has to be tested first or every
refusal reads as acceptance. An explicit refusal now fails the report and shows
the operator its own message pointing at the callsign and passcode, in five
locales. A response we could not parse does not, since the packets may well be
landing and blaming the passcode would send them to fix something that works.
Three defects came out of review after that. The verdict is now bounded by a
deadline rather than a five-line budget, because a server that sent six keepalives
before its answer turned an accepted login into Unknown - telling the operator
their passcode was wrong when it had just been accepted. The greeting gets a short
two-second probe instead of the full login window, which cost eight seconds on
every connect to a server that sends none. And a refusal detected in the greeting
now aborts the connection instead of being overwritten by the next read, which had
made that branch and its comment a lie.
The worst of the three was mine: rewriting the write as print + flush dropped the
line terminator entirely. APRS-IS is a line protocol, so the server's reader never
saw a packet, while the send reported success and the read timed out into the
"silence is normal" branch. It broke healthy connections rather than dead ones and
was designed to have no symptom. Both the packet and the login line now end in an
explicit CRLF as the spec requires, rather than println's platform separator.
That defect is why this adds AprsIsClientSocketTest, which runs the client against
a stand-in server and reads the bytes back: it asserts two packets arrive as two
lines, that a login line arrives complete, that keepalive chatter does not bury the
verdict, that a greeting-less server connects promptly, and that a send to a closed
peer reports failure. Nothing in the pure-logic tests could have caught a missing
newline. Note for anyone extending it: closing the ServerSocket leaves an
established connection alive, so the dead-peer test has to close the accepted
socket - assuming otherwise made a correct implementation look broken.
AprsPacket.formatLogin is deleted, its work moved into AprsLogin.line, which also
replaces spaces in the version string because the server splits that field on
whitespace and the shipped value contained one.
This commit is contained in:
mckero committed
1 parent
ce68f48765
commit
7ac54f0a37
12 files changed
+669
-68
No files matched your search
@@ -1,16 +1,23 @@
|
||||
package com.rtbishop.look4sat.core.data.aprs
|
||||
|
||||
import com.rtbishop.look4sat.core.domain.aprs.AprsLogin
|
||||
import com.rtbishop.look4sat.core.domain.aprs.AprsPacket
|
||||
import java.io.BufferedReader
|
||||
import java.io.IOException
|
||||
import java.io.InputStreamReader
|
||||
import java.io.OutputStreamWriter
|
||||
import java.io.PrintWriter
|
||||
import java.net.InetSocketAddress
|
||||
import java.net.Socket
|
||||
import java.net.SocketTimeoutException
|
||||
|
||||
/**
|
||||
* APRS-IS TCP client (reverse-ported from APRSdroid TcpUploader.scala).
|
||||
* Plain-text protocol: one login line + one packet per line; 30 s reconnect after drop.
|
||||
* APRS-IS TCP client. Plain text: the server greets, the client logs in, then one packet per line.
|
||||
*
|
||||
* Two things here decide whether the operator can trust the app at all. A packet sent on a dead
|
||||
* socket must not report success, and a login the server refused to verify must not look like a
|
||||
* working connection - an unverified client stays connected while the server silently drops
|
||||
* everything it sends.
|
||||
*/
|
||||
class AprsIsClient(
|
||||
private val host: String,
|
||||
@@ -27,86 +34,178 @@ class AprsIsClient(
|
||||
private var reader: BufferedReader? = null
|
||||
private val lock = Any()
|
||||
|
||||
val isConnected: Boolean
|
||||
get() = synchronized(lock) { socket?.isConnected == true && !socket!!.isClosed }
|
||||
/**
|
||||
* What the server said about this login, or null before a login has been attempted.
|
||||
*
|
||||
* Kept as state rather than only thrown, because [AprsLogin.Outcome.Unverified] is not a
|
||||
* connection error: the socket is up and writes succeed. The operator has to be told, or
|
||||
* they will watch reports "succeed" for hours while nothing reaches the network.
|
||||
*/
|
||||
@Volatile
|
||||
var loginOutcome: AprsLogin.Outcome? = null
|
||||
private set
|
||||
|
||||
/** Connect + login (synchronous/blocking; call from a background thread) */
|
||||
val isConnected: Boolean
|
||||
get() = synchronized(lock) { socket?.isConnected == true && socket?.isClosed == false }
|
||||
|
||||
/** True when the server verified the passcode, so packets from this client are accepted. */
|
||||
val isVerified: Boolean get() = loginOutcome is AprsLogin.Outcome.Verified
|
||||
|
||||
/**
|
||||
* True only when the server told us it did NOT verify the login.
|
||||
*
|
||||
* Distinct from `!isVerified` on purpose. [AprsLogin.Outcome.Unverified] means the server
|
||||
* said so and really is discarding our packets. [AprsLogin.Outcome.Unknown] means we could
|
||||
* not recognise its answer - the packets may well be landing - so blaming the operator's
|
||||
* passcode for that would send them to fix something that is not broken.
|
||||
*/
|
||||
val isRefusedByServer: Boolean get() = loginOutcome is AprsLogin.Outcome.Unverified
|
||||
|
||||
/**
|
||||
* Connect and log in. Blocking; call from a background thread.
|
||||
*
|
||||
* Throws when the connection cannot be made or the server rejected the login outright.
|
||||
* A login the server accepted but did not verify returns normally and leaves
|
||||
* [loginOutcome] as [AprsLogin.Outcome.Unverified] for the caller to surface.
|
||||
*/
|
||||
@Throws(Exception::class)
|
||||
fun connect() {
|
||||
disconnect()
|
||||
loginOutcome = null
|
||||
val s = Socket()
|
||||
try {
|
||||
s.connect(InetSocketAddress(host, port), 30_000)
|
||||
s.soTimeout = timeoutSec * 1000
|
||||
s.connect(InetSocketAddress(host, port), CONNECT_MS)
|
||||
s.tcpNoDelay = true
|
||||
synchronized(lock) {
|
||||
socket = s
|
||||
writer = PrintWriter(OutputStreamWriter(s.getOutputStream(), Charsets.ISO_8859_1), true)
|
||||
reader = BufferedReader(InputStreamReader(s.getInputStream(), Charsets.ISO_8859_1), 256)
|
||||
}
|
||||
// Login line
|
||||
val login = AprsPacket.formatLogin(callsign, ssid, passcode, version) + filter
|
||||
writer?.println(login)
|
||||
// Read the login response (aprsc replies # logresp ... verified/unverified)
|
||||
runCatching {
|
||||
s.soTimeout = 8000
|
||||
val resp = reader?.readLine()
|
||||
if (resp != null && (resp.contains("Invalid", ignoreCase = true) ||
|
||||
resp.contains("unverified", ignoreCase = true))) {
|
||||
throw IllegalArgumentException(resp.trim())
|
||||
}
|
||||
// Restore timeout
|
||||
s.soTimeout = timeoutSec * 1000
|
||||
s.soTimeout = LOGIN_MS
|
||||
// The spec has the client log in AFTER the server's identification line, so read the
|
||||
// greeting first. Anything starting with # is a comment and may be skipped.
|
||||
readGreeting(s)?.let { refusal ->
|
||||
// The server refused before we even logged in. Previously this verdict was
|
||||
// computed and then overwritten by readLoginResponse, so the branch was a lie.
|
||||
loginOutcome = refusal
|
||||
throw IllegalArgumentException(refusal.detail)
|
||||
}
|
||||
val login = AprsLogin.line(callsign, ssid, passcode, version, filter)
|
||||
writer?.print(login)
|
||||
writer?.print(CRLF)
|
||||
writer?.flush()
|
||||
loginOutcome = readLoginResponse()
|
||||
s.soTimeout = timeoutSec * 1000
|
||||
val outcome = loginOutcome
|
||||
if (outcome is AprsLogin.Outcome.Rejected) throw IllegalArgumentException(outcome.detail)
|
||||
} catch (e: Exception) {
|
||||
// Close the local socket before re-throwing, so it does not leak when
|
||||
// an exception is raised after s.connect() but before socket = s.
|
||||
// Otherwise periodic reconnect attempts (AprsReporter every 1–60 min)
|
||||
// accumulate leaked fds until the process cannot open any more files.
|
||||
// Close the local socket before re-throwing so it does not leak when the failure
|
||||
// lands after connect() but before the field assignment - otherwise periodic
|
||||
// reconnects accumulate file descriptors until no more can be opened.
|
||||
runCatching { s.close() }
|
||||
synchronized(lock) {
|
||||
writer = null
|
||||
reader = null
|
||||
socket = null
|
||||
}
|
||||
throw e
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sends one APRS packet (one line) and tries to read the server ack.
|
||||
* Returns null=failed to send; Pair(ok, detail)=result (server error text lives in detail)
|
||||
* Consume the server's greeting comment, returning a refusal when it is not one.
|
||||
*
|
||||
* Absence is tolerated because some servers send none, but on a short probe window rather
|
||||
* than the full login timeout: waiting LOGIN_MS for a greeting that will never come cost
|
||||
* eight seconds on every single connect to such a server.
|
||||
*/
|
||||
private fun readGreeting(socket: Socket): AprsLogin.Outcome.Rejected? {
|
||||
val previous = socket.soTimeout
|
||||
return try {
|
||||
socket.soTimeout = GREETING_MS
|
||||
val line = reader?.readLine() ?: return null
|
||||
// A server that opens with anything but a comment is refusing us.
|
||||
if (line.startsWith("#")) null else AprsLogin.Outcome.Rejected(line.trim())
|
||||
} catch (ignored: IOException) {
|
||||
null
|
||||
} finally {
|
||||
runCatching { socket.soTimeout = previous }
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Read lines until the login verdict arrives, skipping keepalive comments.
|
||||
*
|
||||
* Bounded by the read timeout, so an unresponsive server cannot hang the caller.
|
||||
*/
|
||||
private fun readLoginResponse(): AprsLogin.Outcome {
|
||||
// Bounded by a deadline, not a line count: comments are free to skip, and a chatty
|
||||
// server that sent six of them before its verdict used to exhaust a fixed budget and
|
||||
// turn an accepted login into Unknown - telling the operator their passcode was wrong
|
||||
// when it had just been accepted.
|
||||
val deadline = System.currentTimeMillis() + LOGIN_MS
|
||||
while (System.currentTimeMillis() < deadline) {
|
||||
val line = try {
|
||||
reader?.readLine()
|
||||
} catch (timeout: SocketTimeoutException) {
|
||||
return AprsLogin.Outcome.Unknown("no response within ${LOGIN_MS}ms")
|
||||
} catch (failure: IOException) {
|
||||
return AprsLogin.Outcome.Rejected(failure.message ?: "login read failed")
|
||||
} ?: return AprsLogin.Outcome.Rejected("connection closed during login")
|
||||
AprsLogin.parse(line)?.let { return it }
|
||||
}
|
||||
return AprsLogin.Outcome.Unknown("no login response recognised")
|
||||
}
|
||||
|
||||
/**
|
||||
* Send one packet and report what happened.
|
||||
*
|
||||
* Returns null when there is no connection to write to. Otherwise a pair of whether the
|
||||
* packet went out and a detail string for the operator.
|
||||
*/
|
||||
fun sendPacket(packetLine: String): Pair<Boolean, String>? {
|
||||
synchronized(lock) {
|
||||
val w = writer ?: return null
|
||||
w.println(packetLine)
|
||||
// Reading the response inside the same lock: disconnect() may run concurrently and
|
||||
// null these fields, and reading outside the lock raced with that - the read could
|
||||
// hit a just-closed socket and be reported as a successful send.
|
||||
// CRLF explicitly rather than println: the spec requires "TNC2 format terminated by
|
||||
// a carriage return, line feed sequence", and println emits the platform separator,
|
||||
// a bare LF on Android. An earlier draft of this method sent no terminator at all,
|
||||
// which leaves the server's line reader waiting forever while every send reports
|
||||
// success - exactly the failure this class exists to prevent.
|
||||
w.print(packetLine)
|
||||
w.print(CRLF)
|
||||
w.flush()
|
||||
if (w.checkError()) return Pair(false, "write failed")
|
||||
// Read the server response inside the same lock: disconnect() (called
|
||||
// concurrently from stop()/reconnect on another thread) nulls
|
||||
// writer/reader/socket and closes them. Reading outside the lock raced
|
||||
// with that: the response read could hit a just-closed socket and the
|
||||
// swallowing runCatching reported Pair(true,"OK") for a packet that
|
||||
// never left, or read through a stale reference. Serialising keeps
|
||||
// the read on the connection this thread just wrote to. The 3 s read
|
||||
// timeout bounds how long a concurrent disconnect waits.
|
||||
return runCatching {
|
||||
val s = socket ?: return@runCatching Pair(true, "OK")
|
||||
val oldTimeout = s.soTimeout
|
||||
s.soTimeout = 3000
|
||||
try {
|
||||
val resp = reader?.readLine()
|
||||
if (resp != null && (resp.contains("Invalid", ignoreCase = true) ||
|
||||
resp.contains("error", ignoreCase = true))) {
|
||||
Pair(false, resp.trim())
|
||||
} else {
|
||||
Pair(true, if (resp.isNullOrBlank()) "OK" else resp.trim())
|
||||
}
|
||||
} finally {
|
||||
s.soTimeout = oldTimeout
|
||||
}
|
||||
}.getOrElse { Pair(true, "OK") }
|
||||
val s = socket ?: return Pair(false, "not connected")
|
||||
val previousTimeout = s.soTimeout
|
||||
return try {
|
||||
s.soTimeout = ACK_MS
|
||||
classifyAck(reader?.readLine())
|
||||
} catch (timeout: SocketTimeoutException) {
|
||||
// Silence is the normal case: APRS-IS does not acknowledge a position report, so
|
||||
// nothing arriving means the line went out and the server had nothing to say.
|
||||
// Telling this apart from a broken connection is the point of this method - the
|
||||
// previous version treated EVERY exception as success, so a dead socket reported
|
||||
// "sent OK" and made every real failure invisible, including a rejected login.
|
||||
Pair(true, "sent")
|
||||
} catch (failure: IOException) {
|
||||
Pair(false, failure.message ?: "read failed")
|
||||
} finally {
|
||||
runCatching { s.soTimeout = previousTimeout }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Read one line (server response; throws on timeout) */
|
||||
fun readLine(): String? {
|
||||
return reader?.readLine()
|
||||
/** Interpret whatever the server sent back after a packet. */
|
||||
private fun classifyAck(response: String?): Pair<Boolean, String> = when {
|
||||
response == null -> Pair(false, "connection closed by server")
|
||||
// Comments are keepalives and server chatter, not a verdict on this packet.
|
||||
response.startsWith("#") -> Pair(true, "sent")
|
||||
response.contains("Invalid", ignoreCase = true) ||
|
||||
response.contains("error", ignoreCase = true) -> Pair(false, response.trim())
|
||||
else -> Pair(true, response.trim())
|
||||
}
|
||||
|
||||
fun disconnect() {
|
||||
@@ -119,4 +218,16 @@ class AprsIsClient(
|
||||
socket = null
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
/** Line terminator the protocol requires, independent of the platform's own. */
|
||||
const val CRLF = "\r\n"
|
||||
|
||||
const val CONNECT_MS = 30_000
|
||||
const val LOGIN_MS = 8_000
|
||||
const val ACK_MS = 3_000
|
||||
|
||||
/** Probe window for the greeting, short because its absence is legitimate. */
|
||||
const val GREETING_MS = 2_000
|
||||
}
|
||||
}
|
||||
@@ -34,7 +34,17 @@ data class AprsReport(
|
||||
val timestamp: Long,
|
||||
val packet: String,
|
||||
val ok: Boolean,
|
||||
val detail: String
|
||||
val detail: String,
|
||||
/**
|
||||
* False only when the server told us it did not verify the login.
|
||||
*
|
||||
* Carried separately from [ok] because the two are independent: a refused client's writes
|
||||
* still succeed, so the packet leaves the phone and looks sent, while aprsc discards every
|
||||
* one of them. Without surfacing it the operator can watch reports succeed for hours with
|
||||
* nothing reaching the network. A login whose response we simply could not parse leaves this
|
||||
* true, since the packets may be landing and the passcode is not at fault.
|
||||
*/
|
||||
val verified: Boolean = true
|
||||
)
|
||||
|
||||
/** Report scheduler (periodic + manual trigger); connection management lives in the foreground service */
|
||||
@@ -100,15 +110,22 @@ class AprsReporter(
|
||||
val pos = positionProvider()
|
||||
val packetLine = buildPositionPacket(cfg, pos?.first, pos?.second)
|
||||
val result = c.sendPacket(packetLine)
|
||||
val ok = result?.first == true
|
||||
val sent = result?.first == true
|
||||
val detail = result?.second ?: "no connection"
|
||||
onReport(AprsReport(System.currentTimeMillis(), packetLine, ok, detail))
|
||||
// A write that succeeded on a login the server refused to verify is not a delivered
|
||||
// packet: aprsc takes it and drops it, which is what let every real failure hide.
|
||||
// Only an explicit refusal counts against us though - a login whose response we
|
||||
// could not parse may be working fine, and blaming the passcode for that would send
|
||||
// the operator to fix something that is not broken.
|
||||
val refused = c.isRefusedByServer
|
||||
val ok = sent && !refused
|
||||
onReport(AprsReport(System.currentTimeMillis(), packetLine, ok, detail, !refused))
|
||||
if (ok) onState(AprsState.Connected) else onState(AprsState.Error)
|
||||
} catch (e: Exception) {
|
||||
runCatching { client?.disconnect() }
|
||||
client = null
|
||||
onState(AprsState.Error)
|
||||
onReport(AprsReport(System.currentTimeMillis(), "", false, e.message ?: "error"))
|
||||
onReport(AprsReport(System.currentTimeMillis(), "", false, e.message ?: "error", false))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+233
@@ -0,0 +1,233 @@
|
||||
package com.rtbishop.look4sat.core.data.aprs
|
||||
|
||||
import java.io.BufferedReader
|
||||
import java.io.InputStreamReader
|
||||
import java.io.PrintWriter
|
||||
import java.net.ServerSocket
|
||||
import java.net.Socket
|
||||
import java.util.concurrent.CountDownLatch
|
||||
import java.util.concurrent.TimeUnit
|
||||
import kotlin.concurrent.thread
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* Socket-level tests against a stand-in APRS-IS server.
|
||||
*
|
||||
* These exist because the pure-logic tests could not catch the failures that matter here. A draft
|
||||
* of [AprsIsClient.sendPacket] once wrote the packet with no line terminator at all: every send
|
||||
* reported success, the server received a single unterminated stream, and nothing anywhere went
|
||||
* red. APRS-IS is a line protocol and does not acknowledge position reports, so silence is the
|
||||
* normal case - which means only a test that reads the bytes off a real socket can tell a
|
||||
* delivered packet from a lost one.
|
||||
*/
|
||||
class AprsIsClientSocketTest {
|
||||
|
||||
/**
|
||||
* A minimal APRS-IS server. Greets, answers the login as instructed, then records whatever
|
||||
* lines arrive without acknowledging them, which is what the real network does.
|
||||
*/
|
||||
private class FakeServer(
|
||||
private val greeting: String? = "# aprsc 2.1.19-g730c5c0",
|
||||
private val loginResponse: String? = "# logresp TEST verified, server FAKE",
|
||||
private val chatter: List<String> = emptyList()
|
||||
) : AutoCloseable {
|
||||
private val server = ServerSocket(0)
|
||||
private val ready = CountDownLatch(1)
|
||||
|
||||
/**
|
||||
* The accepted connection. Held because closing the ServerSocket only stops it listening
|
||||
* - an established connection survives, so a test that wants a dead peer has to close
|
||||
* this one. Getting that wrong made a correct implementation look broken.
|
||||
*/
|
||||
@Volatile
|
||||
private var peer: Socket? = null
|
||||
|
||||
val port: Int get() = server.localPort
|
||||
|
||||
/** Every complete line the client sent after logging in. */
|
||||
val received = mutableListOf<String>()
|
||||
|
||||
/** Raw bytes of the client's traffic, so a missing terminator is visible. */
|
||||
val rawAfterLogin = StringBuilder()
|
||||
|
||||
@Volatile
|
||||
var loginLine: String? = null
|
||||
|
||||
fun start() {
|
||||
thread(isDaemon = true) {
|
||||
runCatching {
|
||||
server.accept().use { client ->
|
||||
peer = client
|
||||
val out = PrintWriter(client.getOutputStream(), true)
|
||||
val input = BufferedReader(InputStreamReader(client.getInputStream()))
|
||||
greeting?.let { out.print(it + "\r\n"); out.flush() }
|
||||
loginLine = input.readLine()
|
||||
chatter.forEach { out.print(it + "\r\n"); out.flush() }
|
||||
loginResponse?.let { out.print(it + "\r\n"); out.flush() }
|
||||
ready.countDown()
|
||||
// Read lines but never acknowledge, exactly as APRS-IS treats positions.
|
||||
while (true) {
|
||||
val line = input.readLine() ?: break
|
||||
synchronized(received) {
|
||||
received += line
|
||||
rawAfterLogin.append(line)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
ready.countDown()
|
||||
}
|
||||
}
|
||||
|
||||
fun awaitLogin(): Boolean = ready.await(5, TimeUnit.SECONDS)
|
||||
|
||||
fun lines(): List<String> = synchronized(received) { received.toList() }
|
||||
|
||||
/** Close the established connection, so the client is talking to a dead peer. */
|
||||
fun dropClient() {
|
||||
runCatching { peer?.close() }
|
||||
}
|
||||
|
||||
override fun close() {
|
||||
runCatching { server.close() }
|
||||
}
|
||||
}
|
||||
|
||||
private fun client(port: Int, passcode: Int = 12345) = AprsIsClient(
|
||||
host = "127.0.0.1",
|
||||
port = port,
|
||||
callsign = "TEST",
|
||||
ssid = "",
|
||||
passcode = passcode,
|
||||
version = "Look4Sat-test"
|
||||
)
|
||||
|
||||
/**
|
||||
* The regression that motivated this file. Two packets must arrive as two lines; without a
|
||||
* terminator they concatenate into one stream the server can never parse, while both sends
|
||||
* report success.
|
||||
*/
|
||||
@Test
|
||||
fun `each packet arrives as its own line`() {
|
||||
FakeServer().use { server ->
|
||||
server.start()
|
||||
val c = client(server.port)
|
||||
c.connect()
|
||||
assertTrue(server.awaitLogin())
|
||||
val first = c.sendPacket("TEST>APRS,TCPIP*:=0000.00N/00000.00E>one")
|
||||
val second = c.sendPacket("TEST>APRS,TCPIP*:=0000.00N/00000.00E>two")
|
||||
Thread.sleep(300)
|
||||
c.disconnect()
|
||||
|
||||
assertEquals(true, first?.first)
|
||||
assertEquals(true, second?.first)
|
||||
val lines = server.lines()
|
||||
assertEquals("both packets must reach the server as separate lines", 2, lines.size)
|
||||
assertTrue(lines[0].endsWith(">one"))
|
||||
assertTrue(lines[1].endsWith(">two"))
|
||||
}
|
||||
}
|
||||
|
||||
/** The login line has to be terminated too, or the server never reads it. */
|
||||
@Test
|
||||
fun `the server receives a complete login line`() {
|
||||
FakeServer().use { server ->
|
||||
server.start()
|
||||
val c = client(server.port)
|
||||
c.connect()
|
||||
assertTrue(server.awaitLogin())
|
||||
c.disconnect()
|
||||
assertEquals("user TEST pass 12345 vers Look4Sat-test", server.loginLine)
|
||||
}
|
||||
}
|
||||
|
||||
/** A verified login is recognised and lets reports count as delivered. */
|
||||
@Test
|
||||
fun `a verified login is not reported as refused`() {
|
||||
FakeServer().use { server ->
|
||||
server.start()
|
||||
val c = client(server.port)
|
||||
c.connect()
|
||||
assertTrue(server.awaitLogin())
|
||||
assertTrue(c.isVerified)
|
||||
assertFalse(c.isRefusedByServer)
|
||||
c.disconnect()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The case the rewrite exists for: the server accepts the connection, the write succeeds,
|
||||
* and every packet is discarded. The client must say so rather than report success.
|
||||
*/
|
||||
@Test
|
||||
fun `an unverified login is flagged while the connection stays up`() {
|
||||
FakeServer(loginResponse = "# logresp TEST unverified, server FAKE").use { server ->
|
||||
server.start()
|
||||
val c = client(server.port, passcode = -1)
|
||||
c.connect()
|
||||
assertTrue(server.awaitLogin())
|
||||
assertFalse("unverified must not read as verified", c.isVerified)
|
||||
assertTrue("the server explicitly refused", c.isRefusedByServer)
|
||||
// Not a connection error: a receive-only login is legitimate and stays connected.
|
||||
assertTrue(c.isConnected)
|
||||
c.disconnect()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A chatty server used to exhaust a fixed line budget, turning an accepted login into
|
||||
* Unknown and telling the operator their passcode was wrong when it had been accepted.
|
||||
*/
|
||||
@Test
|
||||
fun `keepalive chatter before the verdict does not hide it`() {
|
||||
val chatter = List(8) { "# keepalive $it" }
|
||||
FakeServer(chatter = chatter).use { server ->
|
||||
server.start()
|
||||
val c = client(server.port)
|
||||
c.connect()
|
||||
assertTrue(server.awaitLogin())
|
||||
assertTrue("the verdict must be found past the comments", c.isVerified)
|
||||
c.disconnect()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A server that sends no greeting is legitimate, and must not cost the full login window on
|
||||
* every connect - that was eight seconds per attempt.
|
||||
*/
|
||||
@Test
|
||||
fun `a server without a greeting connects promptly`() {
|
||||
FakeServer(greeting = null).use { server ->
|
||||
server.start()
|
||||
val c = client(server.port)
|
||||
val started = System.currentTimeMillis()
|
||||
c.connect()
|
||||
assertTrue(server.awaitLogin())
|
||||
val elapsed = System.currentTimeMillis() - started
|
||||
c.disconnect()
|
||||
assertTrue("connect took ${elapsed}ms, expected well under the login window",
|
||||
elapsed < 6_000)
|
||||
}
|
||||
}
|
||||
|
||||
/** Sending after the server has gone must report failure, not success. */
|
||||
@Test
|
||||
fun `a send after the server closes is reported as failed`() {
|
||||
val server = FakeServer()
|
||||
server.start()
|
||||
val c = client(server.port)
|
||||
c.connect()
|
||||
assertTrue(server.awaitLogin())
|
||||
// Closing the ServerSocket alone would leave this connection alive.
|
||||
server.dropClient()
|
||||
Thread.sleep(200)
|
||||
// The first write may still land in the socket buffer; by the second the loss is certain.
|
||||
c.sendPacket("TEST>APRS,TCPIP*:=0000.00N/00000.00E>one")
|
||||
val second = c.sendPacket("TEST>APRS,TCPIP*:=0000.00N/00000.00E>two")
|
||||
c.disconnect()
|
||||
assertEquals("a send on a dead connection must not report success", false, second?.first)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,116 @@
|
||||
/*
|
||||
* Look4Sat. Amateur radio satellite tracker and pass predictor.
|
||||
* Copyright (C) 2019-2026 Arty Bishop and contributors.
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU General Public License as published by
|
||||
* the Free Software Foundation, either version 3 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License
|
||||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
package com.rtbishop.look4sat.core.domain.aprs
|
||||
|
||||
/**
|
||||
* The APRS-IS login line and the verdict the server returns for it.
|
||||
*
|
||||
* Kept apart from the socket so it can be tested: whether a login was verified decides whether
|
||||
* anything this app sends reaches the network, and that distinction used to be made by a
|
||||
* substring check inside a runCatching whose result was discarded, so it could not fail loudly.
|
||||
*
|
||||
* Format per aprs-is.net/connecting.aspx:
|
||||
* `user mycall[-ss] pass passcode [vers softwarename softwarevers [filter ...]]`
|
||||
*/
|
||||
object AprsLogin {
|
||||
|
||||
/** What the server decided about a login attempt. */
|
||||
sealed interface Outcome {
|
||||
|
||||
/** The passcode matched the callsign. Packets from this client are accepted. */
|
||||
data class Verified(val callsign: String) : Outcome
|
||||
|
||||
/**
|
||||
* The server accepted the connection but did not verify the login.
|
||||
*
|
||||
* Not an error at the socket level, which is exactly why it needs surfacing: writes keep
|
||||
* succeeding while the server discards every packet. A receive-only login (passcode -1)
|
||||
* lands here legitimately.
|
||||
*/
|
||||
data class Unverified(val callsign: String) : Outcome
|
||||
|
||||
/** The server refused the login outright. */
|
||||
data class Rejected(val detail: String) : Outcome
|
||||
|
||||
/** Nothing recognisable arrived. The connection may still work; we simply do not know. */
|
||||
data class Unknown(val detail: String) : Outcome
|
||||
}
|
||||
|
||||
/** Passcode value that asks for a receive-only connection. */
|
||||
const val RECEIVE_ONLY_PASSCODE = -1
|
||||
|
||||
/**
|
||||
* Build the login line.
|
||||
*
|
||||
* [version] must not contain a space: the server splits `vers` into a software name and a
|
||||
* version, so an embedded space shifts every field after it. Any space becomes a hyphen
|
||||
* rather than silently corrupting the line.
|
||||
*/
|
||||
fun line(
|
||||
callsign: String,
|
||||
ssid: String,
|
||||
passcode: Int,
|
||||
version: String,
|
||||
filter: String = ""
|
||||
): String {
|
||||
val callSsid = AprsPacket.formatCallSsid(callsign, ssid)
|
||||
val safeVersion = version.trim().replace(' ', '-').ifEmpty { "Look4Sat" }
|
||||
val base = "user $callSsid pass $passcode vers $safeVersion"
|
||||
val trimmedFilter = filter.trim()
|
||||
return if (trimmedFilter.isEmpty()) base else "$base $trimmedFilter"
|
||||
}
|
||||
|
||||
/**
|
||||
* Interpret one line of server output, or null when it carries no verdict.
|
||||
*
|
||||
* Returning null for keepalives and identification comments lets the caller keep reading
|
||||
* rather than treating the first comment it sees as an answer.
|
||||
*
|
||||
* aprsc answers `# logresp CALL verified, server X` or `# logresp CALL unverified, server X`.
|
||||
* Note that "unverified" contains "verified", so the negative has to be tested first - a
|
||||
* naive contains("verified") reports every rejected login as accepted.
|
||||
*/
|
||||
fun parse(line: String): Outcome? {
|
||||
val trimmed = line.trim()
|
||||
if (trimmed.isEmpty()) return null
|
||||
if (!trimmed.startsWith("#")) {
|
||||
// A non-comment line during login is the server objecting in plain text.
|
||||
return Outcome.Rejected(trimmed)
|
||||
}
|
||||
val lower = trimmed.lowercase()
|
||||
if (!lower.contains("logresp")) {
|
||||
// Server identification and keepalive comments carry no verdict.
|
||||
return null
|
||||
}
|
||||
val callsign = callsignFrom(trimmed)
|
||||
return when {
|
||||
lower.contains("unverified") -> Outcome.Unverified(callsign)
|
||||
lower.contains("verified") -> Outcome.Verified(callsign)
|
||||
lower.contains("invalid") || lower.contains("error") -> Outcome.Rejected(trimmed)
|
||||
else -> Outcome.Unknown(trimmed)
|
||||
}
|
||||
}
|
||||
|
||||
/** The callsign token in `# logresp CALL verified, ...`, or empty when absent. */
|
||||
private fun callsignFrom(response: String): String {
|
||||
val tokens = response.removePrefix("#").trim().split(Regex("\\s+"))
|
||||
val index = tokens.indexOfFirst { it.equals("logresp", ignoreCase = true) }
|
||||
if (index < 0) return ""
|
||||
return tokens.getOrNull(index + 1)?.trimEnd(',') ?: ""
|
||||
}
|
||||
}
|
||||
@@ -22,12 +22,6 @@ object AprsPacket {
|
||||
return hash and 0x7FFF
|
||||
}
|
||||
|
||||
/** Login line: user CALL-SSID pass XXXX vers XXXX */
|
||||
fun formatLogin(callsign: String, ssid: String, passcode: Int, version: String): String {
|
||||
val callSsid = formatCallSsid(callsign, ssid)
|
||||
return "user $callSsid pass $passcode vers $version"
|
||||
}
|
||||
|
||||
/** Callsign-SSID join (BG7NTA + 5 -> BG7NTA-5) */
|
||||
fun formatCallSsid(callsign: String, ssid: String): String {
|
||||
if (ssid.isNullOrEmpty()) return callsign
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
package com.rtbishop.look4sat.core.domain.aprs
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* The verified/unverified distinction is the whole point of these tests: an unverified client
|
||||
* stays connected and its writes keep succeeding while the server discards every packet, so
|
||||
* getting this wrong means the operator watches reports "succeed" for hours with nothing landing.
|
||||
*/
|
||||
class AprsLoginTest {
|
||||
|
||||
@Test
|
||||
fun `builds the line the specification asks for`() {
|
||||
assertEquals(
|
||||
"user BG7NTA-5 pass 12345 vers Look4Sat-4.6.0",
|
||||
AprsLogin.line("BG7NTA", "5", 12345, "Look4Sat-4.6.0")
|
||||
)
|
||||
}
|
||||
|
||||
/** No SSID means no hyphen; the spec says never to write -0 explicitly. */
|
||||
@Test
|
||||
fun `omits the ssid when there is none`() {
|
||||
assertEquals(
|
||||
"user BG7NTA pass 12345 vers Look4Sat",
|
||||
AprsLogin.line("BG7NTA", "", 12345, "Look4Sat")
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* The server splits `vers` into name and version, so a space inside it shifts every field
|
||||
* after that point. The shipped value was "Look4Sat 4.5.4", which parsed correctly only by
|
||||
* luck - and only because nothing followed it.
|
||||
*/
|
||||
@Test
|
||||
fun `a space in the version cannot shift the following fields`() {
|
||||
val line = AprsLogin.line("BG7NTA", "5", 12345, "Look4Sat 4.6.0")
|
||||
assertEquals("user BG7NTA-5 pass 12345 vers Look4Sat-4.6.0", line)
|
||||
// Six tokens: user, callsign, pass, passcode, vers, version. A space inside the version
|
||||
// would make seven and push the server's field parsing one place along.
|
||||
assertEquals(6, line.split(" ").size)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `appends a filter when one is given`() {
|
||||
val line = AprsLogin.line("BG7NTA", "5", 12345, "Look4Sat", "r/33.25/-96.5/50")
|
||||
assertEquals("user BG7NTA-5 pass 12345 vers Look4Sat r/33.25/-96.5/50", line)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `receive-only logins use the documented passcode`() {
|
||||
val line = AprsLogin.line("BG7NTA", "", AprsLogin.RECEIVE_ONLY_PASSCODE, "Look4Sat")
|
||||
assertEquals("user BG7NTA pass -1 vers Look4Sat", line)
|
||||
}
|
||||
|
||||
/**
|
||||
* "unverified" contains "verified", so the negative has to be tested first. A naive
|
||||
* contains("verified") reports every rejected login as accepted - which is precisely the
|
||||
* failure this replaces.
|
||||
*/
|
||||
@Test
|
||||
fun `unverified is not mistaken for verified`() {
|
||||
val outcome = AprsLogin.parse("# logresp BG7NTA unverified, server T2SYDNEY")
|
||||
assertEquals(AprsLogin.Outcome.Unverified("BG7NTA"), outcome)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a verified login is recognised with its callsign`() {
|
||||
val outcome = AprsLogin.parse("# logresp BG7NTA-5 verified, server T2SYDNEY")
|
||||
assertEquals(AprsLogin.Outcome.Verified("BG7NTA-5"), outcome)
|
||||
}
|
||||
|
||||
/**
|
||||
* Server identification and keepalive comments must return null so the caller keeps reading.
|
||||
* Treating the first comment as an answer would leave every login Unknown.
|
||||
*/
|
||||
@Test
|
||||
fun `comments without a verdict are skipped`() {
|
||||
assertNull(AprsLogin.parse("# aprsc 2.1.19-g730c5c0"))
|
||||
assertNull(AprsLogin.parse("# Tue Aug 25 08:00:00 UTC 2026"))
|
||||
assertNull(AprsLogin.parse(""))
|
||||
assertNull(AprsLogin.parse(" "))
|
||||
}
|
||||
|
||||
/** A non-comment line during login is the server objecting in plain text. */
|
||||
@Test
|
||||
fun `a plain text line during login is a rejection`() {
|
||||
val outcome = AprsLogin.parse("Invalid callsign format")
|
||||
assertTrue(outcome is AprsLogin.Outcome.Rejected)
|
||||
assertEquals("Invalid callsign format", (outcome as AprsLogin.Outcome.Rejected).detail)
|
||||
}
|
||||
|
||||
/** A logresp with no recognisable verdict is Unknown, not silently Verified. */
|
||||
@Test
|
||||
fun `an unrecognised logresp is not treated as success`() {
|
||||
val outcome = AprsLogin.parse("# logresp BG7NTA something-new, server X")
|
||||
assertTrue(outcome is AprsLogin.Outcome.Unknown)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `case does not change the verdict`() {
|
||||
assertEquals(
|
||||
AprsLogin.Outcome.Unverified("BG7NTA"),
|
||||
AprsLogin.parse("# LOGRESP BG7NTA UNVERIFIED, server X")
|
||||
)
|
||||
assertEquals(
|
||||
AprsLogin.Outcome.Verified("BG7NTA"),
|
||||
AprsLogin.parse("# LogResp BG7NTA Verified, server X")
|
||||
)
|
||||
}
|
||||
|
||||
/** The callsign is read back so the UI can show whose login the server acknowledged. */
|
||||
@Test
|
||||
fun `reads the callsign even when the line is oddly spaced`() {
|
||||
assertEquals(
|
||||
AprsLogin.Outcome.Verified("BG7NTA-9"),
|
||||
AprsLogin.parse("# logresp BG7NTA-9 verified, server X")
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -38,6 +38,7 @@
|
||||
<string name="prefs_aprs_calc_passcode">Hitung kode sandi</string>
|
||||
<string name="aprs_toast_ok">APRS: laporan terkirim</string>
|
||||
<string name="aprs_toast_fail">APRS: laporan gagal - %1$s</string>
|
||||
<string name="aprs_toast_unverified">APRS-IS tidak memverifikasi passcode Anda. Laporan Anda tidak diteruskan - periksa tanda panggil dan passcode di Pengaturan.</string>
|
||||
<string name="aprs_toast_not_configured">APRS belum dikonfigurasi - isi panggilan dulu</string>
|
||||
<string name="prefs_aprs_last_report">Laporan terakhir: %1$s %2$s</string>
|
||||
<string name="prefs_aprs_last_ok">OK</string>
|
||||
|
||||
@@ -38,6 +38,7 @@
|
||||
<string name="prefs_aprs_calc_passcode">Hitung kode sandi</string>
|
||||
<string name="aprs_toast_ok">APRS: laporan terkirim</string>
|
||||
<string name="aprs_toast_fail">APRS: laporan gagal - %1$s</string>
|
||||
<string name="aprs_toast_unverified">APRS-IS tidak memverifikasi passcode Anda. Laporan Anda tidak diteruskan - periksa tanda panggil dan passcode di Pengaturan.</string>
|
||||
<string name="aprs_toast_not_configured">APRS belum dikonfigurasi - isi panggilan dulu</string>
|
||||
<string name="prefs_aprs_last_report">Laporan terakhir: %1$s %2$s</string>
|
||||
<string name="prefs_aprs_last_ok">OK</string>
|
||||
|
||||
@@ -39,6 +39,7 @@
|
||||
<string name="prefs_aprs_calc_passcode">Parolayı hesapla</string>
|
||||
<string name="aprs_toast_ok">APRS: rapor gönderildi</string>
|
||||
<string name="aprs_toast_fail">APRS: rapor başarısız - %1$s</string>
|
||||
<string name="aprs_toast_unverified">APRS-IS passcode bilginizi doğrulamadı. Raporlarınız iletilmiyor - Ayarlar bölümünde çağrı işareti ve passcode bilgisini kontrol edin.</string>
|
||||
<string name="aprs_toast_not_configured">APRS yapılandırılmadı - önce çağrı girin</string>
|
||||
<string name="prefs_aprs_last_report">Son rapor: %1$s %2$s</string>
|
||||
<string name="prefs_aprs_last_ok">OK</string>
|
||||
|
||||
@@ -40,6 +40,7 @@
|
||||
<string name="prefs_aprs_calc_passcode">计算验证码</string>
|
||||
<string name="aprs_toast_ok">APRS: 上报成功</string>
|
||||
<string name="aprs_toast_fail">APRS: 上报失败 - %1$s</string>
|
||||
<string name="aprs_toast_unverified">APRS-IS 未验证你的 passcode, 上报不会被转发 - 请在设置里检查呼号与 passcode。</string>
|
||||
<string name="aprs_toast_not_configured">APRS 未配置,请先填呼号</string>
|
||||
<string name="prefs_aprs_last_report">上次上报: %1$s %2$s</string>
|
||||
<string name="prefs_aprs_last_ok">成功</string>
|
||||
|
||||
@@ -41,6 +41,7 @@
|
||||
<string name="prefs_aprs_calc_passcode">Compute passcode</string>
|
||||
<string name="aprs_toast_ok">APRS: report sent OK</string>
|
||||
<string name="aprs_toast_fail">APRS: report failed - %1$s</string>
|
||||
<string name="aprs_toast_unverified">APRS-IS did not verify your passcode. Your reports are not being passed on - check the callsign and passcode in Settings.</string>
|
||||
<string name="aprs_toast_not_configured">APRS not configured - set callsign first</string>
|
||||
<string name="prefs_aprs_last_report">Last report: %1$s %2$s</string>
|
||||
<string name="prefs_aprs_last_ok">OK</string>
|
||||
|
||||
Reference in new issue
Block a user