From d742534dac621d3d5888e03b135d656b2556c8b6 Mon Sep 17 00:00:00 2001 From: tcheeric Date: Wed, 27 May 2026 07:34:56 +0100 Subject: [PATCH 1/3] fix(ws): guard send() with isOpen() to stop CLOSE-on-dying-connection race (spec-026) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit send() lacked the isOpen() guard subscribe() has. A per-subscription Nostr CLOSE issued while a relay connection is breaking reached Tomcat's sendText, where doClose() fires mid-write and emits a WS CLOSE frame while the text write is still pending on the async channel — throwing "Concurrent write operations are not permitted", surfacing as a transport-error reconnect storm. Fail fast on a closed session instead. +regression test. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../springwebsocket/NostrRelayClient.java | 11 +++++++++++ .../NostrRelayClientCloseWriteRaceTest.java | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/nostr-java-client/src/main/java/nostr/client/springwebsocket/NostrRelayClient.java b/nostr-java-client/src/main/java/nostr/client/springwebsocket/NostrRelayClient.java index 858b962a..876dc2aa 100644 --- a/nostr-java-client/src/main/java/nostr/client/springwebsocket/NostrRelayClient.java +++ b/nostr-java-client/src/main/java/nostr/client/springwebsocket/NostrRelayClient.java @@ -479,6 +479,17 @@ public List send(T eventMessage) throws IOExcept @NostrRetryable public List send(String json) throws IOException { + // Fail fast on a closed session instead of entering Tomcat's write path. + // A send on an already-closed/closing session (e.g. a per-subscription + // CLOSE issued while a relay connection is being torn down) otherwise + // reaches WsRemoteEndpointImplBase.sendText, where Tomcat may invoke + // doClose() mid-write and emit a CLOSE frame while the text write is still + // pending on the async channel — throwing "Concurrent write operations are + // not permitted" and surfacing as a transport-error reconnect storm. The + // subscribe() path already guards this way; mirror it here. (spec-026) + if (!clientSession.isOpen()) { + throw new IOException("WebSocket session is closed"); + } PendingRequest request; sendLock.lock(); diff --git a/nostr-java-client/src/test/java/nostr/client/springwebsocket/NostrRelayClientCloseWriteRaceTest.java b/nostr-java-client/src/test/java/nostr/client/springwebsocket/NostrRelayClientCloseWriteRaceTest.java index 7ddc6b93..eb0d61f4 100644 --- a/nostr-java-client/src/test/java/nostr/client/springwebsocket/NostrRelayClientCloseWriteRaceTest.java +++ b/nostr-java-client/src/test/java/nostr/client/springwebsocket/NostrRelayClientCloseWriteRaceTest.java @@ -93,6 +93,24 @@ void sendTimeout_routesCloseThroughCloseStatus_neverNoArgClose() throws Exceptio verify(raw, never()).close(); } + // ---- Guard: send() on an already-closed session must fail fast without + // reaching the delegate, so it never enters Tomcat's sendText → + // doClose-mid-write → "Concurrent write operations are not permitted" + // path (the per-subscription CLOSE-on-a-dying-connection trigger). ---- + @Test + void send_onClosedSession_failsFastWithoutDelegateWrite() throws Exception { + WebSocketSession raw = Mockito.mock(WebSocketSession.class); + Mockito.when(raw.isOpen()).thenReturn(false); // session already closed/closing + + NostrRelayClient client = + NostrRelayClient.forTestWithDecoratedSession(raw, TEST_AWAIT_TIMEOUT_MS); + + IOException ex = assertThrows(IOException.class, () -> client.send(REQ)); + assertTrue(ex.getMessage().contains("closed"), + "expected a closed-session IOException, was: " + ex.getMessage()); + verify(raw, never()).sendMessage(any(TextMessage.class)); + } + // ---- Serialisation: an in-flight subscribe write and a concurrent close // must never overlap on the delegate. ---- @Test From a1beaf88e5931b7d921607444dc2b1a468757df6 Mon Sep 17 00:00:00 2001 From: tcheeric Date: Wed, 27 May 2026 07:39:14 +0100 Subject: [PATCH 2/3] chore(release): nostr-java 2.0.6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit send() isOpen() guard (spec-026) — eliminates the CLOSE-on-dying-connection concurrent-write transport-error storm. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 5 +++++ nostr-java-client/pom.xml | 2 +- nostr-java-core/pom.xml | 2 +- nostr-java-event/pom.xml | 2 +- nostr-java-identity/pom.xml | 2 +- pom.xml | 2 +- 6 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index efd207dc..c88b138d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ The format is inspired by Keep a Changelog, and this project adheres to semantic ## [Unreleased] +## [2.0.6] - 2026-05-27 + +### Fixed +- `NostrRelayClient.send()` now guards on `clientSession.isOpen()` before writing, mirroring `subscribe()`. A per-subscription Nostr `CLOSE` issued while a relay connection was being torn down previously reached Tomcat's `sendText`, where `WsRemoteEndpointImplBase.sendMessageBlockInternal` invokes `doClose()` mid-write and emits a WS CLOSE frame while the text write is still pending on the async channel — throwing `IllegalStateException: Concurrent write operations are not permitted`, which surfaced via `handleTransportError` as a transport-error reconnect storm during subscription teardown of a breaking connection (spec-026). The application-level locks (the decorator, the `sessionGate`, and the upstream adapter `sendLock`) cannot prevent this because it is Tomcat closing the session *inside* a single in-progress send, not two application threads racing. `send()` now fails fast with `IOException("WebSocket session is closed")` on a closed session — the doomed write never enters Tomcat's close-mid-write path. +regression test `send_onClosedSession_failsFastWithoutDelegateWrite`. + ## [2.0.5] - 2026-05-26 ### Fixed diff --git a/nostr-java-client/pom.xml b/nostr-java-client/pom.xml index 2f1f6a46..221ad7cb 100644 --- a/nostr-java-client/pom.xml +++ b/nostr-java-client/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric nostr-java - 2.0.5 + 2.0.6 ../pom.xml diff --git a/nostr-java-core/pom.xml b/nostr-java-core/pom.xml index d38bc5e9..403bac44 100644 --- a/nostr-java-core/pom.xml +++ b/nostr-java-core/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric nostr-java - 2.0.5 + 2.0.6 ../pom.xml diff --git a/nostr-java-event/pom.xml b/nostr-java-event/pom.xml index ba383a77..2120c7b5 100644 --- a/nostr-java-event/pom.xml +++ b/nostr-java-event/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric nostr-java - 2.0.5 + 2.0.6 ../pom.xml diff --git a/nostr-java-identity/pom.xml b/nostr-java-identity/pom.xml index 7001e999..3a7c43b0 100644 --- a/nostr-java-identity/pom.xml +++ b/nostr-java-identity/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric nostr-java - 2.0.5 + 2.0.6 ../pom.xml diff --git a/pom.xml b/pom.xml index 4b0f4314..1c4987e1 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ xyz.tcheeric nostr-java - 2.0.5 + 2.0.6 pom nostr-java From fa7b95fa562ad03c1c3a5b8e80470789c016c779 Mon Sep 17 00:00:00 2001 From: tcheeric Date: Wed, 27 May 2026 15:50:53 +0100 Subject: [PATCH 3/3] =?UTF-8?q?fix(ws):=20close=20TOCTOU=20in=20send()=20i?= =?UTF-8?q?sOpen=20guard=20=E2=80=94=20check=20inside=20read=20lock=20(2.0?= =?UTF-8?q?.7,=20PR=20#526=20review)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2.0.6 guard checked clientSession.isOpen() at the top of send(), before sendFrameGated() took the sessionGate read lock — so a concurrent close() could close+release the session between the check and sendMessage(). Moved the check inside sendFrameGated() after the read lock, making open-check + write atomic w.r.t. closeGated() (write lock). Bump 2.0.6 -> 2.0.7. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 5 +++++ nostr-java-client/pom.xml | 2 +- .../springwebsocket/NostrRelayClient.java | 22 +++++++++---------- nostr-java-core/pom.xml | 2 +- nostr-java-event/pom.xml | 2 +- nostr-java-identity/pom.xml | 2 +- pom.xml | 2 +- 7 files changed, 21 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c88b138d..0a243709 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ The format is inspired by Keep a Changelog, and this project adheres to semantic ## [Unreleased] +## [2.0.7] - 2026-05-27 + +### Fixed +- Closed a TOCTOU window in the 2.0.6 `send()` `isOpen()` guard (PR #526 review). The check was performed at the top of `send()`, *before* `sendFrameGated()` acquired the `sessionGate` read lock — so a concurrent `close()` could acquire the write lock, close the session, and release it between the check and the actual `sendMessage()`, still letting the frame reach a closed session. Moved the `isOpen()` check inside `sendFrameGated()`, immediately after the read lock is taken, so the open-check and the write are now atomic with respect to `closeGated()` (which holds the write lock). 2.0.6 already handled the dominant case (the session already closed when `send()` is invoked); this closes the narrow in-flight-close window. + ## [2.0.6] - 2026-05-27 ### Fixed diff --git a/nostr-java-client/pom.xml b/nostr-java-client/pom.xml index 221ad7cb..5c07b6fc 100644 --- a/nostr-java-client/pom.xml +++ b/nostr-java-client/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric nostr-java - 2.0.6 + 2.0.7 ../pom.xml diff --git a/nostr-java-client/src/main/java/nostr/client/springwebsocket/NostrRelayClient.java b/nostr-java-client/src/main/java/nostr/client/springwebsocket/NostrRelayClient.java index 876dc2aa..15853f49 100644 --- a/nostr-java-client/src/main/java/nostr/client/springwebsocket/NostrRelayClient.java +++ b/nostr-java-client/src/main/java/nostr/client/springwebsocket/NostrRelayClient.java @@ -479,17 +479,6 @@ public List send(T eventMessage) throws IOExcept @NostrRetryable public List send(String json) throws IOException { - // Fail fast on a closed session instead of entering Tomcat's write path. - // A send on an already-closed/closing session (e.g. a per-subscription - // CLOSE issued while a relay connection is being torn down) otherwise - // reaches WsRemoteEndpointImplBase.sendText, where Tomcat may invoke - // doClose() mid-write and emit a CLOSE frame while the text write is still - // pending on the async channel — throwing "Concurrent write operations are - // not permitted" and surfacing as a transport-error reconnect storm. The - // subscribe() path already guards this way; mirror it here. (spec-026) - if (!clientSession.isOpen()) { - throw new IOException("WebSocket session is closed"); - } PendingRequest request; sendLock.lock(); @@ -728,6 +717,17 @@ public void close() throws IOException { private void sendFrameGated(String json) throws IOException { sessionGate.readLock().lock(); try { + // The isOpen() check MUST live inside the read lock so it is mutually + // exclusive with closeGated() (which holds the write lock). A check + // outside the lock leaves a TOCTOU window: a concurrent close() could + // close + release the session between the check and this write, letting + // the frame reach an already-closed session and trip Tomcat's + // close-mid-write race ("Concurrent write operations are not permitted"). + // Per-subscription CLOSEs issued while a relay connection is being torn + // down are the dominant trigger. (spec-026) + if (!clientSession.isOpen()) { + throw new IOException("WebSocket session is closed"); + } clientSession.sendMessage(new TextMessage(json)); } finally { sessionGate.readLock().unlock(); diff --git a/nostr-java-core/pom.xml b/nostr-java-core/pom.xml index 403bac44..d05aec6b 100644 --- a/nostr-java-core/pom.xml +++ b/nostr-java-core/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric nostr-java - 2.0.6 + 2.0.7 ../pom.xml diff --git a/nostr-java-event/pom.xml b/nostr-java-event/pom.xml index 2120c7b5..6ea8bd83 100644 --- a/nostr-java-event/pom.xml +++ b/nostr-java-event/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric nostr-java - 2.0.6 + 2.0.7 ../pom.xml diff --git a/nostr-java-identity/pom.xml b/nostr-java-identity/pom.xml index 3a7c43b0..acfaced8 100644 --- a/nostr-java-identity/pom.xml +++ b/nostr-java-identity/pom.xml @@ -4,7 +4,7 @@ xyz.tcheeric nostr-java - 2.0.6 + 2.0.7 ../pom.xml diff --git a/pom.xml b/pom.xml index 1c4987e1..e5c21e39 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ xyz.tcheeric nostr-java - 2.0.6 + 2.0.7 pom nostr-java