diff --git a/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt b/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt index 472f2bff..4e11d22b 100644 --- a/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt +++ b/app/src/main/java/com/rtbishop/look4sat/AprsForegroundService.kt @@ -11,6 +11,8 @@ import android.content.SharedPreferences import android.widget.Toast import android.content.pm.ServiceInfo import android.os.Build +import android.os.Handler +import android.os.Looper import android.os.IBinder import com.rtbishop.look4sat.MainApplication import com.rtbishop.look4sat.core.presentation.R @@ -39,6 +41,16 @@ class AprsForegroundService : Service() { private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) private var reporter: AprsReporter? = null + + /** + * Handler on the main looper, for anything that must not run on the reporter's IO thread. + * + * onReport is invoked from AprsReporter's Dispatchers.IO scope, and Toast construction there + * throws because that thread has no Looper - an exception the surrounding runCatching then + * swallowed, so every report notice was silently discarded. The messages existed and no + * operator ever saw one. + */ + private val mainHandler = Handler(Looper.getMainLooper()) private var lastState: AprsState = AprsState.Idle override fun onBind(intent: Intent?): IBinder? = null @@ -99,7 +111,7 @@ class AprsForegroundService : Service() { !report.verified -> getString(R.string.aprs_toast_unverified) else -> getString(R.string.aprs_toast_fail, report.detail) } - runCatching { + mainHandler.post { Toast.makeText(this, msg, if (report.ok) Toast.LENGTH_SHORT else Toast.LENGTH_LONG).show() } diff --git a/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsReporter.kt b/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsReporter.kt index 669e27d5..50666e2d 100644 --- a/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsReporter.kt +++ b/core/data/src/main/java/com/rtbishop/look4sat/core/data/aprs/AprsReporter.kt @@ -1,6 +1,7 @@ package com.rtbishop.look4sat.core.data.aprs import com.rtbishop.look4sat.core.domain.aprs.AprsPacket +import com.rtbishop.look4sat.core.domain.aprs.AprsPasscode import com.rtbishop.look4sat.core.domain.aprs.AprsPosition import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -101,7 +102,9 @@ class AprsReporter( port = cfg.port, callsign = cfg.callsign, ssid = cfg.ssid, - passcode = cfg.passcode.toIntOrNull()?.takeIf { it >= 0 } ?: AprsPacket.passcode(cfg.callsign), + // Never derives one: a blank or wrong entry logs in receive-only rather than + // transmitting under a passcode the app invented for an unchecked licence. + passcode = AprsPasscode.loginValue(cfg.callsign, cfg.passcode), version = "Look4Sat 4.5.4" ).also { client = it } if (!c.isConnected) c.connect() diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsPasscode.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsPasscode.kt new file mode 100644 index 00000000..7318b2ab --- /dev/null +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/aprs/AprsPasscode.kt @@ -0,0 +1,101 @@ +/* + * 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 . + */ +package com.rtbishop.look4sat.core.domain.aprs + +/** + * Decides what passcode to present to APRS-IS, and whether the operator's entry is usable. + * + * The app must not derive a transmit passcode for the operator. APRS-IS states that supplying + * the correct passcode to a user is the software author's responsibility, and the passcode + * exists as a licence check - deriving it in-app and shipping the algorithm defeats the point. + * APRSdroid has the same algorithm in the same file and deliberately does not use it for this + * reason, validating the operator's entry instead and linking out to request one. + * + * So this validates. [AprsPacket.passcode] stays, because checking an entry means recomputing + * the expected value, but nothing here substitutes a derived code for a missing one. + */ +object AprsPasscode { + + /** Value that asks APRS-IS for a receive-only connection. Always legitimate. */ + const val RECEIVE_ONLY = -1 + + /** What the operator's passcode entry amounts to. */ + sealed interface Entry { + + /** A passcode that matches the callsign. Reports will be forwarded. */ + data class Transmit(val passcode: Int) : Entry + + /** + * An explicit -1, or a blank entry. + * + * A blank entry lands here rather than being filled in with a derived code: connecting + * receive-only is honest about what an operator without a passcode can do, where a + * derived code silently claims a licence check that was never performed. + */ + data object ReceiveOnly : Entry + + /** Something was typed but it is not this callsign's passcode. */ + data class Mismatch(val expectedFor: String) : Entry + + /** Something was typed that is not a number at all. */ + data object NotANumber : Entry + } + + /** + * Classify what the operator typed. + * + * A mismatch is reported rather than corrected, so the UI can refuse to save and say why. + * Silently swapping in a derived code is how an operator ends up believing they are + * transmitting under a passcode they never obtained. + */ + fun classify(callsign: String, entry: String): Entry { + val trimmed = entry.trim() + if (trimmed.isEmpty()) return Entry.ReceiveOnly + val value = trimmed.toIntOrNull() ?: return Entry.NotANumber + // Checked before the callsign comparison: -1 is the documented receive-only value and + // is never anyone's passcode, so comparing it would report a deliberate choice as a typo. + if (value == RECEIVE_ONLY) return Entry.ReceiveOnly + val call = callsign.trim() + if (call.isEmpty()) return Entry.Mismatch("") + return if (value == AprsPacket.passcode(call)) { + Entry.Transmit(value) + } else { + Entry.Mismatch(call.uppercase()) + } + } + + /** + * The number to send in the login line for this entry. + * + * Anything not usable becomes [RECEIVE_ONLY]: the connection still works, the operator is + * told separately that their reports are not being forwarded, and no packet goes out under + * a passcode the app invented. The previous code sent a derived transmit passcode here, + * and `takeIf { it >= 0 }` additionally made an explicit -1 impossible to use - which also + * blocked the one safe way to test a setup, since a receive-only login is how you confirm + * the connection works without putting anything on the network. + */ + fun loginValue(callsign: String, entry: String): Int = + when (val classified = classify(callsign, entry)) { + is Entry.Transmit -> classified.passcode + else -> RECEIVE_ONLY + } + + /** True when this entry lets the operator's reports reach the network. */ + fun canTransmit(callsign: String, entry: String): Boolean = + classify(callsign, entry) is Entry.Transmit +} diff --git a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsPasscodeTest.kt b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsPasscodeTest.kt new file mode 100644 index 00000000..c32f65ec --- /dev/null +++ b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/aprs/AprsPasscodeTest.kt @@ -0,0 +1,104 @@ +package com.rtbishop.look4sat.core.domain.aprs + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The rule these pin down: the app never invents a transmit passcode. + * + * APRS-IS treats the passcode as a licence check and says supplying it to a user is the software + * author's job. The previous code derived one from the callsign whenever the entry was blank or + * unparseable, so an operator who had never obtained a passcode transmitted anyway - under a + * check that was never performed. + */ +class AprsPasscodeTest { + + /** Recomputed rather than hardcoded: the point is agreement with the shipped algorithm. */ + private val callsign = "BG7NTA" + private val correct = AprsPacket.passcode(callsign) + + @Test + fun `the callsign's own passcode allows transmitting`() { + assertEquals( + AprsPasscode.Entry.Transmit(correct), + AprsPasscode.classify(callsign, correct.toString()) + ) + assertTrue(AprsPasscode.canTransmit(callsign, correct.toString())) + assertEquals(correct, AprsPasscode.loginValue(callsign, correct.toString())) + } + + /** + * A blank entry becomes receive-only rather than a derived code. This is the defect: the old + * line was `cfg.passcode.toIntOrNull()?.takeIf { it >= 0 } ?: AprsPacket.passcode(callsign)`, + * so leaving the field empty transmitted under a computed passcode. + */ + @Test + fun `a blank entry connects receive-only instead of deriving one`() { + assertEquals(AprsPasscode.Entry.ReceiveOnly, AprsPasscode.classify(callsign, "")) + assertEquals(AprsPasscode.Entry.ReceiveOnly, AprsPasscode.classify(callsign, " ")) + assertEquals(AprsPasscode.RECEIVE_ONLY, AprsPasscode.loginValue(callsign, "")) + assertFalse(AprsPasscode.canTransmit(callsign, "")) + } + + /** + * -1 is the documented receive-only value and has to survive. `takeIf { it >= 0 }` used to + * discard it and substitute a transmit passcode, which removed the only safe way to test a + * setup - connecting receive-only puts nothing on the network. + */ + @Test + fun `an explicit -1 stays receive-only`() { + assertEquals(AprsPasscode.Entry.ReceiveOnly, AprsPasscode.classify(callsign, "-1")) + assertEquals(AprsPasscode.RECEIVE_ONLY, AprsPasscode.loginValue(callsign, "-1")) + assertEquals(AprsPasscode.RECEIVE_ONLY, AprsPasscode.loginValue(callsign, " -1 ")) + } + + /** A wrong passcode is reported, not quietly corrected to the right one. */ + @Test + fun `a mismatched passcode is reported rather than fixed`() { + val wrong = (correct + 1).toString() + assertEquals(AprsPasscode.Entry.Mismatch("BG7NTA"), AprsPasscode.classify(callsign, wrong)) + assertFalse(AprsPasscode.canTransmit(callsign, wrong)) + // And it must not be sent: a mismatch logs in receive-only, never as the derived value. + assertEquals(AprsPasscode.RECEIVE_ONLY, AprsPasscode.loginValue(callsign, wrong)) + } + + @Test + fun `something that is not a number is its own case`() { + assertEquals(AprsPasscode.Entry.NotANumber, AprsPasscode.classify(callsign, "abcde")) + assertEquals(AprsPasscode.Entry.NotANumber, AprsPasscode.classify(callsign, "12a45")) + assertEquals(AprsPasscode.RECEIVE_ONLY, AprsPasscode.loginValue(callsign, "abcde")) + } + + /** Surrounding whitespace from a paste must not turn a valid passcode into a mismatch. */ + @Test + fun `whitespace around a valid passcode is tolerated`() { + assertTrue(AprsPasscode.canTransmit(callsign, " $correct ")) + } + + /** The passcode depends on the callsign, so it must not validate against a different one. */ + @Test + fun `a passcode for one callsign does not validate another`() { + assertFalse(AprsPasscode.canTransmit("W1AW", correct.toString())) + assertEquals( + AprsPasscode.Entry.Mismatch("W1AW"), + AprsPasscode.classify("W1AW", correct.toString()) + ) + } + + /** Case must not matter: the algorithm upper-cases before hashing. */ + @Test + fun `a lower case callsign validates the same passcode`() { + assertTrue(AprsPasscode.canTransmit("bg7nta", correct.toString())) + } + + /** The SSID is not part of the hash, so the base callsign's code is the right one. */ + @Test + fun `no callsign cannot transmit`() { + assertEquals(AprsPasscode.Entry.Mismatch(""), AprsPasscode.classify("", "12345")) + assertEquals(AprsPasscode.RECEIVE_ONLY, AprsPasscode.loginValue("", "12345")) + // A blank entry with a blank callsign is still just receive-only, not a mismatch. + assertEquals(AprsPasscode.Entry.ReceiveOnly, AprsPasscode.classify("", "")) + } +}