The publisher-side dispatcher in handleInboundBidi previously matched
inbound Subscribe bidis on `track` only, never checking whether the
requested `broadcast` field matched the suffix our publisher claimed
on this session. A peer (or buggy relay) could open a Subscribe under
broadcast="otherPubkey" track="audio/data" and we would route OUR
audio frames to them — the dispatcher only filtered on track.
Production never bit because moq-rs's relay routes Subscribe messages
to the specific publisher whose broadcast matches the requested path
upstream of us, so an off-target broadcast never landed on our
inbound bidi pump. But a peer-to-peer connection without a relay
in between would have been able to siphon our audio under any
broadcast string they invented.
The fix: validate `sub.broadcast == publisher.suffix` (both already
path-normalised by the codec) before track matching. On mismatch,
reply `SubscribeDrop(errorCode=BROADCAST_DOES_NOT_EXIST,
reasonPhrase="<requested> not published on this session
(we publish <ours>)")` and FIN. New error code
`MoqLiteSubscribeDropCode.BROADCAST_DOES_NOT_EXIST = 0x05L` mirrors
the IETF MoQ-transport `TRACK_NAMESPACE_DOES_NOT_EXIST` semantic so
a watcher can tell apart "wrong room" from "wrong rendition".
Regression test:
`MoqLiteSessionTest.publisher_replies_subscribeDrop_when_broadcast_does_not_match`.
Audit doc: nestsClient/plans/2026-05-09-moq-lite-rfc-compliance.md (M5).
https://claude.ai/code/session_012TGfo99Ugz7fcCPv85a8Tq
Per moq-lite Lite-03 (kixelated/moq `rs/moq-lite/src/lite/announce.rs`),
a publisher MUST only emit `Announce(Active)` for broadcasts whose path
starts with the requested AnnouncePlease prefix. Pre-fix, when
`MoqLitePath.stripPrefix(please.prefix, ourSuffix)` returned `null`, we
fell through to `?: announcePublisher.suffix` and emitted Active under
our full suffix anyway, falsely advertising the broadcast under a
prefix the subscriber didn't request.
Production never bit because the relay always opens its announce bidi
to us with `prefix=""` (which always matches), but a future peer-to-
peer or namespace-scoped subscriber would have observed a ghost
broadcast under whatever prefix they asked about.
The fix: if `stripPrefix` returns `null`, FIN the bidi cleanly without
writing any Announce body. The subscriber sees an empty announce
stream and moves on.
Regression test:
`MoqLiteSessionTest.publisher_skips_announce_when_announce_please_prefix_does_not_match`.
Audit doc: nestsClient/plans/2026-05-09-moq-lite-rfc-compliance.md (M4).
https://claude.ai/code/session_012TGfo99Ugz7fcCPv85a8Tq
- Merge nested if statements in marmot stores' delete() and in
ZoomableContentDialog's MediaUrl* gating block (S1066).
- Replace `if (cond) false else x` with `!cond && x` in
ImageVideoDescription's effectiveStripMetadata (S1125).
- Remove unused `nip95description` local in FileServerSelectionRow.
All changes are body-internal with identical behaviour.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round 2 of the blocking-code audit follow-ups. Both changes remove
commonMain synchronized blocks and replace polling with event-driven
suspension; :quic:jvmTest stays green on JVM and :quic:compileAndroidMain
passes for Android.
QuicStream.resetStream / stopSending
- Replace synchronized(this) double-checked init with
AtomicReference<ResetState?>.compareAndSet(null, …) (and same for
stopSendingState). The pattern is "first call wins"; CAS expresses
that directly without a lock.
- Backing fields converted from `internal var … = null` to
`internal val … = AtomicReference(null)`. The two readers in
QuicConnectionWriter switch to .load(); QuicConnectionWriter gains a
file-level @OptIn(ExperimentalAtomicApi::class).
- The @Volatile resetEmitPending / stopSendingEmitPending writes happen
only on the winning CAS path, so the writer reading the flag still
sees the populated state via volatile happens-before.
QuicConnectionDriver.close polling loop
- Replace `while (status == CLOSING) delay(1)` polling with a
CompletableDeferred<Unit> on QuicConnection (closingDrainSignal).
The deferred is completed at both transitions to CLOSED:
(1) drainOutbound after building the CONNECTION_CLOSE datagram, and
(2) markClosedExternally on its synchronized first-call-wins path.
- close() now awaits the deferred under the existing
CLOSE_FLUSH_TIMEOUT_MILLIS bound — same upper-bound semantics, no
1 ms wakeups during teardown. complete(Unit) is idempotent, so the
two transition sites racing each other is safe.
https://claude.ai/code/session_01CXTjnuHKCNXDmpfyKxgG3V
Audit of blocking and synchronized code in the QUIC module surfaced four
hot-path wins, all verified against :quic:jvmTest.
- PlatformCrypto: header-protection AES-ECB now uses a per-thread cached
Cipher. Previously every inbound and outbound packet paid for
Cipher.getInstance("AES/ECB/NoPadding") provider lookup. ThreadLocal is
safe because the call is stateless — every invocation re-init's with the
caller-supplied key.
- JdkCertificateValidator: gate the SAN-side InetAddress.getByName behind
looksLikeIpLiteral so a malformed cert with a hostname in a type 7 SAN
cannot trigger DNS resolution on the TLS validation path.
- QuicConnectionDriver.close: replace synchronized(this) double-checked
init with AtomicReference<Job?> + compareAndSet on a CoroutineStart.LAZY
job. Lock-free, removes a synchronized from commonMain, preserves the
original "first caller wins, second awaits same Job" contract.
- SendBuffer: mark _nextOffset, nextSendOffset, _finPending, _finSent,
_finAcked @Volatile and drop synchronized from their single-field
getters (nextOffset, sentOffset, finPending, finSent, finAcked). The
compound-formula readableBytes getter still synchronizes.
https://claude.ai/code/session_01CXTjnuHKCNXDmpfyKxgG3V
Move the feedDatagram KDoc below the MAX_QUIC_OFFSET const so it
attaches to the function declaration, and merge the duplicate KDoc
blocks above TlsResumptionState into a single block.
Round 13 — three architectural follow-ups + a closeout note.
* JdkCertificateValidator: ship a hand-picked Public Suffix List
subset covering the high-volume multi-label effective-TLDs
(multi-tenant ccTLDs and major hosting platforms). Pre-fix the
dot-count heuristic accepted `*.co.uk`, `*.s3.amazonaws.com`,
`*.github.io`, etc. — wildcards spanning these would impersonate
every co-tenant. The new MULTI_LABEL_PUBLIC_SUFFIXES set adds a
layer above the dot-count check; combined with the WebPKI / CT
ecosystem already requiring CAs to consult the full PSL when
issuing, this closes the practical attack surfaces. Full
~9000-entry PSL data shipping is still deferred (data-shipping
ask, doc'd); a domain not in the subset that's also a multi-label
ETLD remains a gap.
* JcaChaCha20Poly1305Aead: new JCA-backed implementation mirroring
JcaAesGcmAead's shape (cached Cipher + SecretKeySpec, range
overloads via Cipher.doFinal(input, off, len, output, outOff),
recent-nonce history for legitimate IV reuse on the
Initial-padding rebuild path). bestChaCha20Poly1305Aead(key)
expect/actual factory tries the JCA path first (Java 11+ /
Android API 28+) and falls back to the pure-Kotlin
ChaCha20Poly1305Aead singleton if the algorithm isn't available
(older Android, headless GraalVM native-image without the
standard providers). PacketProtectionBuilder routes the
ChaCha20-Poly1305 cipher suite through the factory instead of
the singleton. On supporting platforms this gives the same
outbound-allocation savings as round 8's AES-GCM range overload.
* QuicConnectionWriter: stop emitting fake ECN counts on ACK
frames. Pre-fix every 1-RTT ACK carried `AckEcnCounts(0, 0, 0)`
— claiming to track ECN while actually never reading inbound TOS
bits. RFC 9000 §13.4.2: "An endpoint that uses ECN MUST report
accurate ECN counts." Hardcoded zeros could be flagged as a
PROTOCOL_VIOLATION by strict peers cross-validating against
outbound packet counts; aioquic / picoquic / quic-go tolerate
it but other stacks may not. With ecnCounts = null we honestly
advertise "this endpoint isn't reporting ECN", peer skips its
own ECN-driven congestion logic for our direction. We still
mark outbound ECT(0) (other peers' tracking benefits from the
path-quality signal); RFC 9000 §13.4 allows the asymmetry.
* MutableSharedFlow migration for QuicStream.incoming declined as
obviated. The audit's suggestion was a workaround for the
cancel-coupling specifically (collector cancel → channel cancel
→ INTERNAL_ERROR) — round 11's `flow { for (c in
incomingChannel) emit(c) }` wrapper solved that. Switching to
MutableSharedFlow would change the semantics from "each byte to
exactly one consumer" (correct for stream bytes) to fan-out
(every emission to all collectors), which is wrong for QUIC
stream data.
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 40s.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Round 12 — six smaller follow-ups across visibility, perf, and secret-handling.
* QuicStream.receiveDirtyForFlowControl gains @Volatile. The parser's
read-loop writes the flag and the writer's send-loop reads it
WITHOUT holding the same lock; without volatile the writer could
miss the parser's update for an unbounded time on JVM (the field is
hot in a drain loop where the JIT might cache it), suppressing
MAX_STREAM_DATA emissions until something else triggered a fresh
load.
* SendBuffer.readableBytes runs in O(1) instead of O(R) by maintaining
a cached `retransmitTotalBytes` counter. Updated in lockstep with
every retransmit deque mutation: addLast in [requeueAllInflight] +
the two paths in [removeOverlap] (RETRANSMIT zero-length + main
range), and add/removeFirst in [takeChunk]. Pre-flight "anything to
send?" check on the writer's hot path was previously walking the
deque per-stream per-drain.
* SendBuffer.requeueAllInflight coalesces adjacent ranges on insert.
Pre-fix the PTO probe path appended each in-flight range as a
separate retransmit entry, so on the next drain takeChunk emitted
one tiny STREAM frame per original-packet boundary. With
coalescing, contiguous bytes get replayed as one chunk + one AEAD
seal. FIN-bearing ranges stay separate (merging across a FIN
changes the implicit final-size invariant).
* TlsResumptionState dropped `data class`. The auto-generated
equals/hashCode used reference equality on its ByteArray fields
(PSK / ticket / peerTransportParameters), so two byte-identical
states compared unequal — almost never useful and a footgun for
caller-side caches. The auto-toString would dump PSK contents into
any log. Replaced with hand-written equals/hashCode using
contentEquals on the byte fields and a redacted toString that
reveals only sizes.
* QuicConnectionParser RESET_STREAM handler bounds finalSize at
[0, 2^62-1] per RFC 9000 §16 (the QUIC offset ceiling). Pre-fix
we accepted any varint, including values that could overflow
downstream Long math.
* PathChallenge/PathResponse IAE leak: re-traced and verified
non-issue. The decoder calls `r.readBytes(8)`, which either throws
QuicCodecException (short read) or returns exactly 8 bytes — the
constructor's `require(data.size == 8)` is unreachable from the
decode path. The audit was over-cautious; no code change.
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 39s.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Round 11 — six follow-ups across perf, correctness, and API hygiene.
* QuicStream.incoming: replace `consumeAsFlow()` with `flow { for (c in
incomingChannel) emit(c) }`. Pre-fix the consume-style flow
cancelled the underlying Channel when the collector terminated,
which coupled "application stopped reading" with "parser
INTERNAL_ERRORs the connection on next delivery". The new emit-only
flow leaves the channel open across collector cancellation, so
applications can stop reading temporarily and resume later
(sequential collects, not concurrent — that's still a race on the
channel iterator).
* RFC 9001 §6.1 client-initiated key update: the existing
[QuicConnection.initiateKeyUpdate] no longer requires the caller
to enforce spec invariants. Returns false if the handshake isn't
yet complete (§6.5: MUST NOT initiate before HANDSHAKE_DONE) or if
a previous rotation is still in flight (§6.4: MUST NOT initiate a
subsequent update until the previous one is confirmed). The parser
clears the [keyUpdateInProgress] flag on the first inbound packet
that AEAD-decrypts under the post-rotation live keys — the
confirmation signal that the peer has rolled forward.
* Aead.sealInto: new range + in-place seal that writes ciphertext+tag
DIRECTLY into a caller-supplied output buffer at a given offset.
Default impl falls back to sealRange + copy; JcaAesGcmAead overrides
to use Cipher.doFinal(input, inOff, inLen, output, outOff).
LongHeaderPacket.build / ShortHeaderPacket.build now pre-allocate
the final packet buffer in a single shot and have the AEAD write
ciphertext+tag in-place. Pre-fix every outbound packet allocated
4 ByteArrays (headerBytes, paddedPlaintext, ciphertext, concat
buffer); now ~2 (the final packet + the AEAD provider's internal
scratch).
* QuicConnectionWriter.drainOutbound: skip the
`active.sortedByDescending { priority }` allocation when EVERY
active stream shares the same priority — not just the
default-zero case. A homogeneous priority-7 workload now keeps
insertion order at no cost.
* QuicConnection.scratchAppFrames / scratchAppTokens: per-connection
reusable lists for buildApplicationPacket. Cleared at function
entry, re-used across drains under streamsLock's single-writer
guarantee. The tokens list is `.toList()`-snapshotted into the
SentPacket record before reuse, so retransmit dispatch is
unaffected. NOT applied to collectHandshakeLevelFrames because
its returned [HandshakeLevelContents] is held across two
buildLongHeaderFromFrames calls (natural-size + padded rebuild)
in drainOutbound.
* Removed dead `parts = mutableListOf<ByteArray>()` declaration at
the top of drainOutbound — never referenced; the actual datagram
assembly uses inline `listOfNotNull(...)` instead.
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 43s.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Round 10 — supersedes the typed-exception approach from round 9.
Round 9 introduced [PskRejectedException] as a "punt to the application
layer" hack, with a comment claiming the in-place fallback was too
subtle to land safely. After tracing the actual derivation path the
fix turns out to be one line.
The key observation: the on-wire ClientHello (with `pre_shared_key`
extension and binder bytes) goes into BOTH client and server
transcript hashes regardless of accept / reject (RFC 8446 §4.2.11).
The transcript hash itself doesn't need any rebuild. The ONLY thing
that differs between accept and reject is how [earlySecret] is
derived:
* accepted: HKDF-Extract(0, PSK)
* rejected: HKDF-Extract(0, 0) ← same as no-resumption path
So when the server returns ServerHello without `pre_shared_key`,
we simply call `keySchedule.deriveEarly()` to overwrite the
PSK-derived [earlySecret] with the zero-keyed value. [deriveHandshake]
runs immediately after with the new earlySecret + ECDHE shared
secret, and the handshake proceeds along the regular non-resumption
path (which is well-tested by every non-resumption test in the
suite).
* `pskAccepted = false` so the
WAITING_CERTIFICATE_OR_FINISHED state correctly demands
Certificate + CertificateVerify (a Finished without those would
still be rejected as unauthenticated).
* Any 0-RTT packets the application emitted under the
PSK-derived [clientEarlyTrafficSecret] are lost — server can't
decrypt them and EncryptedExtensions arrives without the
early_data extension, so [earlyDataAccepted] = false. The
application layer is responsible for replaying any 0-RTT-bound
payload over 1-RTT. The handshake itself proceeds cleanly.
* [PskRejectedException] (added in round 9) deleted as dead code.
[QuicCodecException] reverts to a `final` class.
All 269 :quic:jvmTest tests pass. The fallback re-uses the
deriveEarly() codepath that every non-resumption test exercises,
so test coverage is implicit in the existing suite.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Round 9 — the two largest items remaining from the audit.
* RFC 9002 §6.1.2 timer-driven loss detection. [detectAndRemoveLost]
now returns a [Result] data class carrying both the list of lost
packets and the absolute monotonic deadline at which the next
earliest sub-largest in-flight packet will cross the time threshold.
The parser stores that deadline on each [LevelState.nextLossTimeMs]
per encryption level, and the driver's send loop now sleeps for
`min(ptoDeadline, minNextLossTimeAcrossLevels) - now`. On expiry,
the driver distinguishes:
* Loss-timer wake → run [detectAndRemoveLost] across all levels;
declare time-threshold-lost packets and dispatch their tokens.
No probe budget, no exponential backoff.
* PTO wake → existing [handlePtoFired] path (probe + backoff).
Pre-fix tail loss waited for the full PTO (often 5x the time
threshold) before retransmitting because we had no event between
ACK arrivals to fire loss detection. Now `9/8 * max_rtt` is the
ceiling.
* TLS PSK rejection: instead of the prior generic [QuicCodecException]
("server rejected PSK; full-handshake fallback not implemented"),
raise a typed [PskRejectedException] subclass. Application reconnect
logic can selectively catch this and retry the handshake without
cached resumption state — a path that's correct by construction
(fresh ClientHello, no PSK history, no transcript-rebuild
complexity). [QuicCodecException] is now `open` so the subclass
can extend it.
In-place fallback (clear early secret, rebuild transcript without
PSK extension, replay derivation) remains deferred — the subtle
transcript-hash discrepancies it could introduce would be much
harder to debug than a hard failure that the application
intentionally turns into a retry.
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 47s.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Round 8 — AEAD allocation reduction on the receive hot path, plus the
documentation of two known-fragile-but-not-broken couplings.
* Aead gains [openRange] / [sealRange] taking (array, offset, length)
triples for AAD and plaintext/ciphertext. Default impls slice and
delegate to the existing whole-array methods so non-JCA AEADs
(Aes128Gcm singleton, ChaCha20Poly1305Aead) still work. The
JCA-backed [JcaAesGcmAead] overrides both, passing offsets straight
to `Cipher.updateAAD(byte[], offset, len)` and
`Cipher.doFinal(byte[], inputOffset, inputLen)` — JCA accepts ranges
natively and does no internal copies. Saves ~2 KB ByteArray
allocations per inbound packet (one for the header `aad`, one for
the `ciphertext`) — at audio-room receive rates (~100 packets/sec)
that's ~12 MB/min of GC churn eliminated. Same overload on the
outbound path is wired but currently exercised less because the
build path constructs `headerBytes` and `paddedPlaintext` as
separate fresh allocations.
parseAndDecrypt in both ShortHeaderPacket and LongHeaderPacket now
call openRange instead of slicing into intermediates.
* JdkCertificateValidator.dnsMatches: explicit doc note on the
public-suffix-list gap. The dot-count heuristic accepts wildcards
like `*.co.uk` / `*.github.io` / `*.s3.amazonaws.com` whose effective
TLD spans multiple labels. WebPKI / CT logging mitigates this in
practice (CAs validate against the actual PSL), but our local
validation alone wouldn't catch a rogue cert. Production callers
for sensitive endpoints should rely on OS / NetworkSecurityConfig
pinning rather than QUIC's hostname check alone. Full PSL data
shipping deferred until justified.
* QuicStream.incoming: explicit doc note on the single-collector
contract. [consumeAsFlow] cancels the underlying [Channel] when
its collector terminates, and once cancelled the parser's next
trySend fails, sets [overflowed] = true, and the parser tears down
the connection with INTERNAL_ERROR. Production callers already
follow single-collector + collect-until-FIN, but the doc lays out
the rule so a future refactor doesn't loosen it accidentally.
Long-form discussion of the `MutableSharedFlow` alternative and
why we declined it.
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 40s.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Round 7 — closing out the audit's spec/correctness/perf quick wins.
* WtPeerStreamDemux: validate peer SETTINGS includes ENABLE_WEBTRANSPORT=1
AND ENABLE_CONNECT_PROTOCOL=1 (draft-ietf-webtrans-http3 §3 +
RFC 8441). Pre-fix we accepted any SETTINGS and proceeded to
Extended CONNECT, which the server then rejects with no
diagnostic for the application. Now surfaces as a typed protocol
error on peerH3ProtocolError so the QUIC layer closes deliberately.
* QpackEncoder: set N=1 (never-indexed) on literal field lines for
authorization / cookie / set-cookie / proxy-authorization per
RFC 9204 §4.5.4. Pre-fix sensitive credentials could be cached
by intermediate QPACK encoder caches.
* QuicConnection.getOrCreatePeerStreamLocked: track peerInitiated*Count
via max(current, peerIndex+1) instead of += 1. The counter now
derives from the stream id's index field (RFC 9000 §2.1) and is
idempotent against retransmits-after-eviction. Pre-fix a peer
retransmit on a stream id that aged out of the retired-IDs FIFO
bumped the counter again, eventually triggering spurious
MAX_STREAMS_* emissions.
* applyPeerRetireConnectionIdLocked: reclassify the close-on-seq=0
case as PROTOCOL_VIOLATION (peer fault) instead of INTERNAL_ERROR
(our fault). The diagnostic was misleading — the peer IS
misbehaving (asking us to retire our only SCID with no
replacement available), not us.
* QuicConnectionWriter.drainOutbound: skip the
`streamsView.filter { !it.isClosed }` allocation when no streams
are closed. Quick `any` scan first; only allocate the filtered
list when at least one stream is actually closed. Saves an
N-sized ArrayList per drain in the common case (~50 drains/sec
× N up to ~2000 streams under multiplex load).
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 40s.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Pre-fix QpackHuffman.decode allocated two boxed Integers per output
character: one for the `candidate` Int passed into
`HashMap<Int, Int>.get` and one for the wrapper-Integer return value.
Output also went through `ArrayList<Byte>`, boxing every emitted
byte as a `java.lang.Byte` (~16 bytes per output byte on a 64-bit
JVM). On a typical HTTP/3 response with ~30 header values, that's
hundreds of throwaway wrapper objects per request — pure GC churn
on the hot path.
The new layout keeps two parallel `IntArray`s per code-length:
codes[len] (sorted ascending) and syms[len] (the matching symbol
indices). Lookup is a primitive `IntArray.binarySearch(candidate)`
— no boxing, the array stays in JIT-friendly contiguous memory,
and the per-length arrays are tiny (a few entries each, since the
Huffman table is sparse at any given length).
Output uses a growable `ByteArray` with a manual position index
rather than `ArrayList<Byte>`. Pre-grow to 2× input size as a
rough upper bound — ASCII headers compress to ~62% with HPACK
Huffman, so we rarely need to grow.
Also fixes a latent bug: the new init loop covers lengths 5..30
(previously 5..29), restoring decoding for symbols 10 (LF), 13
(CR), 22 (DC2) which all use 30-bit codes per RFC 7541 Appendix B.
Pre-rewrite the HashMap path included these via `for (sym in 0..255)`
walking the full symbol table; the IntArray rewrite needed an
explicit length range and accidentally cut at 29. Added a unit
test exercising hand-encoded length-30 inputs to lock the fix in.
Behavior verified against RFC 7541 Appendix C test vectors and the
new length-30 round-trip. All 269 :quic:jvmTest tests pass.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
When a Namecoin record's value isn't valid JSON (a real failure mode
when an operator hand-builds the value and miscounts braces), the
NIP-05 path used to silently swallow the parser exception and surface
a misleading "no nostr field" message. That sends the publisher
chasing a phantom missing field when the actual problem is the value
itself.
Concrete case that triggered this: a `name_update` published a
474-byte d/testls value with one closing brace short of balanced. The
string parses up to the missing brace, after which kotlinx.serialization
throws "Unfinished JSON term at EOF at line 1, column 474". That error
was previously dropped, leaving the operator to debug "no nostr field"
without ever seeing the underlying JSON parse failure.
Changes:
- New NamecoinResolveOutcome.MalformedRecord(name, error). Distinct
from NoNostrField. The `error` field is the parser's own diagnostic
(e.g. "Unfinished JSON term at EOF at line 1, column 474") so the
publisher can locate the broken byte without spelunking.
- NamecoinNameResolver.performLookupDetailed: parse via a new
parseValueOrError helper and surface MalformedRecord instead of
collapsing into NoNostrField. Also rejects non-object top-level
values (arrays, primitives, null) with a useful diagnostic
("top-level value is JsonArray, expected JSON object").
- DesktopSearchScreen handles the new outcome by surfacing the parser
error verbatim in the Namecoin status banner, so the column number
reaches the publisher's screen.
Tests (commonTest / NamecoinImportTest):
- "NIP-05 lookup surfaces MalformedRecord with parser detail when
value is broken JSON": a deliberately one-brace-short value yields
MalformedRecord with a non-empty diagnostic.
- "NIP-05 lookup surfaces MalformedRecord when top-level value is a
JSON array": ensures non-object top-level values are rejected with
a useful "expected JSON object" message rather than silently
parsing as something unusable.
Tests don't pin the exact parser wording (kotlinx.serialization can
change it across versions); they only pin that the message is
attributed to JSON parsing rather than to a missing field.
Round 5 of the audit follow-ups. The remaining low-leverage items
from the original audit all addressed in one pass.
* PathValidator: pick the smallest spare CID sequence number rather
than LinkedHashMap insertion order. RFC 9000 §19.15 lets the peer
issue NEW_CONNECTION_ID out of sequence (e.g. retransmits arriving
after newer offers); insertion-order picking would land on
whichever offer arrived first instead of the lowest seq, drifting
away from the RFC-expected ordering. forceRotateToHigherSequence
also now filters >= retirePriorToWatermark explicitly so we never
pick a sequence below the watermark even if the pool somehow holds
one.
* QuicWebTransportSessionState.close: driver.wakeup() AFTER enqueuing
the WT_CLOSE_SESSION capsule + FIN but BEFORE driver.close, so the
capsule actually reaches the wire instead of being short-circuited
by the driver shutdown. Pre-fix the peer saw an abrupt UDP-level
tear-down with no application-error-code surfaced.
* QuicStream.resetStream / stopSending: synchronized(this) atomic
CAS for the "first call wins" gate. Pre-fix two concurrent callers
could both observe `resetState == null` and both write — the
second caller's errorCode would clobber the first while
resetEmitPending was already set, so the writer emitted the
RESET_STREAM with whichever value landed last.
* Http3Settings.decodeBody: per-id value range checks. A peer that
advertises e.g. MAX_FIELD_SECTION_SIZE = 2^60 could otherwise
drive our encoder into unbounded heap. Bounds chosen above any
legitimate value (1 GiB for table-capacity / field-section caps,
1 for boolean flags) and below 2^32 for unknown ids.
* Privatize crypto-relevant static byte arrays:
InitialSecrets.V1_INITIAL_SALT, RetryPacket.V1_RETRY_KEY,
RetryPacket.V1_RETRY_NONCE. Pre-fix these were public mutable
ByteArrays — any caller could stomp on them, and any toString /
reflection would leak the bytes. Crypto material doesn't need to
be reachable outside the deriving / sealing path.
* peekHeader length cast: re-verified as already safe (lengthRaw is
bounded by r.remaining before .toInt() — Int.MAX_VALUE ceiling
enforced).
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 2m 17s.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Round 4 of the audit follow-ups.
* Reserved-bit enforcement on unmasked QUIC headers (RFC 9000 §17.2 /
§17.3.1). Pre-fix the parser silently accepted long-header packets
with bits 0x0C set or short-header packets with bits 0x18 set after
HP unmasking — the spec mandates PROTOCOL_VIOLATION close. Added
[QuicProtocolViolationException] and a top-level catch in
feedDatagram that translates the throw into markClosedExternally.
Long-header parse also drops a now-dead `if form==1` branch on the
first-byte mask: we already early-returned in non-long paths above,
so the mask is always 0x0F.
* TLS handshake-message bounds:
- TlsCertificateChain.decodeBody: reject `listLen > r.remaining`
up front; assert `r.position == end` after the cert loop.
Without this, a malicious peer could push us into reading past
the message limit on per-cert extensions.
- TlsServerHello.decodeBody / TlsEncryptedExtensions.decodeBody:
reject trailing bytes after the extensions block.
- TlsEncryptedExtensions.alpn: enforce RFC 7301 §3.1 (server
returns EXACTLY one protocol_name); validate outerLen matches
remaining and reject multi-name responses.
* SendBuffer.data shrink: pre-fix the doubling-on-grow buffer never
shrank, so a stream that ever held N bytes pinned `data.size = N`
for the connection's lifetime. Long-tail memory retention on
per-stream basis. advanceFlushedFloorIfPossible now releases
capacity once live bytes occupy ≤ 1/4 of the allocation, shrinking
to max(SHRINK_FLOOR_BYTES=4096, 2*dataLen). Below the floor the
doubling cost is negligible; above it the multi-MiB transients
release back to the heap.
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 47s.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Round 3 of the audit follow-ups.
* Http3FrameReader gains a [StreamContext] parameter that enforces
RFC 9114 §7.2 per-stream rules:
- CONTROL: first frame MUST be SETTINGS (else H3_MISSING_SETTINGS);
duplicate SETTINGS, DATA, HEADERS, PUSH_PROMISE all forbidden.
- REQUEST: SETTINGS / GOAWAY / MAX_PUSH_ID / CANCEL_PUSH forbidden.
- PUSH: similar set including PUSH_PROMISE.
- Reserved types 0x02 / 0x06 / 0x08 / 0x09 explicitly rejected.
- WT_BIDI_DATA / WT_UNI_DATA: reader is the wrong tool, throw.
- UNCHECKED preserves prior test behaviour and is the default.
WtPeerStreamDemux's CONTROL drain now constructs the reader with
StreamContext.CONTROL, so a buggy server can no longer slip a DATA
frame into our SETTINGS expectations and silently confuse the
parser. The validation throws QuicCodecException, which the
drainControlStream catch records on a new peerH3ProtocolError
field — the QUIC layer / application reads it to close with the
proper diagnostic instead of having the route() catch swallow it.
* ReceiveBuffer no longer coalesces overlapping segments on insert.
Pre-fix every reorder fill allocated a fresh merged ByteArray of
size (hi - lo) and copyInto'd each existing segment — under a 200-
chunk reorder burst that was O(N²) bytes. The new layout keeps
segments as a sorted, non-overlapping list (binary-searched on
insert) and only allocates at readContiguous time, where it walks
consecutive segments and concats them in a single pass. Adjacent
segments are not eagerly merged — the read-side concat is bounded
by the contiguous prefix the consumer is about to drain anyway.
bufferedAhead becomes O(1) (cached counter) instead of O(N) sum.
* New tests cover the per-context rejection paths (CONTROL-stream
first-frame check, DATA-on-control, SETTINGS-on-request, all four
reserved frame types).
All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Adds a "Hide posts that violate community rules" toggle in Security & Filters
that drops events from community feeds when their author/kind/size fails the
latest cached `kind:34551` (NIP-9A) rules document for that community.
Default OFF preserves pre-9A behaviour. Closes#2761.
When the toggle is on, both `CommunityFeedFilter` (approval-only feed) and
`CommunityModerationFeedFilter` (un-approved feed) construct a per-feed
`CommunityRulesValidator` from the latest `kind:34551` for the community and
drop candidates whose `validate(...)` returns a violation. When no rules
event is cached for the community, the validator is null and the filter
behaves exactly as before — no false positives on pre-9A communities.
`CommunityRulesFilterSubAssembler` (new) joins the existing
`CommunityFilterAssembler.group`, reusing the `CommunityQueryState` keyspace
so any screen mounting the community feed subscription also pulls the rules
document. Filter: `kinds=[34551]`, `authors=<owner+moderators>`, `#a=<addressTag>`.
`CommunityRulesLookup.kt` extracts the cache scan + violation check into pure
helpers (`latestCommunityRules`, `violatesCommunityRules`) so the filter
wiring is unit-testable without LocalCache. The `Account.settings.hideCommunityRulesViolations`
flag follows the existing `useLocalBlossomCache` plumbing pattern: persisted
in `LocalPreferences`, mutated through a `change*` setter on `AccountSettings`,
read into the feed view models at construction time.
Out of scope (deferred to follow-up issues): web-of-trust gates, per-day
quota enforcement (`postsTodayByKind`), stale-rules ratchet UI, "Send anyway"
override on validation. The validator skips wot/quota cleanly when their
callbacks are null, so the feed filter is a strict subset of NIP-9A's
checks for v1.
7 unit tests on `violatesCommunityRules`:
- comment passes when its kind is whitelisted
- text note fails when only comments are whitelisted
- denied author overrides any kind allow-list
- oversize per-kind, under-size per-kind, global max-event-size
- note without an event is treated as passing
Builds clean: `:amethyst:assemblePlayDebug`, `:amethyst:spotlessCheck`,
`:amethyst:testPlayDebugUnitTest --tests "*CommunityRulesLookupTest*"`.
Note: this PR also lands a copy of `CommunityRulesFilterSubAssembler`
identical to the one in PR #2798 (composer-side validation). When either
PR merges first, the other rebases to drop the duplicate; the file is the
same in both.
Round 2 of the audit follow-ups. Each item caps a peer-controlled
allocation that pre-fix could be inflated to hold gigabytes of heap
or pin a CPU core indefinitely.
* Http3FrameReader: cap pending unparsed buffer (1 MiB) and per-frame
body length (16 MiB). A peer streaming a partial-frame prefix
without ever delivering the body now raises QuicCodecException
instead of growing buf indefinitely.
* CapsuleReader: cap pending buffer (1 MiB) and per-capsule body
(64 KiB). Symmetric encoder-side check on WT_CLOSE_SESSION reason
size, matching the existing decoder cap.
* WtPeerStreamDemux: replace Channel.UNLIMITED with bounded channels
+ suspending sends. readyStreams now caps queued peer-initiated
streams at 1024; per-stream chunkChannel caps at 64 chunks. The
collector's suspending send naturally back-pressures via QUIC flow
control when the application is slow, rather than pinning heap.
* WtDatagram.decode: validate quarter-id is in [0, (2^62-1)/4] so
`r.value * 4` cannot overflow Long and wrap into a small signed
value matching our session id (cross-session datagram injection).
* QuicReader.readBytes / skip: translate negative-count into typed
QuicCodecException instead of letting IllegalArgumentException
escape from copyOfRange.
* AckTracker: cap stored disjoint ranges at 64. A peer that sends
alternating-bit-pattern PNs can no longer grow our ACK frame past
what fits in a packet; oldest range evicts on overflow.
* JcaAesGcmAead: track recent encrypt nonces (8) instead of just the
most-recent, so a single intermediate seal between two rebuilds
can't mask a duplicate against the second-most-recent. Drop the
remembered nonce on doFinal failure so a retry takes the safe
fresh-Cipher path. Add synchronized() defence-in-depth.
Each cap has a generous default (above any legitimate use) but
finite. Tests use no-arg construction; existing call sites unaffected.
All 269 :quic:jvmTest tests pass.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
Adds an opt-in editor for NIP-9A `kind:34551` community rules alongside the
existing freeform `rules` text on `kind:34550`. Closes#2760.
Editor sections in `NewCommunityScreen` (rendered after relays):
- Allowed event kinds: filter chips for common community kinds (1, 20, 21,
22, 1111, 30023) plus a custom-kind input. Per-kind limits dialog (long
press / edit icon) for `max-bytes` and `max-per-author-per-day`.
- Banned users: pubkey list with `deny` policy, picked through the same
user-suggestion field the moderator picker uses.
- Web-of-trust gate (optional): root npub-or-hex pubkey + depth, repeatable.
- Global max event size (optional): single bytes input.
`NewCommunityModel` carries the four collections/values as Compose state and
exposes a pure-helper `buildRulesPayload(...)` companion so the mapping from
editor drafts to Quartz tag types is unit-testable without an Account.
`Account.sendCommunityRules(...)` mirrors `sendCommunityDefinition(...)`:
builds a `CommunityRulesEvent` via the Quartz builder, signs with the same
key, and broadcasts on the same outbox path. Reuses the community's `dTag`
so the rules event replaces in place across edits.
`NewCommunityModel.publish(...)` only emits the rules event when at least
one structured rule is present (`hasStructuredRules()`), so existing
communities and form runs that don't touch the new section continue to
behave exactly as before.
Migrating existing communities, the `min_rules_created_at` ratchet UI, and
edit-flow preload of structured rules from `kind:34551` are deliberately
out of scope here (see issue #2760).
7 unit tests on the pure helper:
- empty editor produces a null payload (no event published)
- max-event-size only is enough to publish
- per-kind limits round-trip into the tag
- bare kind rule serialises to ["k", "<kind>"] (no empty fields)
- banned pubkey writes a deny rule
- WoT gate carries root pubkey + depth
- pubkey-input parser accepts hex and npub, rejects garbage
Builds clean: `:amethyst:assemblePlayDebug`, `:amethyst:spotlessCheck`,
`:amethyst:testPlayDebugUnitTest --tests "*NewCommunityModelRulesTest*"`.
Verified and applied 12 focused fixes from a four-agent review of the
quic module. Each fix verified against the actual code; agent
findings that traced to false positives (pendingPing clear-without-emit,
sentPackets on encrypt failure) are documented in the review thread
but not changed.
Concurrency / flakiness:
- PTO consecutive count: double-increment removed; now incremented
exactly once per PTO event in handlePtoFired before requeue, so the
threshold check inside requeueInflightForProbe sees the post-
increment value AND the between-probe re-requeue doesn't bump again.
- Wallclock → monotonic: QuicConnection.nowMillis defaults to
TimeSource.Monotonic anchored at construction, so NTP step /
suspend-resume can't poison RTT samples. Driver uses
connection.nowMillis instead of carrying its own clock.
- Close-state race: atomic CAS via closeStateMonitor in close() and
markClosedExternally so concurrent teardown paths can't both fire
qlog "connection closed" and stomp on closeReason.
- streamsList CME: converted to @Volatile var List<QuicStream> with
immutable-snapshot publishing under streamsLock. closeAllSignals'
iteration is now CME-free without holding the (suspending) lock.
- JcaAesGcmAead: synchronized seal/open; multi-entry recent-nonce
history (was single most-recent — could mask a duplicate against
the second-most-recent under intermediate seals); on seal failure
evict the cached nonce so a retry with the same nonce takes the
safe fresh-Cipher path.
Wire correctness / spec:
- Connection-level send credit no longer debited on retransmits
(added Chunk.isRetransmit; writer skips sendConnectionFlowConsumed
+= data.size when set). Pre-fix a few PTO rounds on a long stream
exhausted credit and stalled the connection.
- ACK-delay shl overflow: clamp ackDelayExponent to 0..20 and
clamp the peer's varint to (Long.MAX_VALUE >>> exponent) before
shift; clamp negative now-vs-recv-time before shift on outbound
AckTracker.
- Key-update commit only when new-phase packet PN exceeds
largestReceived (RFC 9001 §6.1).
- RESET_STREAM final-size validation: enforce equality with prior
FIN size and ≥ highestObservedOffset; close FINAL_SIZE_ERROR
otherwise.
- STOP_SENDING handling: respond with RESET_STREAM on the local
send side (was silently dropped, peer's flow-credit wasted).
- ReceiveBuffer.insert: typed InsertResult; second FIN with
conflicting size and offset-past-FIN data both surface
FINAL_SIZE_ERROR via the parser instead of being silently dropped.
- Retry SCID==DCID self-loop: reject Retry where the peer's SCID
equals our original DCID (RFC 9000 §17.2.5.2).
DoS hardening:
- ACK PN walk: drainAckedSentPackets rewritten to scan in-flight
keys against parsed ranges instead of walking every PN. A peer
with firstAckRange = 2^62-1 used to pin a core forever; now
bounded by the sent-packets map size.
- UDP socket: channel.connect(remote) after bind so the kernel
filters off-path datagrams. Stops trivial source spoofing from
burning AEAD attempts.
https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
When the comment composer is replying into a NIP-72 community
(`replyingTo.event is CommunityDefinitionEvent`), subscribe to the
community's latest `kind:34551` rules document, run
`CommunityRulesValidator.validate(...)` on every draft change, render an
inline banner above the bottom action row when the draft would be
rejected, and disable the post button until the violation is resolved.
- `CommunityRulesFilterSubAssembler` is added to the existing
`CommunityFilterAssembler.group` so any screen that already mounts
`CommunityFilterAssemblerSubscription` (community feed, this composer)
also pulls the latest rules event from the community's relays. Filter
is `kinds=[34551]`, `authors=<owner+moderators>`, `#a=<addressTag>`,
matching the NIP-72 trust model.
- `CommentPostViewModel` observes `kind:34551` via
`LocalCache.observeEvents` keyed on the reply target, picks the latest
`created_at` matching the community address, and re-runs the validator
on every draft change. The validator's `postsTodayByKind` and `wot`
callbacks are intentionally null for this PR (per-day quota lookups
and NIP-02 follow-graph traversal are deferred to follow-ups; the
validator skips those checks cleanly).
- Draft size is conservatively estimated from `content.toByteArray(UTF-8)`
— tags add bytes, so this can under-count on the boundary, but relays
still enforce the real cap. Good enough for a pre-send preview.
- `CommunityRulesViolationBanner` renders the first
`CommunityRulesValidator.Violation` with a localized message; new
strings cover all 7 sealed-violation variants.
- `CommentPostViewModelTest` covers valid drafts, kind-not-allowed,
oversize, denied-author, the under-size boundary, and multibyte UTF-8
size accounting.
Compose state — not StateFlow — backs `validationResult` and
`communityRules` so `canPost()` and the banner recompose without an
explicit `collectAsState` site at the top bar (`isActive` reads
`validationResult` directly).
Refs nostr-protocol/nips#2331
Closes#2759
Surface a soft "this content's relay may be stale" UX cue on addressable
replaceable events (kind:30xxx) when every delivering relay's most recent
NIP-66 kind:30166 Relay Discovery monitor report cached locally is older
than 14 days (or has never been observed).
Read-only: uses only what's already in LocalCache, populated by the
existing RelayInfoNip66FilterSubAssembler. No new network fetches.
Implementation:
- New `StaleRelayHint` composable in `ui/note/elements/`, hooked into
`NoteBody` after the zap-splits row. Skips quietly for non-addressable
events, empty relay sets, or any relay still observed within 14 days.
- Pure `isStaleByLatestMonitorReports(latestPerRelay, now, threshold)`
predicate — `null` (never monitored) and "older than cutoff" both count
as stale, but a single fresh relay short-circuits the hint to off.
- Reactively tracks the note's relay set via
`baseNote.flow().relays.stateFlow` so newly-observed relays update the
hint without recomposition tricks.
- Latest monitor `created_at` per relay is read from `LocalCache` with
the same `Filter(kinds=[30166], #d=[relay.url], limit=1)` shape used
on the Relay Information screen.
Out of scope (per issue):
- Auto-refreshing or hiding stale content.
- Heuristics beyond the "all delivering relays stale" check.
Tests: 9 unit tests for the pure predicate covering empty/single/mixed
sets, null-handling, exact-cutoff boundary, and a custom threshold.
Closes#2762.