Skip to content

feat: add HTTP/3 support and related functionality - #929

Open
niteshpurohit wants to merge 9 commits into
feat/adapters-nghttp2from
feat/adapters-ngtcp2-nghttp3
Open

niteshpurohit wants to merge 9 commits into
feat/adapters-nghttp2from
feat/adapters-ngtcp2-nghttp3

Conversation

@niteshpurohit

Copy link
Copy Markdown
Member
  • Implemented HTTP/3 session management in http3.cpp to handle HTTP/3 connections.
  • Added QUIC session management in quic.cpp to support QUIC protocol operations.
  • Introduced arena memory management in arena_memory.hpp for efficient memory allocation.
  • Updated dependency.cpp to include new dependency operations for QUIC and HTTP/3.
  • Created tests for HTTP/3 functionality in http3.cpp and http3_contract.cpp to ensure correctness.
  • Enhanced error handling and logging for QUIC and HTTP/3 operations.
  • Ensured compatibility with existing QUIC connection ID and event structures.

closes: #61

- Implemented HTTP/3 session management in `http3.cpp` to handle HTTP/3 connections.
- Added QUIC session management in `quic.cpp` to support QUIC protocol operations.
- Introduced arena memory management in `arena_memory.hpp` for efficient memory allocation.
- Updated `dependency.cpp` to include new dependency operations for QUIC and HTTP/3.
- Created tests for HTTP/3 functionality in `http3.cpp` and `http3_contract.cpp` to ensure correctness.
- Enhanced error handling and logging for QUIC and HTTP/3 operations.
- Ensured compatibility with existing QUIC connection ID and event structures.

closes: #61
@niteshpurohit niteshpurohit self-assigned this Sep 18, 2026
@niteshpurohit
niteshpurohit added this pull request to stack #915 September 18, 2026 03:27
Copilot AI lite review requested due to automatic review settings September 18, 2026 03:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical QUIC crypto, validation, and arena-lifetime issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds QUIC and HTTP/3 adapter support using ngtcp2/nghttp3, with arena allocation, dependency integration, tests, and CI updates.

Changes:

  • Added QUIC and HTTP/3 contracts and implementations.
  • Added arena-backed memory management and dependency wiring.
  • Added tests, build configuration, and CI coverage.
File summaries
File Description
tests/adapters/http3.cpp HTTP/3 and QUIC smoke tests
tests/adapters/http3_contract.cpp Contract assertions
src/core/contract/laghu/core/contract.hpp Dependency operation identifiers
src/adapters/quic.cpp QUIC session implementation
src/adapters/private/laghu/adapters/internal/arena_memory.hpp Arena-backed allocator
src/adapters/http3.cpp HTTP/3 session implementation
src/adapters/dependency.cpp Dependency operation mapping
src/adapters/contract/laghu/adapters/quic.hpp QUIC public contract
src/adapters/contract/laghu/adapters/http3.hpp HTTP/3 public contract
CMakeLists.txt Build targets and tests
cmake/LaghuDependencies.cmake Dependency include configuration
.github/workflows/toolchain.yml CI build and test coverage
Review details

Suppressed comments (1)

src/adapters/contract/laghu/adapters/http3.hpp:43

  • Http3Output::bytes is a borrowed view into nghttp3-owned storage, but the contract does not state its lifetime. Callers can reasonably retain it past next_output() or acknowledge_output(), after which a later session operation may invalidate it. Document the same next-operation lifetime guarantee used by Http2Session::next_output().
struct Http3Output final {
  std::int64_t stream_id{-1};
  core::ByteView bytes{};
  bool fin{};
};
  • Files reviewed: 12/12 changed files
  • Comments generated: 10
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/adapters/http3.cpp Outdated
Comment thread src/adapters/quic.cpp
Comment thread src/adapters/quic.cpp
Comment thread src/adapters/quic.cpp Outdated
Comment thread src/adapters/quic.cpp Outdated
Comment thread src/adapters/contract/laghu/adapters/quic.hpp Outdated
Comment thread src/adapters/quic.cpp Outdated
Comment thread src/adapters/quic.cpp
Comment thread src/adapters/quic.cpp Outdated
Comment thread src/adapters/quic.cpp Outdated
@niteshpurohit niteshpurohit changed the title feat(http3): add HTTP/3 support and related functionality feat: add HTTP/3 support and related functionality Sep 18, 2026
- Added new enums for QUIC key direction to improve clarity in key management.
- Refactored QuicConnectionId to use a class structure for better encapsulation of its properties.
- Introduced new crypto callback functions to handle QUIC cryptographic operations.
- Enhanced error handling in QUIC session management to ensure proper resource cleanup.
- Updated tests to validate the behavior of QUIC sessions after arena resets, ensuring stability.
- Improved memory management and allocation checks in QUIC and HTTP/3 session handling.
Copilot AI review requested due to automatic review settings September 18, 2026 03:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate correctness, safety, lifetime, and resource-cleanup issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (9)

