fix(core): fix client crashes on large TLS responses#15
Merged
bluestreak01 merged 5 commits intomainfrom Apr 25, 2026
Merged
Conversation
sklarsa
force-pushed
the
fix/tls-unwrap-buffering
branch
from
b287cb1 to
cfae5fd
Compare
recv() returned 0 from the OK branch whenever SSLEngine.unwrap consumed input but produced no plaintext (for example after a TLS 1.3 NewSessionTicket). The premature return left buffered records sitting in the input buffer, so callers fell back to ioWait and could time out despite having decryptable data on hand. Keep looping instead so the next record is unwrapped. While in there, restore the zero-copy fast path: SSLEngine now decrypts straight into the caller's buffer through a placeholder ByteBuffer repointed at recv() time. The internal buffer is used only when a single TLS record does not fit in the caller's buffer, in which case it grows as needed and any leftover plaintext is drained on subsequent recv() calls. Add tests for the empty-OK loop continuation, the spill-and-grow path, and the existing overflow-then-drain path; share SSLEngine stub boilerplate via a StubSslEngine base. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bluestreak01
changed the title
bluestreak01
changed the title
Adds the /review-pr skill that performs a QuestDB-flavoured code review of a GitHub PR: PR metadata, parallel correctness / concurrency / performance / resource / test / quality / metadata / Rust-safety agents, source-verification of every draft finding to strip false positives, and a structured Critical / Moderate / Minor report with a Downgraded section explaining dismissed findings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bluestreak01
approved these changes
Apr 25, 2026
Contributor
[PR Coverage check]😍 pass : 38 / 57 (66.67%) file detail
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
JavaTlsClientSocket.recv()so a single TLS record larger than the caller's buffer no longer tripsOutput buffer too small to fit a single TLS record. Oversized plaintext is staged in an internal buffer and drained across subsequentrecv()calls.recv()to keep unwrapping afterSSLEngine.unwrap()returnsOKwith no plaintext produced (e.g. after a TLS 1.3NewSessionTicket). The previousreturn 0short-circuit left buffered records sitting in the input buffer and made callers likeHttpClient.recvOrDiefall back toioWaitand time out despite having decryptable data on hand.SSLEnginedecrypts straight into the caller's buffer in the common case via a placeholderByteBufferrepointed atrecv()time. The internal spill buffer is used only when a single record does not fit, in which case it grows as needed.StubSslEnginebase.Tooling
.claude/skills/review-pr/so future/review-prruns against this repo apply the same QuestDB review conventions used to vet the changes here.Testing
JAVA_HOME=$(/usr/libexec/java_home -v 17) mvn -pl core test(1493 passing, 1 skipped)JAVA_HOME=$(/usr/libexec/java_home -v 17) mvn -pl core -Dtest=JavaTlsClientSocketTest test