From 17609acc1e22b1dca47f506309e908d38a1f6b58 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Wed, 12 Aug 2026 19:01:24 -0400 Subject: [PATCH] fix: keep the LNURL body read off the UI thread and bound its size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SendDialog's two LNURL effects returned the Response out of their withContext(Dispatchers.IO) block and then called response.body.string() outside it. body.string() is a blocking socket read and LaunchedEffect resumes on the composition dispatcher, so the read ran on the EDT — the withContext gave the appearance of off-thread IO while doing almost none of it, since the cost of a request is mostly the body transfer. A slow or hostile LNURL server froze the wallet dialog. The Response was also never closed with use { }. On the happy path body.string() closes the source itself, but both effects are keyed on sendState and restart on every state change, so a cancellation between the headers arriving and the body read leaked the connection. Both effects now share fetchLnurlJson(), which does the request, the capped body read and the parse inside one withContext(Dispatchers.IO) and always closes the Response. Adds the two missing guards: - Size cap. LUD-06/LUD-16 documents are a few hundred bytes and the body is buffered in memory, so it is capped at 64 KiB. Note this does NOT copy Nip11Fetcher's pattern, which does not work: okio's readUtf8() is buffer.writeAll(source) + readUtf8(), and writeAll drains the entire upstream, so request(MAX) only pre-buffers and never limits the read. With a 1 MB source and a 1 KB "cap" that pattern returns all 1,000,000 bytes. Reading from source.buffer after request(MAX + 1) caps for real. Nip11Fetcher has the same latent bug and is left for a separate change. - Status code. Not a hard isSuccessful gate: LUD-06 servers report failures as HTTP 200 + {"status":"ERROR","reason":...} and some use 4xx with a usable reason body, so throwing before parsing would discard the server's message. The body is parsed first and the status is surfaced only when the body is not usable JSON — which is the HTML-error-page case that previously produced a raw Jackson error. Co-Authored-By: Claude Opus 5 (1M context) --- .../desktop/ui/wallet/WalletColumnScreen.kt | 74 +++++++++++++------ 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/wallet/WalletColumnScreen.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/wallet/WalletColumnScreen.kt index 90fd06b871..457046879e 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/wallet/WalletColumnScreen.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/wallet/WalletColumnScreen.kt @@ -62,6 +62,8 @@ import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import androidx.compose.ui.window.Dialog +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.vitorpamplona.amethyst.commons.icons.symbols.Icon import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols @@ -78,10 +80,16 @@ import com.vitorpamplona.quartz.lightning.LnInvoiceUtil import com.vitorpamplona.quartz.lightning.Lud06 import com.vitorpamplona.quartz.nip47WalletConnect.Nip47WalletConnect.Nip47URINorm import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.coroutines.executeAsync import java.awt.Toolkit import java.awt.datatransfer.DataFlavor import java.awt.datatransfer.StringSelection +import java.io.IOException import java.text.NumberFormat import java.util.Locale @@ -505,6 +513,46 @@ private fun classifyAndProcess(input: String): SendState { return SendState.Idle } +/** + * Max bytes read from an LNURL endpoint. LUD-06 payRequest and LUD-16 invoice documents are a few + * hundred bytes; the body is buffered in memory, so a hostile or broken server needs a hard ceiling. + */ +private const val MAX_LNURL_RESPONSE_BYTES = 64 * 1024L + +/** + * GETs [url] and parses its size-capped JSON body. + * + * Connect, body read and parse all happen on [Dispatchers.IO] and the response is always closed: + * callers run inside a `LaunchedEffect` that resumes on the composition dispatcher and can be + * cancelled at any suspension point, so neither the blocking read nor the close can be left to them. + * + * LUD-06 servers report failures as HTTP 200 + `{"status":"ERROR","reason":…}`, so the body is + * parsed first and the status code is only surfaced when the body isn't usable JSON — otherwise + * the caller's `reason`/`message` extraction would be skipped for servers that use 4xx instead. + */ +private suspend fun fetchLnurlJson( + httpClient: OkHttpClient, + mapper: ObjectMapper, + url: String, +): JsonNode = + withContext(Dispatchers.IO) { + val request = Request.Builder().url(url).build() + httpClient.newCall(request).executeAsync().use { response -> + val source = response.body.source() + // request() buffers up to N bytes and stops, but readUtf8() on the SOURCE would drain + // the whole stream regardless of what was buffered — so read the BUFFER to cap for real. + source.request(MAX_LNURL_RESPONSE_BYTES + 1) + if (source.buffer.size > MAX_LNURL_RESPONSE_BYTES) { + throw IOException("LNURL response exceeds $MAX_LNURL_RESPONSE_BYTES bytes") + } + val body = source.buffer.readUtf8() + runCatching { mapper.readTree(body) }.getOrNull() + ?: throw IOException( + if (response.isSuccessful) "malformed LNURL response" else "HTTP ${response.code}", + ) + } + } + @Composable private fun SendDialog( onDismiss: () -> Unit, @@ -529,18 +577,7 @@ private fun SendDialog( val state = sendState if (state is SendState.Resolving) { try { - val httpClient = DesktopHttpClient.currentClient() - val request = - okhttp3.Request - .Builder() - .url(state.url) - .build() - val response = - kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.IO) { - httpClient.newCall(request).execute() - } - val body = response.body.string() - val json = mapper.readTree(body) + val json = fetchLnurlJson(DesktopHttpClient.currentClient(), mapper, state.url) val callback = json.get("callback")?.asText()?.ifBlank { null } if (callback == null) { val errorMsg = json.get("reason")?.asText() ?: json.get("message")?.asText() ?: "Invalid LNURL endpoint" @@ -569,21 +606,10 @@ private fun SendDialog( val state = sendState if (state is SendState.FetchingInvoice) { try { - val httpClient = DesktopHttpClient.currentClient() val urlBinder = if (state.callbackUrl.contains("?")) "&" else "?" val encodedComment = java.net.URLEncoder.encode(state.comment, "utf-8") val url = "${state.callbackUrl}${urlBinder}amount=${state.amountMilliSats}&comment=$encodedComment" - val request = - okhttp3.Request - .Builder() - .url(url) - .build() - val response = - kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.IO) { - httpClient.newCall(request).execute() - } - val body = response.body.string() - val json = mapper.readTree(body) + val json = fetchLnurlJson(DesktopHttpClient.currentClient(), mapper, url) val pr = json.get("pr")?.asText()?.ifBlank { null } if (pr != null) { sendState = SendState.ReadyToPay(pr)