src/adapters/contract/laghu/adapters/http3.hpp:22

  • Http3Event forwards header/data views backed by nghttp3 callback buffers, but the copyable event type does not document that those views expire when the sink callback returns. Consumers may otherwise retain a dangling view; document the callback-only lifetime or make events own their payloads.
struct Http3Event final {

src/adapters/contract/laghu/adapters/quic.hpp:54

  • QuicEvent::data is populated from ngtcp2 callback buffers, which are only valid during the sink invocation. Because the event is copyable and the contract does not state that lifetime, consumers can retain a dangling ByteView; document the callback-only lifetime (as the HTTP/2 event contract does) or provide owned storage.
struct QuicEvent final {

src/adapters/http3.cpp:193

  • nghttp3_conn_writev_stream can return zero vectors with fin set for an empty DATA/body completion. Returning a default output here discards the stream ID and FIN, so the caller cannot call nghttp3_conn_add_write_offset(..., 0) and the stream never gets its required zero-byte FIN acknowledged. Preserve stream/fin when count == 0 and return an empty view for that case.
  if (count == 0) return Http3Output{};
  const auto view = *core::ByteView::from(std::span<const std::byte>{
      reinterpret_cast<const std::byte*>(vector.base), vector.len});
  return Http3Output{stream, view, fin != 0};

src/adapters/http3.cpp:178

  • The final uni argument to nghttp3_conn_read_stream2 is hard-coded to false, so the adapter cannot feed control, QPACK encoder, or QPACK decoder streams as unidirectional streams. Those streams are essential to HTTP/3 and will be classified as bidirectional/request streams; derive the flag from the QUIC stream ID (bit 1) or expose it in the contract.
  const auto result = nghttp3_conn_read_stream2(static_cast<nghttp3_conn*>(connection_), stream,
      reinterpret_cast<const std::uint8_t*>(data.data()), data.size(), fin ? 1 : 0, 0);

src/adapters/quic.cpp:552

  • The session configures ngtcp2 with limits.maximum_packet_bytes, but this guard only requires the hard-coded 1200-byte minimum. ngtcp2 requires the destination buffer to be at least max_tx_udp_payload_size; thus a session configured for 1500 bytes accepts a 1200-byte buffer and invokes the native API outside its documented contract. Retain the configured maximum in State and validate against that value.
  if (output.size() < NGTCP2_MAX_UDP_PAYLOAD_SIZE) {
    return std::unexpected{core_error(core::ErrorCode::invalid_range,
                                      "QUIC output is smaller than the minimum packet buffer")};

src/adapters/quic.cpp:450

  • NGTCP2_MAX_UDP_PAYLOAD_SIZE is the protocol's upper bound (65527 bytes), not the minimum allowed packet size. This rejects normal MTU-sized configurations such as the 1500-byte limit used by the new tests, so QuicSession::create fails before a QUIC session can be established. Validate against the QUIC minimum packet size (or otherwise allow the configured maximum) instead.
      limits.maximum_packet_bytes < NGTCP2_MAX_UDP_PAYLOAD_SIZE ||
      crypto.random_fill == nullptr || crypto.start == nullptr ||
      crypto.receive == nullptr || crypto.retry == nullptr || crypto.update == nullptr) {

src/adapters/quic.cpp:480

  • write_packet passes stream_data directly to ngtcp2, but this callback table does not register acked_stream_data_offset. ngtcp2 may retain the submitted bytes for retransmission until they are acknowledged or the stream closes, so a caller that releases or reuses the ByteView after this method returns can cause corrupted retransmissions; the contract currently provides no lifetime rule or acknowledgement signal. Either copy/retain the data in adapter-owned storage or expose and wire an acknowledgement/lifetime contract before accepting borrowed buffers here.
  callbacks.recv_stream_data = stream_data;
  callbacks.stream_open = stream_opened;
  callbacks.stream_reset = stream_reset;
  callbacks.recv_stop_sending = stop_sending_received;
  callbacks.stream_close2 = stream_closed;

src/adapters/quic.cpp:51

  • Stream-limit and flow-control failures are classified as corrupt_data here. open_bidirectional_stream() can return NGTCP2_ERR_STREAM_ID_BLOCKED, and writes can be blocked by NGTCP2_ERR_STREAM_DATA_BLOCKED; mapping these to exhaustion is necessary so callers can wait/retry instead of treating normal backpressure as peer corruption.
[[nodiscard]] core::DependencyStatus status_for(int code) noexcept {
  if (code == NGTCP2_ERR_NOMEM) return core::DependencyStatus::exhaustion;
  if (code == NGTCP2_ERR_INVALID_ARGUMENT) return core::DependencyStatus::invalid_input;
  if (code == NGTCP2_ERR_CRYPTO) return core::DependencyStatus::crypto;
  return core::DependencyStatus::corrupt_data;

src/adapters/quic.cpp:619

  • After an arena reset, this deliberately skips ngtcp2_conn_del, so ngtcp2 never invokes delete_aead/delete_cipher for key contexts that carry caller-provided destroy callbacks. The public API and the test both allow sessions to outlive a reset, which leaks those external crypto resources; add a reset-time cleanup path or prevent reset while a QUIC session is still live.
void QuicSession::release() noexcept {
  if (connection_ != nullptr && arena_ != nullptr && arena_->generation() == generation_) {
    ngtcp2_conn_del(static_cast<ngtcp2_conn*>(connection_));
  }
  • Files reviewed: 12/12 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/adapters/quic.cpp Outdated
- Added ArenaPin class to manage memory pinning in BoundedArena.
- Updated Http3Session and QuicSession constructors to accept ArenaPin for better memory management.
- Modified session creation methods to ensure proper pinning and unpinning of resources.
- Improved error handling for session cleanup during arena resets.
- Enhanced test cases to validate session behavior during arena resets.
Copilot AI review requested due to automatic review settings September 18, 2026 04:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical compile-time, acknowledgement, and arena-lifetime issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

CMakeLists.txt:250

  • These new HTTP/3/QUIC sources are not added to the build-identity input manifest. cmake/LaghuBuildIdentity.cmake:114-119 explicitly hashes the HTTP/2 contract and implementation, but has no corresponding HTTP/3, QUIC, or arena-memory entries, so changing this library can leave the reported build identity and any cache/determinism checks unchanged. Add the new public/private sources to that manifest.
  add_library(laghu_quic STATIC
    src/adapters/quic.cpp

src/adapters/contract/laghu/adapters/http3.hpp:59

  • This constructor stores an ArenaPin, whose destructor dereferences the arena, so the arena must outlive every Http3Session. The public contract does not state that precondition (unlike Http2Session at src/adapters/contract/laghu/adapters/http2.hpp:145); destroying the arena first leaves the session destructor with a dangling pointer. Document this lifetime requirement on the API.
  [[nodiscard]] static core::Result<Http3Session> create(
      Http3Role role, core::WorkerId worker, core::BoundedArena& arena,
      Http3Limits limits, Http3EventSink events = {},
      DependencyLogSink log_sink = {}) noexcept;

src/adapters/contract/laghu/adapters/quic.hpp:165

  • This constructor stores an ArenaPin, whose destructor dereferences the arena, so the arena must outlive every QuicSession. The public contract does not state that precondition (unlike Http2Session at src/adapters/contract/laghu/adapters/http2.hpp:145); destroying the arena first leaves the session destructor with a dangling pointer. Document this lifetime requirement on the API.
  [[nodiscard]] static core::Result<QuicSession> create(
      QuicRole role, const QuicConnectionId& destination,
      const QuicConnectionId& source, core::BoundedArena& arena,
      QuicLimits limits, QuicCryptoCallbacks crypto,
      QuicEventSink events = {}, DependencyLogSink log_sink = {}) noexcept;

src/adapters/contract/laghu/adapters/quic.hpp:174

  • HTTP/3 needs three locally-created unidirectional streams (control, QPACK encoder, and QPACK decoder), but this contract exposes only open_bidirectional_stream(). Because ngtcp2 types are private and Http3Session::bind_streams() accepts IDs that must already exist, an API consumer has no way to establish the HTTP/3 control plane; the smoke test's hard-coded IDs mask this gap. Expose unidirectional stream creation (and cover the QUIC-to-HTTP/3 setup path) before treating the HTTP/3 adapter as usable.
  [[nodiscard]] core::Result<std::int64_t> open_bidirectional_stream() noexcept;

src/adapters/quic.cpp:458

  • NGTCP2_MAX_UDP_PAYLOAD_SIZE is the protocol's upper bound (65527 bytes), not the minimum packet size. Comparing against it rejects normal MTU-sized configurations such as the 1500-byte limit used by every QUIC construction test, so QuicSession::create fails before any native session is created. Validate against NGTCP2_MIN_INITIAL_DGRAM_SIZE instead (or the intended lower bound).
      limits.maximum_packet_bytes < NGTCP2_MAX_UDP_PAYLOAD_SIZE ||
  • Files reviewed: 13/13 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread src/adapters/http3.cpp Outdated
Comment thread src/adapters/quic.cpp
Comment thread src/core/contract/laghu/core/bounded_arena.hpp
- Added support for unidirectional streams in QuicSession.
- Introduced new methods for acknowledging stream data in Http3Session.
- Updated input hash generation to include HTTP/3 and QUIC headers.
- Improved memory management checks in BoundedArena destructor.
- Enhanced tests to validate new functionality in HTTP/3 and QUIC.
Copilot AI review requested due to automatic review settings September 18, 2026 04:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical QUIC/HTTP/3 lifecycle and configuration issues, plus a transmit-timestamp issue, remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/adapters/quic.cpp:576

  • ngtcp2_conn_writev_stream requires ngtcp2_conn_update_pkt_tx_time after packet generation so pacing and expiry_ns() advance. This adapter never updates the connection after a successful write, so repeated writes can use stale pacing/expiry state; update the transmit timestamp after the packet is handed off (or expose an explicit send-completion operation).
  const auto result = ngtcp2_conn_writev_stream(static_cast<ngtcp2_conn*>(connection_), nullptr,
      nullptr, reinterpret_cast<std::uint8_t*>(output.data()), output.size(), &consumed,
      flags, stream_id, stream_data.empty() ? nullptr : &vector,
      stream_data.empty() ? 0U : 1U, now_ns);
  • Files reviewed: 15/15 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread src/adapters/http3.cpp
Comment thread src/adapters/quic.cpp
Comment thread src/adapters/quic.cpp
- Added `packet_transmitted` method to `QuicSession` for managing packet transmission state.
- Introduced `terminal` and `packet_pending_transmit` flags in `State` struct to track session status.
- Updated error handling in `receive_packet` and `write_packet` methods to set terminal state on errors.
- Modified tests to validate new functionality and ensure proper session cleanup on arena reset.
Copilot AI review requested due to automatic review settings September 18, 2026 05:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Fix the reversed maximum_packet_bytes validation in src/adapters/quic.cpp.

Review details

Suppressed comments (1)

src/adapters/quic.cpp:466

  • This comparison rejects every normal QUIC packet buffer (for example the 1500-byte buffers used by the new tests) and accepts values above ngtcp2's maximum instead. maximum_packet_bytes should be bounded above by NGTCP2_MAX_UDP_PAYLOAD_SIZE (and reject zero), otherwise session creation fails for valid MTUs and can pass an invalid setting to ngtcp2.
      limits.maximum_packet_bytes < NGTCP2_MAX_UDP_PAYLOAD_SIZE ||
  • Files reviewed: 15/15 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

- Added validation for maximum packet size in QuicSession creation to ensure it does not exceed the defined limits.
- Updated test cases to check for valid and invalid session creation scenarios, improving robustness against incorrect configurations.
Copilot AI review requested due to automatic review settings September 18, 2026 05:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

QUIC CID routing, terminal expiry handling, path updates, and arena reuse have unresolved issues.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

tests/adapters/http3.cpp:131

  • The QUIC portion of this test suite never exercises a successful packet path: the crypto callbacks either do nothing or explicitly return false, receive_packet is tested only with malformed input, and the write test expects failure. As a result, the newly added key installation, encryption/decryption, stream, expiry, and packet-transmission callbacks can regress without detection. Add a loopback test with deterministic keys/crypto that successfully writes, receives, and acknowledges a packet.

src/adapters/private/laghu/adapters/internal/arena_memory.hpp:23

  • free_list is owned by each session's ArenaMemory, so blocks returned by arena_free during ngtcp2_conn_del remain reachable only from the session state that is about to be discarded. Creating another QUIC/HTTP/3 session in the same generation cannot reuse those blocks and keeps advancing the monotonic arena offset until it exhausts, even though the native allocations were freed. Move the reusable allocation bookkeeping to an arena/session-generation-owned structure, or otherwise reclaim/reuse these blocks across sessions before relying on this allocator for connection churn.
struct ArenaMemory final {
  core::BoundedArena* arena{};
  core::WorkerId worker;
  ArenaAllocation* free_list{};
};

src/adapters/quic.cpp:666

  • ngtcp2_conn_handle_expiry can return terminal expiry errors such as NGTCP2_ERR_IDLE_CLOSE, but this path never marks State::terminal (unlike receive/write). The error is returned while require_valid() remains successful, allowing later operations to call a closed native connection; classify expiry terminal errors and transition the session to terminal before returning.
core::Result<void> QuicSession::handle_expiry(std::uint64_t now_ns) noexcept {
  if (const auto valid = require_valid(); !valid.has_value()) return valid;
  const int result = ngtcp2_conn_handle_expiry(static_cast<ngtcp2_conn*>(connection_), now_ns);
  if (const auto random = require_random(*static_cast<State*>(state_)); !random.has_value()) {
    return random;
  }
  if (result != 0) return std::unexpected{native_error(core::DependencyOperation::quic_expiry,
      result, static_cast<State*>(state_)->log)};

src/adapters/quic.cpp:488

  • The session's ngtcp2_path is permanently built from zeroed sockaddr_in values, and receive_packet provides no way to supply or update the actual local/remote addresses. This prevents ngtcp2 from validating address changes and makes QUIC migration/path ownership impossible even though the contract is intended to preserve that policy; include the packet path (or an explicit path-update operation) in the transport contract.
  state->path = {
      {reinterpret_cast<sockaddr*>(&state->local), sizeof(state->local)},
      {reinterpret_cast<sockaddr*>(&state->remote), sizeof(state->remote)}, nullptr};
  • Files reviewed: 15/15 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/adapters/quic.cpp
- Added a new NativeMemoryPool class to manage memory allocations for adapter sessions.
- Updated Http3Session and QuicSession to utilize NativeMemoryPool instead of BoundedArena directly.
- Refactored memory allocation logic to improve memory management and reuse of freed blocks.
- Enhanced error handling during session creation to ensure proper memory allocation.
- Introduced tests for validating memory reuse and encrypted QUIC packet handling.
Copilot AI review requested due to automatic review settings September 18, 2026 06:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved QUIC and HTTP/3 correctness and error-handling issues remain, including a critical RNG-failure path.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/adapters/contract/laghu/adapters/quic.hpp:188

  • connection_ids is declared optional here, but QuicSession::create rejects any null connection_ids.write at src/adapters/quic.cpp:533, so a caller using the documented defaults can never construct a session. Since the adapter requires this callback for its NEW_CONNECTION_ID path, make the sink a required parameter (and likewise remove the preceding events default to preserve valid C++ default-argument ordering), or implement a defined no-sink policy instead of rejecting the default.
      QuicEventSink events = {}, QuicConnectionIdSink connection_ids = {},

src/adapters/http3.cpp:195

  • nghttp3_conn_read_stream2 returns a status code, not the number of input bytes consumed; successful calls therefore return 0 here even after accepting data. Callers using the Result<std::size_t> contract will treat every successful non-empty packet as unconsumed and may retry or fail to advance their receive buffer. Return data.size() on success (or change the contract to Result<void>).
  return static_cast<std::size_t>(result);

src/adapters/quic.cpp:352

  • The hp_mask callback's destination is the QUIC header-protection mask, which is NGTCP2_HP_MASKLEN bytes (5), while NGTCP2_HP_SAMPLELEN is the 16-byte input sample. Passing a 16-byte view makes every header-protection implementation write past ngtcp2's destination buffer, as the test callback does here, causing memory corruption. Use the mask-length constant for output and retain the sample-length constant for sample_view.
  const auto output = *core::MutableByteView::from(std::span<std::byte>{
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/adapters/quic.cpp
- Updated QuicSession::create to handle random failure during session creation.
- Modified receive_packet in Http3Session to return the size of data received.
- Enhanced address handling in Quic to use memcpy for better safety.
- Added a new test case for constructor random failure rejection.
Copilot AI review requested due to automatic review settings September 18, 2026 07:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Three moderate issues remain unresolved in the HTTP/3 and QUIC implementations.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/adapters/http3.cpp:103

  • maximum_field_section_bytes is only passed to nghttp3 as its receive setting; native_headers() accepts up to 64 fields without checking the aggregate name/value size. A caller can therefore submit a header block larger than the configured bound, allowing unbounded QPACK/native allocation and making this limit ineffective for outbound requests/responses. Track the configured limit and reject an overflow-safe aggregate before constructing the nghttp3_nv array, as the HTTP/2 adapter does for header submissions.
  for (const auto& header : headers) {
    if (header.name.empty()) {
      return std::unexpected{core_error(core::ErrorCode::invalid_input,
                                        "HTTP/3 header name must not be empty")};
    }
    result.values[result.size] = {
                      const_cast<std::uint8_t*>(reinterpret_cast<const std::uint8_t*>(header.name.data())),
                      const_cast<std::uint8_t*>(reinterpret_cast<const std::uint8_t*>(header.value.data())),
                      header.name.size(), header.value.size(), NGHTTP3_NV_FLAG_NONE};
    ++result.size;

src/adapters/quic.cpp:671

  • Once write_packet returns a positive packet, the caller must call packet_transmitted() before generating another packet so ngtcp2's transmit timestamp/state stays synchronized with the packet actually sent. This method does not reject a second write_packet() while packet_pending_transmit is true, so the second packet can overwrite the pending state and the first packet may never be accounted for. Reject writes while a packet is pending (or explicitly support multiple outstanding packets with corresponding transmit bookkeeping).
core::Result<QuicPacketWrite> QuicSession::write_packet(
    core::MutableByteView output, std::int64_t stream_id, core::ByteView stream_data,
    bool fin, std::uint64_t now_ns) noexcept {
  if (const auto valid = require_valid(); !valid.has_value()) return std::unexpected{valid.error()};
  const auto required = static_cast<State*>(state_)->maximum_packet_bytes;
  if (output.size() < required) {
    return std::unexpected{core_error(core::ErrorCode::invalid_range,
                                      "QUIC output is smaller than the configured packet buffer")};
  }
  ngtcp2_ssize consumed{-1};
  const ngtcp2_vec vector{reinterpret_cast<std::uint8_t*>(const_cast<std::byte*>(stream_data.data())),
                           stream_data.size()};
  const std::uint32_t flags = fin ? NGTCP2_WRITE_STREAM_FLAG_FIN : NGTCP2_WRITE_STREAM_FLAG_NONE;
  ngtcp2_path_storage output_path{};
  ngtcp2_path_storage_zero(&output_path);
  const auto result = ngtcp2_conn_writev_stream(static_cast<ngtcp2_conn*>(connection_),
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/adapters/http3.cpp Outdated
- Added maximum field section bytes to State for better control over header size.
- Updated native_headers function to validate header sizes against the maximum limit.
- Enhanced Http3Session creation to check for valid field-section limits.
- Improved error handling for oversized headers in tests to ensure robustness.
Copilot AI review requested due to automatic review settings September 19, 2026 12:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Critical QUIC retry handling and stale arena-generation validation issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity

Open (2)
Resolved since last review (2)

Comment thread src/adapters/quic.cpp
Comment thread src/adapters/quic.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create ngtcp2 and nghttp3 adapter contracts

3 participants