From 0c653418a67fe960bb3126559fe77833faaa385c Mon Sep 17 00:00:00 2001 From: Lena Date: Sun, 23 Aug 2026 00:00:00 +0000 Subject: app: remove duplicated and dead code upgradeLegacy assembled every remote, folder and schedule twice: once as Remote and Folder objects to validate, and again as a parallel JSON tree to return. Build the objects and serialise them, and drop the dead endpointAllowed branch, the redundant pin emptiness test, and the excludes length pre-check that excludesAllowed already makes. atomicWrite existed verbatim in Config and Keys and was open-coded a third time in LastSync, which then reached into Config's companion for the bounded reader. The same bounded read was open-coded twice more, for the encrypted key and for an imported one. Both are plain file operations on the same private directory and belong in one place, with readText a thin wrapper over readBytes. Generating and importing an identity key ran the same twenty lines of worker thread, error capture, refresh, toast and dialog; only the action and two strings differed. rotateIfBig rotated at length >= MAX_BYTES, which line() already subsumes, and its only caller invoked it immediately before a line(). notifyError and notifyDeleted were the same builder twice. openSettings nested a try/catch and repeated one toast; take the candidates as a vararg. RemoteActivity.save defaulted its argument to current(), which validates and writes the status field, so the default hid that side effect from one of two callers. Dead on arrival: Outcome.deleted's default, which no caller omits; the rsa-sha2-* arms of hostKeyFile, since rsh prints PublicKey.Type() and an RSA host key is always ssh-rsa; and the limit <= 0 branch in SyncLog.text, which tail already handles and handled more accurately. The config recovery note is now written with an explicit charset like every other write here. --- app/src/main/java/invalid/lena/rsend/Config.kt | 147 ++++++--------------- app/src/main/java/invalid/lena/rsend/Files.kt | 39 ++++++ .../java/invalid/lena/rsend/FolderEditActivity.kt | 7 +- .../invalid/lena/rsend/FolderPickerActivity.kt | 3 +- app/src/main/java/invalid/lena/rsend/Keys.kt | 32 +---- app/src/main/java/invalid/lena/rsend/LastSync.kt | 12 +- .../main/java/invalid/lena/rsend/MainActivity.kt | 110 ++++++--------- .../main/java/invalid/lena/rsend/RemoteActivity.kt | 8 +- app/src/main/java/invalid/lena/rsend/Rsync.kt | 2 +- app/src/main/java/invalid/lena/rsend/SyncLog.kt | 13 +- app/src/main/java/invalid/lena/rsend/SyncWorker.kt | 44 +++--- 11 files changed, 153 insertions(+), 264 deletions(-) create mode 100644 app/src/main/java/invalid/lena/rsend/Files.kt (limited to 'app/src/main/java/invalid') diff --git a/app/src/main/java/invalid/lena/rsend/Config.kt b/app/src/main/java/invalid/lena/rsend/Config.kt index a38fbde..701ce66 100644 --- a/app/src/main/java/invalid/lena/rsend/Config.kt +++ b/app/src/main/java/invalid/lena/rsend/Config.kt @@ -4,10 +4,8 @@ import android.content.Context import android.util.AtomicFile import org.json.JSONArray import org.json.JSONObject -import java.io.ByteArrayOutputStream import java.io.File import java.io.FileNotFoundException -import java.io.InputStream import java.util.Base64 // Config is rsend's whole state: the named remote targets, the schedule, and @@ -141,7 +139,7 @@ data class Config( internal const val MAX_CONFIG_BYTES = 1024 * 1024 private const val MAX_REMOTES = 64 private const val MAX_FOLDERS = 256 - fun file(ctx: Context): File = File(ctx.filesDir, "config.json") + private fun file(ctx: Context): File = File(ctx.filesDir, "config.json") private fun recoveryFile(ctx: Context): File = File(ctx.filesDir, "config-recovery.txt") private fun legacyFile(ctx: Context): File = File(ctx.filesDir, "config.json.0.1") @@ -172,22 +170,13 @@ data class Config( } } - // AtomicFile keeps the previous complete config if a write is interrupted. @Synchronized fun save(ctx: Context, c: Config) { val json = c.toJson() fromJson(json) val bytes = json.toString(2).toByteArray(Charsets.UTF_8) require(bytes.size <= MAX_CONFIG_BYTES) { "config exceeds $MAX_CONFIG_BYTES bytes" } - val file = AtomicFile(file(ctx)) - val out = file.startWrite() - try { - out.write(bytes) - file.finishWrite(out) - } catch (e: Exception) { - file.failWrite(out) - throw e - } + atomicWrite(file(ctx), bytes) } // overLimit reports why c cannot be persisted, or null. These are the @@ -213,9 +202,9 @@ data class Config( // remote becomes the sole entry named after its host, every retained // safe folder points at it, and the folder's old "remote" field was the // destination path. pin is the known_hosts line 0.1.x kept in a separate - // file; it is carried over only if it still describes this host and port, so a - // pin that no longer matches costs one Test connection rather than the - // whole config. + // file; it is carried over only if it still describes this host and + // port, so a pin that no longer matches costs one Test connection rather + // than the whole config. // // This is a deliberate, documented exception to the no-compatibility // rule: 0.1.3 is the last published release, so it is what every @@ -226,81 +215,51 @@ data class Config( val host = old.optString("host").trim().removeSurrounding("[", "]").trim() val port = old.optInt("port", 22) val user = old.optString("user") - val candidate = Remote( - name = host, - host = host, - port = port, - user = user, - hostKey = pin, - ) - val endpointAllowed = RemoteRules.nameAllowed(host) && - RemoteRules.hostAllowed(host) && RemoteRules.userAllowed(user) && port in 1..65535 - val remote = JSONObject() - .put("name", host) - .put("host", host) - .put("port", port) - .put("user", user) - .put("hostKey", "") - if (endpointAllowed && RemoteRules.hostKeyAllowed(candidate) && pin.isNotEmpty()) { - remote.put("hostKey", pin) + val remote = Remote(name = host, host = host, port = port, user = user, hostKey = pin) + val endpointAllowed = RemoteRules.nameAllowed(host) && RemoteRules.hostAllowed(host) && + RemoteRules.userAllowed(user) && port in 1..65535 + val remotes = when { + !endpointAllowed -> emptyList() + RemoteRules.hostKeyAllowed(remote) -> listOf(remote) + else -> listOf(remote.copy(hostKey = "")) } - val folders = JSONArray() - val accepted = ArrayList() + val folders = ArrayList() val fa = o.optJSONArray("folders") ?: JSONArray() for (i in 0 until fa.length()) { - if (!endpointAllowed || accepted.size >= MAX_FOLDERS) break - val f = fa.getJSONObject(i) - val oldExcludes = f.optJSONArray("excludes") ?: JSONArray() - if (oldExcludes.length() > FolderRules.MAX_EXCLUDES) continue - val excludes = JSONArray() - for (j in 0 until oldExcludes.length()) { - excludes.put(oldExcludes.getString(j)) - } - val folder = Folder( - name = f.optString("name"), - local = f.optString("local"), + if (remotes.isEmpty() || folders.size >= MAX_FOLDERS) break + val fo = fa.getJSONObject(i) + val ex = fo.optJSONArray("excludes") ?: JSONArray() + val f = Folder( + name = fo.optString("name"), + local = fo.optString("local"), remoteName = host, - remotePath = f.optString("remote"), - delete = f.optBoolean("delete", false), - excludes = (0 until excludes.length()).map(excludes::getString), - ) - val trial = Config( - remotes = if (endpointAllowed) listOf(candidate.copy(hostKey = "")) else emptyList(), - folders = accepted, + remotePath = fo.optString("remote"), + delete = fo.optBoolean("delete", false), + excludes = (0 until ex.length()).map(ex::getString), ) - if (FolderRules.nameAllowed(folder.name) && - FolderRules.localPathAllowed(folder.local) && - FolderRules.remotePathAllowed(folder.remotePath) && - FolderRules.excludesAllowed(folder.excludes) && - FolderRules.destinationConflict(trial, folder, -1) == null + if (FolderRules.nameAllowed(f.name) && + FolderRules.localPathAllowed(f.local) && + FolderRules.remotePathAllowed(f.remotePath) && + FolderRules.excludesAllowed(f.excludes) && + FolderRules.destinationConflict(Config(remotes = remotes, folders = folders), f, -1) == null ) { - accepted.add(folder) - folders.put( - JSONObject() - .put("name", folder.name) - .put("local", folder.local) - .put("remoteName", host) - .put("remotePath", folder.remotePath) - .put("delete", folder.delete) - .put("excludes", excludes), - ) + folders.add(f) } } - val oldSchedule = o.optJSONObject("schedule") ?: JSONObject() - val schedule = JSONObject() - .put("enabled", oldSchedule.optBoolean("enabled", false)) - .put( - "intervalMinutes", - oldSchedule.optInt("intervalMinutes", 120) + + val s = o.optJSONObject("schedule") ?: JSONObject() + return Config( + remotes = remotes, + schedule = Schedule( + enabled = s.optBoolean("enabled", false), + intervalMinutes = s.optInt("intervalMinutes", 120) .coerceAtLeast(Schedule.MIN_INTERVAL_MINUTES), - ) - .put("wifiOnly", oldSchedule.optBoolean("wifiOnly", true)) - .put("requireCharging", oldSchedule.optBoolean("requireCharging", false)) - return JSONObject() - .put("remotes", if (endpointAllowed) JSONArray().put(remote) else JSONArray()) - .put("schedule", schedule) - .put("folders", folders) + wifiOnly = s.optBoolean("wifiOnly", true), + requireCharging = s.optBoolean("requireCharging", false), + ), + folders = folders, + ).toJson() } private fun preserveLegacy(ctx: Context, text: String, old: JSONObject, migrated: Config) { @@ -445,7 +404,7 @@ data class Config( val message = "Configuration $what and was reset. The original file was preserved as ${broken.name}. " + "Reason: $detail" - atomicWrite(recoveryFile(ctx), (message + "\n").toByteArray()) + atomicWrite(recoveryFile(ctx), (message + "\n").toByteArray(Charsets.UTF_8)) return Config() } @@ -462,29 +421,5 @@ data class Config( } return target } - - internal fun readText(input: InputStream, limit: Int): String { - val out = ByteArrayOutputStream(minOf(limit, 8192)) - val buf = ByteArray(4096) - while (true) { - val n = input.read(buf) - if (n < 0) break - if (out.size() + n > limit) throw IllegalArgumentException("file exceeds $limit bytes") - out.write(buf, 0, n) - } - return String(out.toByteArray(), Charsets.UTF_8) - } - - private fun atomicWrite(f: File, bytes: ByteArray) { - val file = AtomicFile(f) - val out = file.startWrite() - try { - out.write(bytes) - file.finishWrite(out) - } catch (e: Exception) { - file.failWrite(out) - throw e - } - } } } diff --git a/app/src/main/java/invalid/lena/rsend/Files.kt b/app/src/main/java/invalid/lena/rsend/Files.kt new file mode 100644 index 0000000..60cfdf5 --- /dev/null +++ b/app/src/main/java/invalid/lena/rsend/Files.kt @@ -0,0 +1,39 @@ +package invalid.lena.rsend + +import android.util.AtomicFile +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.InputStream + +// Whole-file reads and writes of rsend's private state: the config, the +// encrypted identity key, the derived known_hosts, and the last sync status. + +// AtomicFile keeps the previous complete file if a write is interrupted. +internal fun atomicWrite(f: File, bytes: ByteArray) { + val file = AtomicFile(f) + val out = file.startWrite() + try { + out.write(bytes) + file.finishWrite(out) + } catch (e: Exception) { + file.failWrite(out) + throw e + } +} + +// Nothing guarantees a file on disk is still the one rsend wrote, so stop at +// limit rather than buffer whatever is there. +internal fun readBytes(input: InputStream, limit: Int): ByteArray { + val out = ByteArrayOutputStream(minOf(limit, 8192)) + val buf = ByteArray(4096) + while (true) { + val n = input.read(buf) + if (n < 0) break + if (out.size() + n > limit) throw IllegalArgumentException("file exceeds $limit bytes") + out.write(buf, 0, n) + } + return out.toByteArray() +} + +internal fun readText(input: InputStream, limit: Int): String = + String(readBytes(input, limit), Charsets.UTF_8) diff --git a/app/src/main/java/invalid/lena/rsend/FolderEditActivity.kt b/app/src/main/java/invalid/lena/rsend/FolderEditActivity.kt index 3024a52..7571508 100644 --- a/app/src/main/java/invalid/lena/rsend/FolderEditActivity.kt +++ b/app/src/main/java/invalid/lena/rsend/FolderEditActivity.kt @@ -64,11 +64,10 @@ class FolderEditActivity : AppCompatActivity() { excludes.setText(f.excludes.joinToString(", ")) delete.isChecked = f.delete - val names = cfg.remotes.map { it.name }.toMutableList() - remoteNames = names + remoteNames = cfg.remotes.map { it.name } remoteName.adapter = - ArrayAdapter(this, android.R.layout.simple_spinner_dropdown_item, names) - val sel = names.indexOf(f.remoteName) + ArrayAdapter(this, android.R.layout.simple_spinner_dropdown_item, remoteNames) + val sel = remoteNames.indexOf(f.remoteName) if (sel >= 0) remoteName.setSelection(sel) findViewById