Skip to content

net: replace hand-rolled transport with HttpClient#98

Merged
codymullins merged 1 commit into
mainfrom
feat/httpclient-transition
Jul 7, 2026
Merged

net: replace hand-rolled transport with HttpClient#98
codymullins merged 1 commit into
mainfrom
feat/httpclient-transition

Conversation

@codymullins

Copy link
Copy Markdown
Collaborator

What

Replaces the ~7,300-line hand-rolled Starling.Net stack (UDP DNS, raw Socket dialer, BouncyCastle TLS, HTTP/1.1 parser, HTTP/2 + HPACK, connection pool, body decoders) with System.Net.Http.HttpClient over a configured SocketsHttpHandler.

This reverses the founding "no HttpClient, no SslStream" decision (approved by the user) and accepts a native-TLS dependency under the BCL. StarlingHttpClient stays the public facade, so no consumer changed — fetch, XHR, iframe loads, and the engine fetchers all still call SendAsync(HttpRequest) -> Result<HttpResponse, NetworkError>.

Starling.Net production code drops from ~7,315 → ~1,793 lines; BouncyCastle.Cryptography is gone.

Browser policy stays above the transport

  • AllowAutoRedirect=false — the engine's redirect loop (Engine.FetchHtmlAsync) still owns response.url / per-hop control.
  • UseCookies=false — our CookieJar (PSL, __Host-/__Secure-, Secure) drives cookies, not CookieContainer.
  • AutomaticDecompression=All replaces the hand-rolled body decoders.
  • SecureConnector (new) does the TLS handshake itself in ConnectCallback (returns an authenticated SslStream, allowed since .NET 7): validates the chain against the bundled CCADB root store via a rewritten managed CertificateVerifier (X509Chain + CustomRootTrust, OS store not consulted), captures the verified leaf per origin for the shell lock UI, and holds the dns/tcp/tls telemetry spans.
  • Response headers copied via the .NonValidated view so duplicate Set-Cookie and raw values survive for getSetCookie() / getAllResponseHeaders().

Tradeoffs

  • Pro: deletes ~5.5k lines of security-sensitive protocol code; battle-tested H1/H2/pooling/decompression; HTTP/3 now available; streaming becomes trivial.
  • Con: native TLS re-enters the engine (no longer strictly pure-managed); per-phase telemetry is now ConnectCallback-derived rather than a true span tree; lost custom TLS/cipher pinning, CRLSet hook, and the custom UDP DNS resolver.

Tests

  • Net 55/55, Bindings 457/457, Engine 181/181 green. Full solution builds; interop grep stays clean (no P/Invoke in our code).
  • Live TLS smoke passes on macOS — example.com (H1) + cloudflare.com (H2 via ALPN), with cert capture. The wp:M3-06e macOS TLS 1.3 crash does not reproduce on the current SDK.
  • Retired the internal-protocol tests (H1/H2/HPACK, DNS, TCP, pool). Added CertificateVerifierTests (managed trust/host/untrusted-root) and a duplicate-Set-Cookie fidelity test.

Known caveat (not a regression)

EngineLiveHttpsTests fails its SSIM golden for example.com (0.9526 vs 0.99). This is a rendering-fidelity golden, not a fetch failure — body-content asserts pass, and networking can't alter pixels given identical HTML. Re-baselining is a separate rendering decision.

Docs

Updated browser-plan/00_INDEX.md (locked-decision row), browser-plan/03_NETWORKING.md (status banner + fixes), and AGENTS.md (interop-policy carve-out).

Swap the ~7,300-line hand-rolled Starling.Net stack (UDP DNS, raw Socket
dialer, BouncyCastle TLS, HTTP/1.1 parser, HTTP/2 + HPACK, connection pool,
body decoders) for System.Net.Http.HttpClient over a configured
SocketsHttpHandler. This reverses the founding "no HttpClient, no SslStream"
decision, approved by the user, and accepts the native TLS dependency.

StarlingHttpClient stays the public facade, so consumers (fetch, XHR, iframe,
engine fetchers) do not change. Browser policy stays above the transport:

- AllowAutoRedirect=false — the engine follows redirects.
- UseCookies=false — our CookieJar (PSL, prefixes) drives cookies.
- SecureConnector does the TLS handshake in ConnectCallback, validates the
  chain against the bundled CCADB root store via a managed CertificateVerifier
  (X509Chain + CustomRootTrust, no OS store), captures the verified leaf per
  origin for the lock UI, and holds the dns/tcp/tls telemetry spans.
- Response headers are copied via the NonValidated view so duplicate
  Set-Cookie survives for fetch getSetCookie()/getAllResponseHeaders().

Net.Net drops from ~7,315 to ~1,793 production lines. Drops the
BouncyCastle.Cryptography dependency. Retires the internal-protocol tests
(H1/H2/HPACK, DNS, TCP, pool) and adds managed cert-verify and header-fidelity
tests. Docs updated: 00_INDEX locked decision, 03_NETWORKING banner, AGENTS
interop carve-out.
Copilot AI review requested due to automatic review settings July 7, 2026 18:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

As provided in the diff/context, this pull request updates the Test262 conformance “ratchet floor” thresholds used by the Starling.Js.Test262.Tests suite. Note that this does not match the PR title/description (which describe a major Starling.Net transport rewrite), so the PR metadata likely needs correction or the full set of code changes needs to be included.

Changes:

  • Raised the Test262 “built-ins” conformance floor to 95.9%.
  • Raised the Test262 “intl402” conformance floor to 99.5% and updated the rationale comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/Starling.Js.Test262.Tests/Test262Tests.cs
@codymullins
codymullins force-pushed the feat/httpclient-transition branch from 89f7eec to 15df7ab Compare July 7, 2026 18:21
@codymullins
codymullins merged commit f50f998 into main Jul 7, 2026
3 of 5 checks passed
@codymullins
codymullins deleted the feat/httpclient-transition branch July 7, 2026 18:24
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 7, 2026
@codymullins
codymullins restored the feat/httpclient-transition branch July 7, 2026 18:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants