Skip to content

Fix pre existing issue#18

Merged
mwfj merged 19 commits intomainfrom
fix-pre-existing-issue
Apr 14, 2026
Merged

Fix pre existing issue#18
mwfj merged 19 commits intomainfrom
fix-pre-existing-issue

Conversation

@mwfj
Copy link
Copy Markdown
Owner

@mwfj mwfj commented Apr 13, 2026

fix(h2): close three pre-existing technical debt items

Summary

Closes all three items under Pre-existing Technical Debt :

  1. HTTP/2 :authority vs host default-port normalization
    example.com and example.com:80 (http) / example.com:443 (https) are now treated as equivalent.

  2. 103 Early Hints (RFC 8297) for HTTP/1.1 and HTTP/2
    Async handlers can emit InterimResponseSender(...) before the final response.

  3. HTTP/2 server push (RFC 9113 §8.4)
    Opt-in via http2.enable_push, with a bound ResourcePusher on AsyncHandler for async routes and http::PushResource() for sync routes.

Test count: 372 → 407 (+35)
Full suite is green twice in a row, including stress, race, and timeout suites.


Phase 1 — Item 2: Authority normalization

Task 1: Scheme-aware AuthorityMatch (server/http2_stream.cc)

  • AuthorityMatch now strips the default port (:80 for http, :443 for https) from both sides before comparing.
  • Case-insensitive hostname compare; IPv6 brackets preserved.

New tests (+8):

  • Both directions
  • Default HTTP + default HTTPS
  • Non-default port mismatch
  • Wrong-default mismatch
  • IPv6
  • Case insensitivity

Phase 2 — Scaffolding for Early Hints + Push

Task 2: Extend AsyncHandler signature

Files:

  • include/http/http_callbacks.h

  • include/http/http_router.h

  • server/http_server.cc

  • Added type aliases:

    • InterimResponseSender
    • ResourcePusher
using AsyncHandler = std::function<void(
    const HttpRequest&,
    InterimResponseSender,
    ResourcePusher,
    AsyncCompletionCallback
)>;
  • H1 and H2 sync request paths bind both closures to no-ops at dispatch time.
  • Senders are always bound, never null; handlers never need to check.
  • Phase 3 / Phase 4 fill in each closure without further signature changes.

Phase 3 — Item 3: 103 Early Hints

Task 3: HTTP/1 SendInterimResponse

Files:

  • include/http/http_connection_handler.h

  • server/http_connection_handler.cc

  • server/http_server.cc

  • HttpConnectionHandler::SendInterimResponse(status, headers) serializes HTTP/1.1 interim responses via SendRaw.

HTTP/1.1 {status} {reason}\r\n
Headers\r\n
\r\n
  • Atomic final_response_sent_ flag (acquire/release) makes cross-thread post-final drops race-safe.
  • HTTP/1.0 clients are rejected with a debug log (RFC 8297 requires HTTP/1.1+ for 1xx).
  • Forbidden / hop-by-hop headers are stripped.
  • Oversize responses (> max_header_size) are dropped.

New tests (+6):

  • Basic
  • Multiple-before-final
  • HTTP/1.0 reject
  • Forbidden-header strip
  • Post-final drop
  • 100-continue → 103 → 200 ordering

Task 4: HTTP/2 SubmitInterimHeaders + SendInterimResponse

Files:

  • include/http2/http2_stream.h

  • include/http2/http2_session.h

  • server/http2_session.cc

  • include/http2/http2_connection_handler.h

  • server/http2_connection_handler.cc

  • server/http_server.cc

  • Per-stream final_response_submitted_ flag is set in SubmitResponse on success; SubmitInterimHeaders refuses once final has been submitted.

  • Http2Session::SubmitInterimHeaders uses nghttp2_submit_headers with NGHTTP2_FLAG_NONE (no END_STREAM) so the same stream carries both the 103 and the 200.

  • Http2ConnectionHandler::SendInterimResponse enforces a dispatcher-thread-only contract; off-thread callers get a warn log + drop (no nghttp2 corruption).

  • Added ConnectionHandler::IsOnDispatcherThread() helper.

  • Forbidden + pseudo-headers are stripped.

  • Invalid status codes (< 102, >= 200, 101) are rejected.

