diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md new file mode 100644 index 00000000..80c3dbf3 --- /dev/null +++ b/.claude/skills/review-pr/SKILL.md @@ -0,0 +1,210 @@ +--- +name: review-pr +description: Review a GitHub pull request against QuestDB coding standards +argument-hint: [PR number or URL] +allowed-tools: Bash(gh *), Read, Grep, Glob, Agent +--- + +Review the pull request `$ARGUMENTS`. + +## Review mindset + +You are a senior QuestDB engineer performing a blocking code review. QuestDB is mission-critical software deployed on spacecraft — bugs can cause data loss or system failures that cannot be patched after deployment. There is zero tolerance for correctness issues, resource leaks, or undefined behavior. Be critical, thorough, and opinionated. Your job is to catch problems before they ship, not to be nice. + +- **Assume nothing is correct until you've verified it.** Read surrounding code to understand context — don't just look at the diff in isolation. +- **Flag every issue you find**, no matter how small. Do not soften language or hedge. Say "this is wrong" not "this might be an issue". +- **Do not praise the code.** Skip "looks good", "nice work", "clever approach". Focus entirely on problems and risks. +- **Think adversarially.** For each change, ask: what inputs break this? What happens under concurrent access? What if this runs on a 10-billion-row table? What if the column is NULL? What if the partition is empty? +- **Check what's missing**, not just what's there. Missing tests, missing error handling, missing edge cases, missing documentation for non-obvious behavior. +- **Verify every claim.** If the PR title says "fix", verify the bug actually existed and the fix is correct. If it says "improve performance", look for benchmarks or reason about the algorithmic change — does it actually improve things, or could it regress in other cases? If it says "simplify", verify the new code is actually simpler and doesn't drop behavior. Treat the PR description as an unverified hypothesis, not a statement of fact. +- **Read the full context of changed files** when the diff alone is ambiguous. Use Read/Grep/Glob to inspect the surrounding code, callers, and related tests. +- **Assess reachability before reporting.** For every potential bug, trace the actual callers and inputs. If a problem + requires physically impossible conditions (billions of columns, corrupted JNI inputs, values that no caller can + produce), it is not a real finding — drop it. Focus on bugs that real workloads can trigger, not theoretical edge + cases that exist only in the type system. +- **QuestDB runs with Java assertions enabled (`-ea`).** Assertions are a valid guard for invariants that indicate + corruption or internal bugs. Do NOT flag `assert` as insufficient — it is the preferred mechanism for conditions + that should never occur in a non-corrupt database. Only flag an `assert` if the condition can plausibly be triggered + by normal (non-corrupt) user operations. + +## Step 1: Gather PR context + +Fetch PR metadata, diff, and any review comments: + +```bash +gh pr view $ARGUMENTS --json number,title,body,labels,state +gh pr diff $ARGUMENTS +gh pr view $ARGUMENTS --comments +``` + +## Step 2: PR title and description + +Check against CLAUDE.md conventions: +- Title follows Conventional Commits: `type(scope): description` +- Description repeats the verb (e.g., `fix(sql): fix ...` not `fix(sql): DECIMAL ...`) +- Description speaks to end-user impact, not implementation internals +- If fixing an issue, `Fixes #NNN` is at the top of the body +- Tone is level-headed and analytical, no superlatives or bold emphasis on numbers +- Labels match the PR scope (SQL, Performance, Core, etc.) + +## Step 3: Parallel review + +Launch the following agents in parallel. Each agent receives the full PR diff and should read surrounding source files as needed for context. + +**Agent 1 — Correctness & bugs:** NULL handling, edge cases, logic errors, off-by-one, operator precedence, error paths. + +**Agent 2 — Concurrency:** Race conditions, shared mutable state, missing volatile, lock ordering, thread-safety of data structures. + +**Agent 3 — Performance & allocations:** Regressions, zero-GC violations, `java.util.*` collections vs `io.questdb.std`, string creation/concatenation on hot paths, SIMD opportunities. Algorithmic complexity: for each new loop, traversal, or data structure, analyze how it scales with data size (row count, partition count, join fan-out). Flag any O(n^2) or worse patterns that could regress on large tables (1M+ rows, 1000+ partitions). Check whether new code paths are compile-time-only or data-path — compile-time allocations are acceptable, data-path allocations are not. + +**Agent 4 — Resource management:** Leaks on all code paths (especially errors), try-with-resources, native memory, pool management. + +**Agent 5 — Test review & coverage:** Coverage gaps, error path tests, NULL tests, boundary conditions, regression tests, test quality, `assertMemoryLeak()` usage. + +**Agent 6 — Code quality & standards:** Code smell, member ordering, naming conventions, modern Java features, dead code, third-party dependencies. + +**Agent 7 — PR metadata & conventions:** Title format, description quality, commit messages, labels, SQL style in tests. + +**Agent 8 — Rust safety (only if PR contains .rs files):** Check for any code that can panic at runtime — `unwrap()`, +`expect()`, array indexing without bounds checks, `panic!()`, `unreachable!()`, `todo!()`, integer overflow in release +mode, `slice::from_raw_parts` with invalid inputs. In mission-critical software a panic in Rust code called via JNI/FFI +will abort the entire JVM process with no recovery. Every fallible operation must use `Result`/`Option` with proper +error propagation. Flag every potential panic site. + +Combine all agent findings into a single deduplicated **draft** report. Do NOT present this draft to the user yet — it goes straight into verification. + +## Step 3b: Verify every finding against source code + +The parallel review agents work from the diff alone and frequently produce false positives — especially around memory ownership, polymorphic dispatch, Rust control-flow guarantees, and JNI lifecycle conventions. Every finding MUST be verified before it is reported. + +For each finding in the draft report: + +1. **Read the actual source code** at the exact lines cited. Do not rely on the agent's description alone. +2. **Trace the full code path**: follow callers, inheritance hierarchies, and runtime types. A method called on a base-class reference may dispatch to a subclass override (e.g., `PartitionDescriptor.clear()` vs `OwnedMemoryPartitionDescriptor.clear()`). +3. **Check both sides of JNI/FFI boundaries**: if a finding involves Java↔Rust interaction, read both the Java caller and the Rust JNI function. Verify ownership transfer, error propagation, and cleanup on both sides. +4. **For resource leak claims**: trace every allocation to its corresponding free/close on ALL code paths (happy path, + error path, finally blocks). Check for polymorphic `close()`/`clear()` overrides. Before claiming a leak between + allocation and cleanup registration, verify that the intervening code can actually throw. +5. **For Rust panic claims**: verify whether the panic site is actually reachable. Trace control flow backwards — a + preceding guard or early return may make it unreachable. +6. **For Rust panic claims via JNI**: trace the Java caller to check whether it can actually pass parameters that + trigger the panic. If every caller validates inputs before the JNI call, the panic is unreachable — drop it. +7. **For Rust numeric overflow claims**: check whether the overflow is reachable at realistic scale. QuestDB handles + billions to a few trillion rows, thousands of tables, and thousands of columns — not billions of columns or + quintillions of rows. If overflow requires values beyond that scale, drop it. +8. **For performance claims**: check whether the cost is measurable in a realistic scenario. Downgrade to a nit if the + saving is negligible relative to the surrounding work. Exception: GC allocations on a hot path are always worth + flagging, even a single one. +9. **Classify each finding** as: + - **CONFIRMED** — the bug is real and reproducible via the traced code path + - **FALSE POSITIVE** — the code is actually correct (explain why) + - **CONFIRMED with nuance** — the issue exists but is less severe than stated (explain) + +**Move false positives to a separate "Downgraded" section** at the end of the report. For each, give a one-line explanation of why it was dismissed. This lets the PR author verify the reasoning and catch verification mistakes. + +Launch verification agents in parallel where findings are independent. Each verification agent should read surrounding source files, not just the diff. + +## Review checklists + +Review the diff for: + +### Correctness & bugs +- NULL handling: distinguish sentinel NULL vs actual NULL +- Edge cases and error paths +- SqlException positions point at the offending character, not the expression start +- Logic errors, off-by-one, incorrect bounds, wrong operator precedence + +### Concurrency +- Race conditions: unsynchronized shared mutable state, missing volatile, unsafe publication +- Lock ordering issues that could cause deadlocks +- Thread-safety of data structures used across threads + +### Performance +- Performance regressions: changes that make hot paths slower or increase complexity +- Unnecessary allocations on data paths (zero-GC requirement) +- Use of `java.util.*` collections (HashMap, ArrayList, etc.) instead of QuestDB's own zero-GC collections in `io.questdb.std` +- String creation or concatenation on hot paths (use CharSink, StringSink, or direct char[] instead) +- Capturing lambdas on hot paths — lambdas that capture local variables or instance fields allocate a new object on every invocation. Non-capturing lambdas (static method refs, no closed-over state) are safe as the JVM caches them. Flag any capturing lambda on a data path. +- Autoboxing on hot paths — primitive-to-wrapper conversions (`int` → `Integer`, `long` → `Long`, etc.) allocate silently. Watch for primitives passed to generic methods, stored in `java.util.*` collections, or returned from methods with wrapper return types. +- Missing SIMD or vectorization opportunities +- Inefficient algorithms where QuestDB already provides optimized alternatives +- Algorithmic complexity at scale: for each new loop or traversal, what is the time complexity as a function of row count, partition count, or join fan-out? Flag O(n^2) or worse patterns. Consider: what happens with 1M outer rows? 10K partitions? 100-way fan-out per row? +- Compile-time vs data-path distinction: allocations and O(n) scans during SQL compilation/optimization are acceptable; the same on per-row data paths are not + +### Code quality +- Code smell: overly complex methods, deep nesting, unclear intent, dead code +- No third-party Java dependencies on data paths + +### QuestDB coding standards +- Class members grouped by kind (static vs instance) and visibility, sorted alphabetically +- Boolean names use `is...` / `has...` prefix +- Modern Java features: enhanced switch, multiline strings, pattern variables in instanceof + +### Resource management +- Resources properly closed in all code paths (especially error paths) +- try-with-resources used where applicable +- Native memory freed correctly + +### SQL conventions (if tests or SQL involved) +- Keywords in UPPERCASE +- `expr::TYPE` cast syntax preferred over CAST() +- Underscores in numbers >= 5 digits (e.g., 1_000_000) +- Multiline strings for complex queries +- No DELETE statements (suggest DROP PARTITION or soft delete) +- Tests use `assertMemoryLeak()`, `assertQueryNoLeakCheck()`, `execute()` for DDL +- Single INSERT for multiple rows + +### Enterprise permissions & ACL (if PR introduces new SQL statements or ALTER operations) +- New ALTER TABLE operations almost always require a new enterprise permission. If the PR adds a new ALTER statement (or any new SQL statement that modifies state), flag it if there is no corresponding `SecurityContext.authorize*()` call in the execution path. +- New features in OSS should have an enterprise counterpart that wires up ACL. Check whether the PR introduces `authorize*` methods in `SecurityContext` and whether all enterprise `SecurityContext` implementations (`EntSecurityContextBase`, `AdminSecurityContext`, `AbstractReplicaSecurityContext`, and test mocks) are updated. +- New permissions must be registered in `Permission.java` (constant, name maps, and included in `TABLE_PERMISSIONS`/`ALL_PERMISSIONS` as appropriate). +- The `PermissionParser` must be able to parse GRANT/REVOKE for the new permission name — especially if the name contains SQL keywords like `ON`, `TO`, or `FROM` that could conflict with parser grammar. +- Replica security contexts must deny new write operations (`deniedOnReplica()`). + +### Test review +- **Coverage gaps:** For every new or changed code path, verify a corresponding test exists. If not, flag it explicitly as "missing test for X". +- **Error path coverage:** Are failure cases, exceptions, and edge conditions tested — not just the happy path? +- **NULL tests:** Are NULL inputs, NULL columns, and NULL expression results tested? +- **Boundary conditions:** Empty tables, empty partitions, single-row tables, max-value inputs, zero-length strings. +- **Concurrency tests:** If the code touches shared state, are there tests that exercise concurrent access? +- **Resource leak tests:** Tests must use `assertMemoryLeak()` for anything that allocates native memory. +- **Test quality:** Are tests actually asserting the right thing? Watch for tests that pass trivially, assert on wrong values, or test implementation details instead of behavior. +- **Regression tests:** If this PR fixes a bug, is there a test that reproduces the original bug and would fail without the fix? +- Use Grep/Glob to find existing test files for the changed classes and verify they cover the new behavior. + +### Unresolved TODOs and FIXMEs +- Scan the diff for `TODO`, `FIXME`, `HACK`, `XXX`, and `WORKAROUND` comments. For each one found: + - Is it a pre-existing comment that was just moved/reformatted, or newly introduced in this PR? + - If newly introduced: does it represent unfinished work that should block the merge, or a known limitation that is acceptable to ship? Flag any that look like deferred bugs or incomplete implementations. + - If the TODO references a ticket/issue number, verify the reference exists. + +### Commit messages +- Plain English titles (no Conventional Commits prefix), under 50 chars +- Full long-form body description, line breaks at 72 chars +- Active voice, naming the acting subject + +## Step 4: Output + +Present ONLY verified findings (false positives are excluded). Structure as: + +### Critical +Issues that must be fixed before merge. Each must include: +- Exact file path and line numbers +- Code path trace showing why the bug is real +- Suggested fix + +### Moderate +Issues worth addressing but not blocking. + +### Minor +Style nits and suggestions. + +### Downgraded (false positives) +Findings from the initial review that were dismissed after source code verification. For each, state: +- The original claim (one line) +- Why it was dismissed (one line, citing the specific code that disproves it) + +### Summary +- One-line verdict: approve, request changes, or needs discussion +- Highlight any regressions or tradeoffs +- State how many draft findings were verified vs dropped as false positives (e.g., "8 findings verified, 4 false positives removed") diff --git a/core/src/main/java/io/questdb/client/network/JavaTlsClientSocket.java b/core/src/main/java/io/questdb/client/network/JavaTlsClientSocket.java index 89610878..4d363fbb 100644 --- a/core/src/main/java/io/questdb/client/network/JavaTlsClientSocket.java +++ b/core/src/main/java/io/questdb/client/network/JavaTlsClientSocket.java @@ -77,6 +77,7 @@ public X509Certificate[] getAcceptedIssuers() { private static final int STATE_EMPTY = 0; private static final int STATE_PLAINTEXT = 1; private static final int STATE_TLS = 2; + private final ByteBuffer callerOutputBuffer; private final Socket delegate; private final Logger log; private final ClientTlsConfiguration tlsConfig; @@ -87,6 +88,7 @@ public X509Certificate[] getAcceptedIssuers() { private SSLEngine sslEngine; private int state = STATE_EMPTY; private long unwrapInputBufferPtr; + private long unwrapOutputBufferPtr; private long wrapOutputBufferPtr; JavaTlsClientSocket(NetworkFacade nf, Logger log, ClientTlsConfiguration tlsConfig) { @@ -94,18 +96,19 @@ public X509Certificate[] getAcceptedIssuers() { this.log = log; this.tlsConfig = tlsConfig; - // wrapInputBuffer are just placeholders. we set the internal address, capacity and limit in send() and recv(). - // so read/write from/to a buffer supplied by the caller and avoid unnecessary memory copies. - // also, handshake does not to read/write from/to these buffers so it does not matter if they have capacity = 0 - // during handshake. + // wrapInputBuffer and callerOutputBuffer are just placeholders: their address, capacity and + // limit are reset to point at the caller's buffer in send() and recv() respectively, so the + // SSLEngine can read/write the caller's memory directly without an extra copy. They have + // capacity = 0 during handshake because handshake does not touch them. this.wrapInputBuffer = ByteBuffer.allocateDirect(0); - this.unwrapOutputBuffer = ByteBuffer.allocateDirect(0); + this.callerOutputBuffer = ByteBuffer.allocateDirect(0); - // wrapOutputBuffer and unwrapInputBuffer are crated with capacity 0. why? - // we allocate the actual memory only when starting a new TLS session. - // this way we can reuse the same ByteBuffer instances for multiple TLS sessions. + // wrapOutputBuffer, unwrapInputBuffer and unwrapOutputBuffer all back internal allocations + // that are only created when starting a new TLS session, so the ByteBuffer instances can be + // reused across sessions. this.wrapOutputBuffer = ByteBuffer.allocateDirect(0); this.unwrapInputBuffer = ByteBuffer.allocateDirect(0); + this.unwrapOutputBuffer = ByteBuffer.allocateDirect(0); } @Override @@ -167,51 +170,73 @@ public void of(int fd) { public int recv(long bufferPtr, int bufferLen) { assert sslEngine != null; - resetBufferToPointer(unwrapOutputBuffer, bufferPtr, bufferLen); - unwrapOutputBuffer.position(0); - try { - int plainBytesReceived = 0; + // Pending plaintext from a previous spill is held in the internal buffer. Drain it first. + if (unwrapOutputBuffer.position() != 0) { + return drainUnwrapOutputBuffer(bufferPtr, bufferLen); + } + + // Fast path: have the SSLEngine decrypt straight into the caller's buffer. We only fall + // back to the internal spill buffer if a single TLS record does not fit in the caller's + // buffer, in which case we drain the spill buffer to the caller and return. + resetBufferToPointer(callerOutputBuffer, bufferPtr, bufferLen); + ByteBuffer output = callerOutputBuffer; + boolean spilling = false; + for (; ; ) { int n = readFromSocket(); assert unwrapInputBuffer.position() == 0 : "unwrapInputBuffer is not compacted"; int bytesAvailable = unwrapInputBuffer.limit(); if (n < 0 && bytesAvailable == 0) { - if (plainBytesReceived == 0) { - // we didn't manage to read anything from the socket, let's return the error - return n; + if (output.position() != 0) { + return spilling ? drainUnwrapOutputBuffer(bufferPtr, bufferLen) : output.position(); } - // we have some data to return, let's return it - return plainBytesReceived; + return n; } - if (bytesAvailable == 0) { - // nothing to unwrap, we are done - return plainBytesReceived; + if (output.position() != 0) { + return spilling ? drainUnwrapOutputBuffer(bufferPtr, bufferLen) : output.position(); + } + return 0; } - SSLEngineResult result = sslEngine.unwrap(unwrapInputBuffer, unwrapOutputBuffer); - plainBytesReceived += result.bytesProduced(); + SSLEngineResult result = sslEngine.unwrap(unwrapInputBuffer, output); // compact the TLS buffer int bytesConsumed = result.bytesConsumed(); int bytesRemaining = bytesAvailable - bytesConsumed; - Vect.memcpy(unwrapInputBufferPtr, unwrapInputBufferPtr + bytesConsumed, bytesRemaining); + if (bytesRemaining > 0) { + Vect.memcpy(unwrapInputBufferPtr, unwrapInputBufferPtr + bytesConsumed, bytesRemaining); + } unwrapInputBuffer.position(0); unwrapInputBuffer.limit(bytesRemaining); switch (result.getStatus()) { case BUFFER_UNDERFLOW: - // we need more data to unwrap, let's return whatever we have - return plainBytesReceived; + if (output.position() != 0) { + return spilling ? drainUnwrapOutputBuffer(bufferPtr, bufferLen) : output.position(); + } + return 0; case BUFFER_OVERFLOW: - if (unwrapOutputBuffer.position() == 0) { - // not even a single byte was written to the output buffer even the buffer is empty - throw new AssertionError("Output buffer too small to fit a single TLS record. This should not happen, please report as a bug."); + if (output.position() != 0) { + // Output already has plaintext: hand it to the caller and let the + // unprocessed record be picked up on the next recv() call. + return spilling ? drainUnwrapOutputBuffer(bufferPtr, bufferLen) : output.position(); } - // we have some data to return, let's return it - return plainBytesReceived; + if (spilling) { + // Internal buffer cannot fit a single record either: grow and retry. + growUnwrapOutputBuffer(); + } else { + // Caller's buffer cannot fit a single record. Switch to the internal + // spill buffer for this record (and any further records that fit), then + // drain to the caller. + output = unwrapOutputBuffer; + spilling = true; + } + break; case OK: + // Plaintext (if any) is accumulating in `output`. Keep looping so we either + // batch more records or hit BUFFER_UNDERFLOW / BUFFER_OVERFLOW. break; case CLOSED: log.debug("SSL engine closed"); @@ -220,7 +245,10 @@ public int recv(long bufferPtr, int bufferLen) { // If a caller calls recv() again and we have no remaining plaintext to return, we will return -1 so the // caller learned that the connection is closed. // If we have no plaintext data to return now then we can immediately indicate that we are done with the connection. - return plainBytesReceived == 0 ? -1 : plainBytesReceived; + if (output.position() != 0) { + return spilling ? drainUnwrapOutputBuffer(bufferPtr, bufferLen) : output.position(); + } + return -1; } } } catch (SSLException e) { @@ -303,9 +331,17 @@ public void startTlsSession(CharSequence peerName) throws TlsSessionInitFailedEx // there cannot be underflow since wrap() during handshake does not read from the input buffer at all throw new AssertionError("Buffer underflow during TLS handshake. This should not happen. please report as a bug"); case BUFFER_OVERFLOW: - // in theory, this can happen if the output buffer is too small to fit a single TLS handshake record, but that would indicate - // our starting buffer is too small. - throw new AssertionError("Buffer overflow during TLS handshake. This should not happen, please report as a bug"); + if (wrapOutputBuffer.position() != 0) { + // wrap() left bytes behind without producing a complete record. The OK + // branch is the only place that drains and clears, so a non-empty + // buffer here means we would re-enter NEED_WRAP with identical state + // and spin forever. Fail loudly instead. + throw new AssertionError("Buffer overflow during TLS handshake with non-empty output buffer. This should not happen, please report as a bug"); + } + // in theory, this can happen if the output buffer is too small to fit a single TLS handshake record, + // but that would indicate our starting buffer is too small. + growWrapOutputBuffer(); + break; case OK: // wrapOutputBuffer: write mode int written = 0; @@ -336,7 +372,16 @@ public void startTlsSession(CharSequence peerName) throws TlsSessionInitFailedEx // we need to receive more data from a socket, let's try again break; case BUFFER_OVERFLOW: - throw new AssertionError("Buffer overflow during TLS handshake. This should not happen, please report as a bug"); + if (unwrapOutputBuffer.position() != 0) { + // unwrap() produced plaintext but signalled overflow without consuming + // the next record. Nothing in the handshake loop drains this buffer, + // so re-entering NEED_UNWRAP would spin forever. Fail loudly. + throw new AssertionError("Buffer overflow during TLS handshake with non-empty output buffer. This should not happen, please report as a bug"); + } + // in theory, this can happen if the output buffer is too small to fit a single TLS handshake record, + // but that would indicate our starting buffer is too small. + growUnwrapOutputBuffer(); + break; case OK: // good, let's see what we need to do next break; @@ -352,7 +397,8 @@ public void startTlsSession(CharSequence peerName) throws TlsSessionInitFailedEx unwrapInputBuffer.limit(0); // write mode and empty - unwrapOutputBuffer.clear(); + unwrapOutputBuffer.position(0); + unwrapOutputBuffer.limit(unwrapOutputBuffer.capacity()); wrapOutputBuffer.clear(); state = STATE_TLS; } catch (KeyManagementException | NoSuchAlgorithmException | KeyStoreException | IOException | @@ -478,6 +524,13 @@ private void freeInternalBuffers() { ptrToFree = unwrapInputBufferPtr; unwrapInputBufferPtr = 0; Unsafe.free(ptrToFree, capacity, MemoryTag.NATIVE_TLS_RSS); + + capacity = unwrapOutputBuffer.capacity(); + assert capacity != 0; + resetBufferToPointer(unwrapOutputBuffer, 0, 0); + ptrToFree = unwrapOutputBufferPtr; + unwrapOutputBufferPtr = 0; + Unsafe.free(ptrToFree, capacity, MemoryTag.NATIVE_TLS_RSS); } } @@ -485,11 +538,28 @@ private void growWrapOutputBuffer() { wrapOutputBufferPtr = expandBuffer(wrapOutputBuffer, wrapOutputBufferPtr); } + private void growUnwrapOutputBuffer() { + unwrapOutputBufferPtr = expandBuffer(unwrapOutputBuffer, unwrapOutputBufferPtr); + } + private void prepareInternalBuffers() { int initialCapacity = Integer.getInteger("questdb.experimental.tls.buffersize", INITIAL_BUFFER_CAPACITY_BYTES); this.wrapOutputBufferPtr = allocateMemoryAndResetBuffer(wrapOutputBuffer, initialCapacity); this.unwrapInputBufferPtr = allocateMemoryAndResetBuffer(unwrapInputBuffer, initialCapacity); unwrapInputBuffer.flip(); // read mode + this.unwrapOutputBufferPtr = allocateMemoryAndResetBuffer(unwrapOutputBuffer, initialCapacity); + } + + private int drainUnwrapOutputBuffer(long bufferPtr, int bufferLen) { + unwrapOutputBuffer.flip(); + int oldPosition = unwrapOutputBuffer.position(); + int len = Math.min(bufferLen, unwrapOutputBuffer.remaining()); + if (len > 0) { + Vect.memcpy(bufferPtr, unwrapOutputBufferPtr + oldPosition, len); + } + unwrapOutputBuffer.position(oldPosition + len); + unwrapOutputBuffer.compact(); + return len; } private int readFromSocket() { @@ -544,4 +614,4 @@ private int writeToSocket(int bytesToSend) { LIMIT_FIELD_OFFSET = Unsafe.getUnsafe().objectFieldOffset(limitField); CAPACITY_FIELD_OFFSET = Unsafe.getUnsafe().objectFieldOffset(capacityField); } -} \ No newline at end of file +} diff --git a/core/src/test/java/io/questdb/client/test/network/JavaTlsClientSocketHandshakeOverflowTest.java b/core/src/test/java/io/questdb/client/test/network/JavaTlsClientSocketHandshakeOverflowTest.java new file mode 100644 index 00000000..6d76ab49 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/network/JavaTlsClientSocketHandshakeOverflowTest.java @@ -0,0 +1,342 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.network; + +import io.questdb.client.ClientTlsConfiguration; +import io.questdb.client.network.JavaTlsClientSocket; +import io.questdb.client.network.NetworkFacade; +import io.questdb.client.network.NetworkFacadeImpl; +import org.junit.Assert; +import org.junit.Test; +import org.slf4j.LoggerFactory; + +import javax.net.ssl.KeyManager; +import javax.net.ssl.SSLContextSpi; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLEngineResult; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLServerSocketFactory; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSessionContext; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.TrustManager; +import java.lang.reflect.Constructor; +import java.nio.ByteBuffer; +import java.security.Provider; +import java.security.SecureRandom; +import java.security.Security; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.function.BiFunction; + +public class JavaTlsClientSocketHandshakeOverflowTest { + + private static final String PROVIDER_NAME = "HandshakeOverflowTestProvider"; + + /* + * Demonstrates that startTlsSession() spins forever in the NEED_WRAP branch when + * SSLEngine.wrap() returns BUFFER_OVERFLOW with wrapOutputBuffer.position() > 0 + * and handshakeStatus stays NEED_WRAP. The new code path + * + * case BUFFER_OVERFLOW: + * if (wrapOutputBuffer.position() == 0) { growWrapOutputBuffer(); } + * break; + * + * does not drain or grow when position > 0, so the outer while-loop re-enters + * NEED_WRAP with identical state and never makes progress. The original code + * threw an AssertionError here, which at least failed loudly. + */ + @Test + public void testHandshakeWrapOverflowWithNonEmptyBufferShouldNotLoopForever() throws Exception { + Provider provider = new HandshakeOverflowProvider(); + Security.insertProviderAt(provider, 1); + Thread t = null; + try { + try (JavaTlsClientSocket socket = newSocket()) { + socket.of(0); // -> STATE_PLAINTEXT + + CountDownLatch done = new CountDownLatch(1); + t = new Thread(() -> { + try { + socket.startTlsSession("test.host"); + } catch (Throwable ignored) { + // Expected: a healthy handshake loop should fail loudly here, + // not spin forever. Any exception (AssertionError, SSLException, + // TlsSessionInitFailedException) counts as "did not hang". + } finally { + done.countDown(); + } + }); + t.setDaemon(true); + t.start(); + + boolean completed = done.await(2, TimeUnit.SECONDS); + Assert.assertTrue( + "startTlsSession looped without making progress — handshake BUFFER_OVERFLOW " + + "with wrapOutputBuffer.position() > 0 has no break-out path", + completed + ); + } + } finally { + Security.removeProvider(PROVIDER_NAME); + if (t != null && t.isAlive()) { + t.interrupt(); + } + } + } + + private static JavaTlsClientSocket newSocket() throws Exception { + Constructor ctor = JavaTlsClientSocket.class.getDeclaredConstructor( + NetworkFacade.class, + org.slf4j.Logger.class, + ClientTlsConfiguration.class + ); + ctor.setAccessible(true); + return ctor.newInstance( + new NoOpFacade(), + LoggerFactory.getLogger(JavaTlsClientSocketHandshakeOverflowTest.class), + ClientTlsConfiguration.INSECURE_NO_VALIDATION + ); + } + + public static final class HandshakeOverflowProvider extends Provider { + public HandshakeOverflowProvider() { + super(PROVIDER_NAME, "1.0", "test-only"); + put("SSLContext.TLS", HandshakeOverflowSslContextSpi.class.getName()); + } + } + + public static final class HandshakeOverflowSslContextSpi extends SSLContextSpi { + public HandshakeOverflowSslContextSpi() { + } + + @Override + protected SSLEngine engineCreateSSLEngine() { + return new HandshakeOverflowEngine(); + } + + @Override + protected SSLEngine engineCreateSSLEngine(String host, int port) { + return new HandshakeOverflowEngine(); + } + + @Override + protected SSLSessionContext engineGetClientSessionContext() { + return null; + } + + @Override + protected SSLServerSocketFactory engineGetServerSocketFactory() { + throw new UnsupportedOperationException(); + } + + @Override + protected SSLSessionContext engineGetServerSessionContext() { + return null; + } + + @Override + protected SSLSocketFactory engineGetSocketFactory() { + throw new UnsupportedOperationException(); + } + + @Override + protected void engineInit(KeyManager[] km, TrustManager[] tm, SecureRandom sr) { + } + } + + private static final class HandshakeOverflowEngine extends SSLEngine { + @Override + public void beginHandshake() { + } + + @Override + public void closeInbound() { + } + + @Override + public void closeOutbound() { + } + + @Override + public Runnable getDelegatedTask() { + return null; + } + + @Override + public boolean getEnableSessionCreation() { + return false; + } + + @Override + public String[] getEnabledCipherSuites() { + return new String[0]; + } + + @Override + public String[] getEnabledProtocols() { + return new String[0]; + } + + @Override + public SSLEngineResult.HandshakeStatus getHandshakeStatus() { + return SSLEngineResult.HandshakeStatus.NEED_WRAP; + } + + @Override + public boolean getNeedClientAuth() { + return false; + } + + @Override + public SSLParameters getSSLParameters() { + return new SSLParameters(); + } + + @Override + public SSLSession getSession() { + return null; + } + + @Override + public String[] getSupportedCipherSuites() { + return new String[0]; + } + + @Override + public String[] getSupportedProtocols() { + return new String[0]; + } + + @Override + public boolean getUseClientMode() { + return true; + } + + @Override + public boolean getWantClientAuth() { + return false; + } + + @Override + public boolean isInboundDone() { + return false; + } + + @Override + public boolean isOutboundDone() { + return false; + } + + @Override + public void setEnableSessionCreation(boolean flag) { + } + + @Override + public void setEnabledCipherSuites(String[] suites) { + } + + @Override + public void setEnabledProtocols(String[] protocols) { + } + + @Override + public void setNeedClientAuth(boolean need) { + } + + @Override + public void setSSLParameters(SSLParameters params) { + } + + @Override + public void setUseClientMode(boolean mode) { + } + + @Override + public void setWantClientAuth(boolean want) { + } + + @Override + public SSLEngineResult unwrap(ByteBuffer src, ByteBuffer[] dsts, int offset, int length) { + // Not used — handshake stays in NEED_WRAP. + return new SSLEngineResult( + SSLEngineResult.Status.OK, + SSLEngineResult.HandshakeStatus.NEED_WRAP, + 0, + 0 + ); + } + + @Override + public SSLEngineResult wrap(ByteBuffer[] srcs, int offset, int length, ByteBuffer dst) { + // Write one byte to dst to make wrapOutputBuffer.position() > 0 on the + // very first call. From then on we always return BUFFER_OVERFLOW with + // NEED_WRAP, modelling a (contrived but spec-permitted) engine where + // wrap() advances dst.position but signals overflow. + if (dst.remaining() > 0) { + dst.put((byte) 0x42); + } + return new SSLEngineResult( + SSLEngineResult.Status.BUFFER_OVERFLOW, + SSLEngineResult.HandshakeStatus.NEED_WRAP, + 0, + 1 + ); + } + + // Java 9+ overrides — stubbed so the test compiles on JDK 17. + @Override + public String getApplicationProtocol() { + return null; + } + + @Override + public String getHandshakeApplicationProtocol() { + return null; + } + + @Override + public BiFunction, String> getHandshakeApplicationProtocolSelector() { + return null; + } + + @Override + public void setHandshakeApplicationProtocolSelector(BiFunction, String> selector) { + } + } + + private static final class NoOpFacade extends NetworkFacadeImpl { + @Override + public int recvRaw(int fd, long buffer, int bufferLen) { + return 0; + } + + @Override + public int sendRaw(int fd, long buffer, int bufferLen) { + return bufferLen; // Pretend the bytes were sent so the OK send-loop terminates if reached. + } + } +} diff --git a/core/src/test/java/io/questdb/client/test/network/JavaTlsClientSocketTest.java b/core/src/test/java/io/questdb/client/test/network/JavaTlsClientSocketTest.java new file mode 100644 index 00000000..7b2736e3 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/network/JavaTlsClientSocketTest.java @@ -0,0 +1,515 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.network; + +import io.questdb.client.ClientTlsConfiguration; +import io.questdb.client.network.JavaTlsClientSocket; +import io.questdb.client.network.NetworkFacade; +import io.questdb.client.network.NetworkFacadeImpl; +import io.questdb.client.std.MemoryTag; +import io.questdb.client.std.Unsafe; +import io.questdb.client.test.tools.TestUtils; +import org.junit.Assert; +import org.junit.Test; +import org.slf4j.LoggerFactory; + +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLEngineResult; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSession; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.nio.ByteBuffer; +import java.util.List; +import java.util.function.BiFunction; + +import static org.junit.Assert.assertEquals; + +public class JavaTlsClientSocketTest { + + private static final String TLS_BUFFER_SIZE_PROP = "questdb.experimental.tls.buffersize"; + + @Test + public void testRecvGrowsTlsOutputBufferAndDrainsRemainder() throws Exception { + String previous = System.getProperty(TLS_BUFFER_SIZE_PROP); + try { + System.setProperty(TLS_BUFFER_SIZE_PROP, "8"); + TestUtils.assertMemoryLeak(() -> { + try (JavaTlsClientSocket socket = newSocket()) { + invoke(socket, "prepareInternalBuffers"); + setField(socket, "sslEngine", new OverflowThenPayloadSslEngine("abcdef".getBytes())); + setIntField(socket, "state", 2); + + ByteBuffer unwrapInputBuffer = getField(socket, "unwrapInputBuffer"); + long unwrapInputBufferPtr = getLongField(socket, "unwrapInputBufferPtr"); + for (int i = 0; i < 6; i++) { + Unsafe.getUnsafe().putByte(unwrapInputBufferPtr + i, (byte) ('0' + i)); + } + unwrapInputBuffer.position(0); + unwrapInputBuffer.limit(6); + + long out1 = Unsafe.malloc(4, MemoryTag.NATIVE_DEFAULT); + long out2 = Unsafe.malloc(4, MemoryTag.NATIVE_DEFAULT); + try { + int n1 = socket.recv(out1, 4); + assertEquals(4, n1); + assertBytes("abcd", out1, n1); + + int n2 = socket.recv(out2, 4); + assertEquals(2, n2); + assertBytes("ef", out2, n2); + + ByteBuffer unwrapOutputBuffer = getField(socket, "unwrapOutputBuffer"); + assertEquals(0, unwrapOutputBuffer.position()); + } finally { + Unsafe.free(out2, 4, MemoryTag.NATIVE_DEFAULT); + Unsafe.free(out1, 4, MemoryTag.NATIVE_DEFAULT); + } + } + }); + } finally { + if (previous == null) { + System.clearProperty(TLS_BUFFER_SIZE_PROP); + } else { + System.setProperty(TLS_BUFFER_SIZE_PROP, previous); + } + } + } + + @Test + public void testRecvSpillsAndGrowsInternalBufferForOversizedRecord() throws Exception { + String previous = System.getProperty(TLS_BUFFER_SIZE_PROP); + try { + System.setProperty(TLS_BUFFER_SIZE_PROP, "4"); + TestUtils.assertMemoryLeak(() -> { + try (JavaTlsClientSocket socket = newSocket()) { + invoke(socket, "prepareInternalBuffers"); + setField(socket, "sslEngine", new RealisticOverflowSslEngine("012345".getBytes())); + setIntField(socket, "state", 2); + + ByteBuffer unwrapInputBuffer = getField(socket, "unwrapInputBuffer"); + long unwrapInputBufferPtr = getLongField(socket, "unwrapInputBufferPtr"); + for (int i = 0; i < 4; i++) { + Unsafe.getUnsafe().putByte(unwrapInputBufferPtr + i, (byte) ('a' + i)); + } + unwrapInputBuffer.position(0); + unwrapInputBuffer.limit(4); + + long out1 = Unsafe.malloc(4, MemoryTag.NATIVE_DEFAULT); + long out2 = Unsafe.malloc(4, MemoryTag.NATIVE_DEFAULT); + try { + int n1 = socket.recv(out1, 4); + assertEquals(4, n1); + assertBytes("0123", out1, n1); + + // Caller's 4-byte buffer forced a spill into the internal buffer; the internal + // buffer was 4 bytes too, so the spill path had to grow it (4 -> 8) before the + // SSLEngine could write the 6-byte payload. + ByteBuffer unwrapOutputBuffer = getField(socket, "unwrapOutputBuffer"); + assertEquals(8, unwrapOutputBuffer.capacity()); + + int n2 = socket.recv(out2, 4); + assertEquals(2, n2); + assertBytes("45", out2, n2); + } finally { + Unsafe.free(out2, 4, MemoryTag.NATIVE_DEFAULT); + Unsafe.free(out1, 4, MemoryTag.NATIVE_DEFAULT); + } + } + }); + } finally { + if (previous == null) { + System.clearProperty(TLS_BUFFER_SIZE_PROP); + } else { + System.setProperty(TLS_BUFFER_SIZE_PROP, previous); + } + } + } + + @Test + public void testRecvProcessesBufferedRecordAfterEmptyOkUnwrap() throws Exception { + String previous = System.getProperty(TLS_BUFFER_SIZE_PROP); + try { + System.setProperty(TLS_BUFFER_SIZE_PROP, "32"); + TestUtils.assertMemoryLeak(() -> { + try (JavaTlsClientSocket socket = newSocket()) { + invoke(socket, "prepareInternalBuffers"); + setField(socket, "sslEngine", new TicketThenDataSslEngine("DATA12".getBytes())); + setIntField(socket, "state", 2); + + ByteBuffer unwrapInputBuffer = getField(socket, "unwrapInputBuffer"); + long unwrapInputBufferPtr = getLongField(socket, "unwrapInputBufferPtr"); + // 12 bytes: first 6 simulate a post-handshake control record (e.g. TLS 1.3 NewSessionTicket) + // that consumes input but produces no plaintext, the next 6 bytes are a real data record. + for (int i = 0; i < 12; i++) { + Unsafe.getUnsafe().putByte(unwrapInputBufferPtr + i, (byte) ('a' + i)); + } + unwrapInputBuffer.position(0); + unwrapInputBuffer.limit(12); + + long out = Unsafe.malloc(16, MemoryTag.NATIVE_DEFAULT); + try { + int n = socket.recv(out, 16); + // recv must keep unwrapping after an OK-with-zero-output result and deliver the + // plaintext from the buffered data record. + assertEquals(6, n); + assertBytes("DATA12", out, n); + } finally { + Unsafe.free(out, 16, MemoryTag.NATIVE_DEFAULT); + } + } + }); + } finally { + if (previous == null) { + System.clearProperty(TLS_BUFFER_SIZE_PROP); + } else { + System.setProperty(TLS_BUFFER_SIZE_PROP, previous); + } + } + } + + private static void assertBytes(String expected, long ptr, int len) { + Assert.assertEquals(expected.length(), len); + for (int i = 0; i < len; i++) { + assertEquals((byte) expected.charAt(i), Unsafe.getUnsafe().getByte(ptr + i)); + } + } + + private static void invoke(Object obj, String methodName) throws Exception { + Method method = obj.getClass().getDeclaredMethod(methodName); + method.setAccessible(true); + method.invoke(obj); + } + + private static JavaTlsClientSocket newSocket() throws Exception { + var constructor = JavaTlsClientSocket.class.getDeclaredConstructor( + NetworkFacade.class, + org.slf4j.Logger.class, + ClientTlsConfiguration.class + ); + constructor.setAccessible(true); + return constructor.newInstance( + new NoOpNetworkFacade(), + LoggerFactory.getLogger(JavaTlsClientSocketTest.class), + ClientTlsConfiguration.INSECURE_NO_VALIDATION + ); + } + + @SuppressWarnings("unchecked") + private static T getField(Object obj, String fieldName) throws Exception { + Field field = obj.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + return (T) field.get(obj); + } + + private static long getLongField(Object obj, String fieldName) throws Exception { + Field field = obj.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + return field.getLong(obj); + } + + private static void setField(Object obj, String fieldName, Object value) throws Exception { + Field field = obj.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(obj, value); + } + + private static void setIntField(Object obj, String fieldName, int value) throws Exception { + Field field = obj.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.setInt(obj, value); + } + + private static final class NoOpNetworkFacade extends NetworkFacadeImpl { + @Override + public int recvRaw(int fd, long buffer, int bufferLen) { + return 0; + } + + @Override + public int sendRaw(int fd, long buffer, int bufferLen) { + return 0; + } + } + + private static final class OverflowThenPayloadSslEngine extends StubSslEngine { + private final byte[] payload; + private int unwrapCalls; + + private OverflowThenPayloadSslEngine(byte[] payload) { + this.payload = payload; + } + + @Override + public SSLEngineResult unwrap(ByteBuffer src, ByteBuffer[] dsts, int offset, int length) { + if (length == 0) { + throw new IllegalArgumentException("no destination buffers"); + } + unwrapCalls++; + ByteBuffer dst = dsts[offset]; + if (unwrapCalls == 1) { + return new SSLEngineResult( + SSLEngineResult.Status.BUFFER_OVERFLOW, + SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING, + 0, + 0 + ); + } + if (unwrapCalls > 2) { + throw new IllegalStateException("unexpected extra unwrap call"); + } + if (dst.remaining() < payload.length) { + throw new IllegalStateException("destination should have been grown"); + } + for (byte b : payload) { + dst.put(b); + } + src.position(src.position() + payload.length); + return new SSLEngineResult( + SSLEngineResult.Status.OK, + SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING, + payload.length, + payload.length + ); + } + } + + private static final class RealisticOverflowSslEngine extends StubSslEngine { + private final byte[] payload; + private boolean payloadWritten; + + private RealisticOverflowSslEngine(byte[] payload) { + this.payload = payload; + } + + @Override + public SSLEngineResult unwrap(ByteBuffer src, ByteBuffer[] dsts, int offset, int length) { + ByteBuffer dst = dsts[offset]; + if (dst.remaining() < payload.length) { + return new SSLEngineResult( + SSLEngineResult.Status.BUFFER_OVERFLOW, + SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING, + 0, + 0 + ); + } + if (payloadWritten) { + throw new IllegalStateException("payload already written"); + } + int consumed = src.remaining(); + src.position(src.position() + consumed); + payloadWritten = true; + for (byte b : payload) { + dst.put(b); + } + return new SSLEngineResult( + SSLEngineResult.Status.OK, + SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING, + consumed, + payload.length + ); + } + } + + private static abstract class StubSslEngine extends SSLEngine { + @Override + public void beginHandshake() { + } + + @Override + public void closeInbound() { + } + + @Override + public void closeOutbound() { + } + + @Override + public String getApplicationProtocol() { + return null; + } + + @Override + public Runnable getDelegatedTask() { + return null; + } + + @Override + public String[] getEnabledCipherSuites() { + return new String[0]; + } + + @Override + public String[] getEnabledProtocols() { + return new String[0]; + } + + @Override + public boolean getEnableSessionCreation() { + return false; + } + + @Override + public String getHandshakeApplicationProtocol() { + return null; + } + + @Override + public BiFunction, String> getHandshakeApplicationProtocolSelector() { + return null; + } + + @Override + public SSLEngineResult.HandshakeStatus getHandshakeStatus() { + return SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING; + } + + @Override + public boolean getNeedClientAuth() { + return false; + } + + @Override + public SSLParameters getSSLParameters() { + return new SSLParameters(); + } + + @Override + public SSLSession getSession() { + return null; + } + + @Override + public String[] getSupportedCipherSuites() { + return new String[0]; + } + + @Override + public String[] getSupportedProtocols() { + return new String[0]; + } + + @Override + public boolean getUseClientMode() { + return true; + } + + @Override + public boolean getWantClientAuth() { + return false; + } + + @Override + public boolean isInboundDone() { + return false; + } + + @Override + public boolean isOutboundDone() { + return false; + } + + @Override + public void setEnableSessionCreation(boolean flag) { + } + + @Override + public void setEnabledCipherSuites(String[] suites) { + } + + @Override + public void setEnabledProtocols(String[] protocols) { + } + + @Override + public void setHandshakeApplicationProtocolSelector(BiFunction, String> selector) { + } + + @Override + public void setNeedClientAuth(boolean need) { + } + + @Override + public void setSSLParameters(SSLParameters params) { + } + + @Override + public void setUseClientMode(boolean mode) { + } + + @Override + public void setWantClientAuth(boolean want) { + } + + @Override + public SSLEngineResult wrap(ByteBuffer[] srcs, int offset, int length, ByteBuffer dst) { + return new SSLEngineResult( + SSLEngineResult.Status.OK, + SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING, + 0, + 0 + ); + } + } + + private static final class TicketThenDataSslEngine extends StubSslEngine { + private final byte[] payload; + private int unwrapCalls; + + private TicketThenDataSslEngine(byte[] payload) { + this.payload = payload; + } + + @Override + public SSLEngineResult unwrap(ByteBuffer src, ByteBuffer[] dsts, int offset, int length) { + unwrapCalls++; + if (unwrapCalls == 1) { + // Simulate a post-handshake control record (e.g. NewSessionTicket): consume input, + // produce no plaintext. + src.position(src.position() + 6); + return new SSLEngineResult( + SSLEngineResult.Status.OK, + SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING, + 6, + 0 + ); + } + if (unwrapCalls > 2) { + throw new IllegalStateException("unexpected extra unwrap call"); + } + ByteBuffer dst = dsts[offset]; + for (byte b : payload) { + dst.put(b); + } + src.position(src.position() + payload.length); + return new SSLEngineResult( + SSLEngineResult.Status.OK, + SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING, + payload.length, + payload.length + ); + } + } +}