Open
Conversation
…va-questdb-client into jh_experiment_new_ilp
sendPongFrame() used the shared sendBuffer, calling reset() which destroyed any partially-built frame the caller had in progress via getSendBuffer(). This could happen when a PING arrived during receiveFrame()/tryReceiveFrame() while the caller was mid-way through constructing a data frame. Add a dedicated 256-byte controlFrameBuffer for sending pong responses. RFC 6455 limits control frame payloads to 125 bytes plus a 14-byte max header, so 256 bytes is sufficient and never needs to grow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sendCloseFrame() used reason.length() (UTF-16 code units) to calculate the payload size, but wrote reason.getBytes(UTF_8) (UTF-8 bytes) into the buffer. For non-ASCII close reasons, UTF-8 encoding can be longer than the UTF-16 length, causing writes past the declared payload size. This corrupted the frame header length, the masking range, and could overrun the allocated buffer. Compute the UTF-8 byte array upfront and use its length for all sizing calculations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When receiving a CLOSE frame from the server, the client now echoes a close frame back before marking the connection as no longer upgraded. This is required by RFC 6455 Section 5.5.1. The close code parsing was moved out of the handler-null check so the code is always available for the echo. The echo uses the dedicated controlFrameBuffer to avoid clobbering any in-progress frame in the main send buffer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Handle CONTINUATION frames (opcode 0x0) in tryParseFrame() which were previously silently dropped. Fragment payloads are accumulated in a lazily-allocated native memory buffer and delivered as a complete message to the handler when the final FIN=1 frame arrives. The FIN bit is now checked on TEXT/BINARY frames: FIN=0 starts fragment accumulation, FIN=1 delivers immediately. Protocol errors are raised for continuation without an initial fragment and for overlapping fragmented messages. The fragment buffer is freed in close() and the fragmentation state is reset on disconnect(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a configurable maximum size for the WebSocket receive buffer, mirroring the pattern already used by WebSocketSendBuffer. Previously, growRecvBuffer() doubled the buffer without any upper bound, allowing a malicious server to trigger out-of-memory by sending arbitrarily large frames. Add getMaximumResponseBufferSize() to HttpClientConfiguration (defaulting to Integer.MAX_VALUE for backwards compatibility) and enforce the limit in both growRecvBuffer() and appendToFragmentBuffer(), which had the same unbounded growth issue for fragmented messages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests that expect connection failure were hardcoding ports (9000, 19999) which could collide with running services. When a QuestDB server is running on port 9000, the WebSocket connection succeeds and the test fails with "Expected LineSenderException". Replace hardcoded ports with dynamically allocated ephemeral ports via ServerSocket(0). The port is bound and immediately closed, guaranteeing nothing is listening when the test tries to connect. Affected tests: - testBuilderWithWebSocketTransportCreatesCorrectSenderType - testConnectionRefused - testWsConfigString - testWsConfigString_missingAddr_fails - testWsConfigString_protocolAlreadyConfigured_fails Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Sec-WebSocket-Accept header validation used case-sensitive String.contains(), which violates RFC 7230 (HTTP headers are case-insensitive). A server sending the header in a different casing (e.g., sec-websocket-accept) would cause the handshake to fail. Replace with a containsHeaderValue() helper that uses String.regionMatches(ignoreCase=true) for the header name lookup, avoiding both the case-sensitivity bug and unnecessary string allocation from toLowerCase(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace byte-by-byte native-heap copies in writeToSocket and readFromSocket with Unsafe.copyMemory(), using the 5-argument form that bridges native memory and Java byte arrays via Unsafe.BYTE_OFFSET. Add WebSocketChannelTest with a local echo server that verifies data integrity through the copy paths across various payload sizes and patterns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move maxSentSymbolId and sentSchemaHashes updates to after the send/enqueue succeeds in both async and sync flush paths. Previously these were updated before the send, so if sealAndSwapBuffer() threw (async) or sendBinary()/waitForAck() threw (sync), the next batch's delta dictionary would omit symbols the server never received, silently corrupting subsequent data. Also move sentSchemaHashes.add() inside the messageSize > 0 guard in the sync path, where it was incorrectly marking schemas as sent even when no data was produced. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The validate() range check used TYPE_DECIMAL256 (0x15) as the upper bound, which excluded TYPE_CHAR (0x16). CHAR columns would throw IllegalArgumentException on validation. Extend the upper bound to TYPE_CHAR and add tests covering all valid type codes, nullable CHAR, and invalid type rejection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace raw AssertionError with LineSenderException when a token parameter is provided in ws:: or wss:: configuration strings. The else branch in config string parsing was unreachable when the code only supported HTTP and TCP, but became reachable after WebSocket support was added. Users now get a clear "token is not supported for WebSocket protocol" error instead of a cryptic AssertionError. Add test assertions for both ws:: and wss:: schemas with token. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merge QwpWebSocketSenderResetTest and QwpWebSocketSenderFlushCacheTest into a single QwpWebSocketSenderStateTest class. Both tested QwpWebSocketSender internal state management with the same reflection pattern and superclass. The merged class unifies the setField/ setConnected helpers into one setField method and keeps all three test methods in alphabetical order. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WebSocketSendQueue accepted a queueCapacity parameter in its constructor, validated it, and logged it, but never used it. The actual queue is a single volatile slot (pendingBuffer) by design — matching the double-buffering scheme where at most one sealed buffer is pending while the other is being filled. Remove the parameter from the entire chain: WebSocketSendQueue constructor, QwpWebSocketSender field and factory methods, Sender.LineSenderBuilder API, and all tests and benchmark clients that referenced it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename elementSize() to elementSizeInBuffer() in QwpTableBuffer to make explicit that it returns the in-memory buffer stride, not the wire-format encoding size. Update javadoc on both elementSizeInBuffer() and QwpConstants.getFixedTypeSize() to document the distinction: getFixedTypeSize() returns wire sizes (0 for bit-packed BOOLEAN, -1 for variable-width GEOHASH), while elementSizeInBuffer() returns the off-heap buffer stride (1 for BOOLEAN, 8 for GEOHASH). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
encodeColumn() and encodeColumnWithGlobalSymbols() had nearly identical switch statements across 15+ type cases, differing only in the SYMBOL case. Merge them into a single encodeColumn() with a boolean useGlobalSymbols parameter. Similarly merge the duplicate encodeTable() and encodeTableWithGlobalSymbols() into one method. This removes ~100 lines of duplicated code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove WebSocketChannel, ResponseReader, and WebSocketChannelTest. These classes are dead code — the actual sender implementation (QwpWebSocketSender) uses WebSocketClient and WebSocketSendQueue instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
addDecimal128 and addDecimal64 rescale values via a Decimal256 temporary, but only read the lower 128 or 64 bits back. If the rescaled value overflows into the upper bits, the data is silently truncated. Add fitsInStorageSizePow2() checks after rescaling and throw LineSenderException when the result no longer fits in the target storage size. Add tests for both Decimal128 and Decimal64 overflow paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Port ~30 Gorilla encoder tests from the core module to the java-questdb-client module, adapted for the client's off-heap memory API. The new test file includes helper methods for writing timestamps to off-heap memory and decoding delta-of-delta values for round-trip verification without porting the full decoder. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a debug assertion in OffHeapAppendMemory.jumpTo() that validates the offset is non-negative and within the current append position. This is consistent with the assertion pattern used in MemoryPARWImpl.jumpTo() in core. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
decimalColumn(CharSequence, CharSequence) previously allocated a BigDecimal and a new Decimal256 on every call. Replace this with a reusable Decimal256 field and its ofString() method, which parses the CharSequence directly into the mutable object in-place via DecimalParser — no intermediate objects needed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
allocateStorage() incorrectly grouped TYPE_UUID with the array types (TYPE_DOUBLE_ARRAY, TYPE_LONG_ARRAY), causing it to allocate arrayDims and arrayCapture instead of a dataBuffer. Since addUuid() and addNull() for UUID both write to dataBuffer, any use of a UUID column would NPE. Move TYPE_UUID to the TYPE_DECIMAL128 case, which correctly allocates a 256-byte OffHeapAppendMemory for the 16-byte fixed-width data. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GlobalSymbolDictionary.getSymbolsInRange() had no production callers. Remove the method to reduce dead code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test class belongs with the code it tests. Move it from core's test tree into the client module under the matching package io.questdb.client.test.cutlass.qwp.client. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the 18 tests that exercise only client-side classes (GlobalSymbolDictionary, QwpWebSocketEncoder, QwpTableBuffer, QwpBufferWriter) from core into the client submodule. Drop three round-trip tests that depend on server-only classes (QwpStreamingDecoder, QwpMessageCursor, QwpSymbolColumnCursor). Adapt imports from io.questdb.std to io.questdb.client.std and from io.questdb.cutlass.qwp.protocol.QwpConstants to io.questdb.client.cutlass.qwp.protocol.QwpConstants. Remove the unused throws QwpParseException from encodeAndDecode and its helper. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move NativeBufferWriterTest, MicrobatchBufferTest, QwpWebSocketEncoderTest, QwpWebSocketSenderTest, and WebSocketSendQueueTest from core's websocket test directory into the java-questdb-client submodule. These tests only exercise classes in io.questdb.client.* and do not depend on core-module server-side classes. For NativeBufferWriterTest and MicrobatchBufferTest, the client module already had versions with different test methods, so the core methods are merged into the existing files. For the other three, new files are created with package and import paths adjusted to the client module (io.questdb.std -> io.questdb.client.std, etc.). Four integration tests that span both client and core modules remain in the core test tree. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add client-side test coverage for QwpVarint (16 tests), QwpZigZag (10 tests), QwpConstants (14 tests), QwpNullBitmap (13 tests), and QwpSchemaHash (20 tests). Adaptations from core originals: - MemoryTag.NATIVE_DEFAULT → NATIVE_ILP_RSS - QwpParseException → IllegalArgumentException - QwpConstants tests cover newer type codes (0x10–0x16) and FLAG_DELTA_SYMBOL_DICT Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
totalAcked and totalFailed used volatile += (read-modify-write) without synchronization, which is racy when the acker thread updates concurrently with readers. Replace with VarHandle getAndAdd() for atomic updates and getOpaque() for tear-free reads, avoiding the overhead of AtomicLong while keeping the fields in-line with the rest of the lock-free design. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WebSocketSendQueue.close() busy-waited with Thread.sleep(10) outside the processingLock monitor to drain pending batches. This burned CPU unnecessarily and read pendingBuffer without holding the lock, risking a missed-update race. Replace the sleep loop with processingLock.wait(), matching the pattern already used by flush() and enqueue(). The I/O thread already calls processingLock.notifyAll() after polling a batch, so close() now wakes up promptly on notification or at the exact shutdown deadline. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
validateUpgradeResponse() previously only checked the HTTP 101 status line and Sec-WebSocket-Accept header. RFC 6455 Section 4.1 also requires the client to verify that the server response contains "Upgrade: websocket" and "Connection: Upgrade" headers. Add both checks with case-insensitive value matching as the RFC requires. The existing containsHeaderValue() helper gains an ignoreValueCase parameter so the Upgrade and Connection checks use equalsIgnoreCase while the Sec-WebSocket-Accept check retains its exact base64 match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace non-cryptographic Rnd (xorshift seeded with nanoTime/currentTimeMillis) with ChaCha20-based SecureRnd for generating the Sec-WebSocket-Key during the upgrade handshake. WebSocketSendBuffer already uses SecureRnd for frame masking, so this aligns the handshake key generation with the same standard. SecureRnd seeds once from SecureRandom at construction time, then produces unpredictable output with no heap allocations. The handshake key needs only 16 nextInt() calls (one ChaCha20 block) and runs once per connection, so the performance impact is negligible. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Raise javac.target from 11 to 17 in the client module's pom.xml, matching the core module's language level. This enables enhanced switch expressions. Use enhanced switch expressions in getFixedTypeSize() and getTypeName(), matching the core module's style. Remove nine unused constants: CAPABILITY_REQUEST_SIZE, CAPABILITY_RESPONSE_SIZE, DEFAULT_MAX_STRING_LENGTH, HEADER_OFFSET_MAGIC, HEADER_OFFSET_PAYLOAD_LENGTH, HEADER_OFFSET_TABLE_COUNT, HEADER_OFFSET_VERSION, MAX_COLUMN_NAME_LENGTH, MAX_TABLE_NAME_LENGTH. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The client-side parser always operates in client mode (rejects masked frames, accepts unmasked). Remove the serverMode flag and setServerMode()/setStrictMode()/setMaskKey()/getMaskKey() methods that were only needed by the server-side copy. Add WebSocketFrameParserTest with 34 tests covering client-mode frame parsing: opcodes, length encodings, fragmentation, control frames, error cases, and masked-frame rejection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
No description provided.