New tests (+7):

  • Basic 103 → 200
  • Multiple 103s
  • Dropped-after-final
  • Invalid-status
  • Peer-closed-stream safety
  • 100-continue + 103 + 200 ordering
  • Off-dispatcher-thread reject

Task 5: Docs

Files:

  • docs/http.md

  • docs/http2.md

  • New ## Early Hints (103) sections in both docs, with usage examples, contract table, and RFC references.


Phase 4 — Item 1: HTTP/2 server push

Task 6: Config plumbing — http2.enable_push

Files:

  • include/config/server_config.h

  • server/config_loader.cc

  • include/http2/http2_session.h

  • server/http2_session.cc

  • server/http_server.cc

  • test/http2_test.h

  • Http2Config::enable_push = false (default OFF).

  • JSON parsing + REACTOR_HTTP2_ENABLE_PUSH env override + Serialize().

  • Http2Session::Settings::enable_push; SendServerPreface builds iv[] dynamically:

    • enable_push=false → advertise {ENABLE_PUSH, 0} on the wire
    • enable_push=true → omit the entry (RFC 9113 §7: server MUST NOT write 1; nghttp2's local default of 1 applies internally)
  • Plumbed into h2_settings_ at startup and Reload() (new connections only — RFC forbids SETTINGS_ENABLE_PUSH changes after preface).

New tests (+2):

  • Raw-socket regression tests verifying SETTINGS bytes on the wire in both modes

Task 7: Push primitives on Http2Session

Files:

  • include/http2/http2_session.h

  • server/http2_session.cc

  • PushEnabled() gates on local config and the peer's nghttp2_session_get_remote_settings(SETTINGS_ENABLE_PUSH).

  • CreateServerInitiatedStream(id) skips the incomplete-stream lifecycle so pushed streams are never subject to parse_timeout_sec RST or OldestIncompleteStreamStart tracking.

  • EraseStream(id) provides rollback on submit failure.

  • SubmitPushPromise(parent, method, scheme, authority, path, response) performs atomic PUSH_PROMISE + response submission with validation:

    • GET / HEAD only
    • scheme ∈ {http, https}
    • path starts with /
    • non-empty authority
    • push enabled
    • no GOAWAY
    • parent open
  • Submit failure triggers best-effort RST + EraseStream rollback.

Task 8: PushResource + server binding + sync-handler helper

Files:

  • include/http2/http2_connection_handler.h

  • server/http2_connection_handler.cc

  • include/http/http_server.h

  • server/http_server.cc

  • include/http/push_helper.h (new)

  • Http2ConnectionHandler::PushResource adds dispatcher-thread and shutdown guards, delegates to SubmitPushPromise, increments local_stream_count_, and flushes.

  • SetupH2Handlers now binds a real push_resource closure (replacing the phase-2 no-op).

  • Sync-handler access is provided via HttpServer::current_sync_pusher_ thread-local, installed around router_.Dispatch on the H2 sync path with an RAII guard.

  • Free helper http::PushResource(method, scheme, authority, path, response) reads the thread-local and returns -1 with a debug log when called outside a sync H2 dispatch, including any HTTP/1.x handler.

Task 9: Push integration test matrix

Files:

  • test/http2_test.h

  • Http2TestClient extended with:

    • Response::pushed vector
    • PUSH_PROMISE tracking via OnFrameRecv + OnHeader
    • SetRefusePushes() opt-out for client SETTINGS
    • wait loop drains all promised children

New tests (+13):

  1. Basic — push enabled + peer accepts → 1 promised stream with body
  2. Peer refused via client SETTINGS
  3. Disabled by config
  4. Invalid method POST rejected
  5. Empty path rejected
  6. Empty authority rejected
  7. Invalid scheme ftp rejected
  8. HEAD response body suppressed
  9. Not counted in total_requests
  10. Sync handler via http::PushResource()
  11. http::PushResource() on H1 returns -1
  12. Async handler push (in-dispatcher)
  13. Reload() toggles enable_push for new connections

Task 10: Docs + roadmap + simplify

Files:

  • docs/http2.md

  • server/http_server.cc

  • test/http2_test.h

  • New ## Server Push section in docs/http2.md covering:

    • opt-in flag
    • async / sync usage
    • contract table
    • stream accounting
    • Chromium / Firefox caveat
  • Moved all 3 roadmap items from Pre-existing Technical Debt to Completed with full implementation notes.

  • Simplify pass:

    • Extracted MakeH2ResourcePusher() factory in http_server.cc to remove duplicated closure bodies across async + sync binding sites
    • Extracted Http2TestClient::CollectCompletedResponse() to remove duplicated wait-loop code from SendRequest / SendRequestRaw

Files changed

Area Files
Config include/config/server_config.h, server/config_loader.cc
HTTP/1 layer include/http/http_callbacks.h, include/http/http_router.h, include/http/http_connection_handler.h, server/http_connection_handler.cc
HTTP/2 layer include/http2/http2_stream.h, include/http2/http2_session.h, include/http2/http2_connection_handler.h, server/http2_session.cc, server/http2_stream.cc, server/http2_connection_handler.cc
Transport include/connection_handler.h (new IsOnDispatcherThread() helper)
Server wiring include/http/http_server.h, server/http_server.cc, include/http/push_helper.h
Tests test/http2_test.h, test/http_test.h, test/proxy_test.h, test/route_test.h
Docs docs/http.md, docs/http2.md

Test plan

  • make clean && make -j4 && ./test_runner — 407 tests pass
  • Second run: ./test_runner — 407 tests pass (stability)
  • ./test_runner stress — pass
  • ./test_runner race — pass
  • ./test_runner timeout — pass
  • ./test_runner http2 — 66 tests pass (was 51; +8 authority, +7 Early Hints, +2 SETTINGS-wire, +13 push)
  • ./test_runner http — +6 H1 Early Hints tests pass

New tests exercise

  • Authority match across HTTP/HTTPS default ports + IPv6 + case
  • 103 Early Hints happy path, multiple, post-final drop (H1 + H2)
  • 100-continue + 103 interleave ordering
  • Off-dispatcher-thread guard (H2)
  • Forbidden-header strip (interim + push)
  • SETTINGS_ENABLE_PUSH wire format (both modes)
  • Push validation matrix (method, scheme, path, authority)
  • Push accounting (total_requests, HEAD body suppression)
  • Sync vs async vs H1 push paths
  • Reload() toggles enable_push for new connections

Spec references

  • RFC 8297 — An HTTP Status Code for Indicating Hints (103 Early Hints)
  • RFC 9113 §6.5.2, §7, §8.1, §8.4, §8.6 — HTTP/2 SETTINGS semantics, interim HEADERS, server push, forbidden 101
  • RFC 9110 §9.3.2 — HEAD response body semantics

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3c4a70e78

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for HTTP/2 server push and HTTP 103 Early Hints. It updates the async handler signature to include InterimResponseSender and ResourcePusher callables, adds necessary configuration and environment variable support, and provides documentation and tests for these new features. I have identified a potential thread-safety issue in the HTTP/1.1 implementation of SendInterimResponse where it lacks the dispatcher-thread-only enforcement required by the callback contract.

@mwfj
Copy link
Copy Markdown
Owner Author

mwfj commented Apr 14, 2026

LGTM

@mwfj mwfj merged commit 6220f17 into main Apr 14, 2026
3 of 4 checks passed
@mwfj mwfj deleted the fix-pre-existing-issue branch April 14, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant