Skip to content

flint-fronted: meek polling client + vantage-point scanner (Shir-o-Khorshid CDN-fronting)#10

Merged
myleshorton merged 11 commits into
mainfrom
fisk/fronted-meek
Jun 25, 2026
Merged

flint-fronted: meek polling client + vantage-point scanner (Shir-o-Khorshid CDN-fronting)#10
myleshorton merged 11 commits into
mainfrom
fisk/fronted-meek

Conversation

@myleshorton

@myleshorton myleshorton commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a self-bootstrapping, domain-fronted meek polling transport to flint-fronted — the Shir-o-Khorshid CDN-fronting model (no MITM, no local cert). It's the reusable core the spark fronted-meek transport builds on (spark PR linked below).

This is deliberately distinct from the existing MeekStream/FrontedMeekDialer, which are h2-bidirectional-streaming — a different wire protocol the deployed Lantern meek-server does not speak. Only the solid fronting layer (FrontedTlsDialer, FrontResolver, Config/Front/Masquerade, race_materialized_with) is reused.

What's added

  • meek_pollMeekPollConn (AsyncRead+AsyncWrite) porting lantern-box protocol/meek PR #282: discrete POSTs keyed by X-Session-Id + a monotonic, server-deduped X-Meek-Seq (a lost request/response replays — no gap, no dup), a 256 KB negotiated body (X-Meek-Max-Body), and retriable polls. Both h1 and h2 backends. FrontedMeekPollDialer + open_meek_poll glue; the app and the poll task are bridged by tokio::io::duplex.
  • sys_dnsSystemResolver (getaddrinfo) implementing FrontResolver. The load-bearing local-DNS trick: a censor returns truthful, geo-local Akamai edge IPs because blocking Akamai breaks domestic sites; flint's DoH resolver would bypass exactly those answers.
  • scanner — vantage-point front discovery: Akamai edge local-DNS resolution + CloudFront & Aliyun/Alibaba IP-prefix sampling, with a two-stage ranked probe.
  • socks5 — minimal SOCKS5 CONNECT over the tunnel (the meek-server's upstream is microsocks).
  • dial_fronts — race scanner-discovered fronts without needing a Config.

Testing

  • 37 hermetic lib tests pass, including multi-poll reassembly and retry-replay under dropped responses with no gap/duplication (the #282 integrity property), SOCKS5 framing, and scanner candidate-generation/ranking for all three CDNs.
  • Live (passing), tests/meek_live.rs (run with --features boring --ignored): a real Akamai edge (system-resolved) → meek over h1 → the deployed meek-server → SOCKS5 → example.com → 200.

Follow-up

flint_dial::TlsStream is a blanket-impl trait that hides the negotiated ALPN, so the meek client can't yet auto-select h1/h2 per connection. Surfacing the selected protocol out of the boring stream is a follow-up (tracked in the spark PR); until then the protocol is config-driven.

🤖 Generated with Claude Code

https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr

Summary by CodeRabbit

  • New Features
    • Added public ALPN-aware helpers to dial and race multiple fronted connections, enabling HTTP-version selection.
    • Exposed new fronted meek polling capabilities over fronted TLS, with configurable retrying and payload sizing.
    • Added CDN front scanning to generate, probe concurrently, and return latency-ranked candidates.
    • Introduced system DNS resolution and an async SOCKS5 no-auth CONNECT tunneling module.
  • New Features / API
    • Added ALPN-aware TLS dialing and an AlpnStream wrapper to inspect negotiated protocol.
  • Tests
    • Added/extended Tokio tests for DNS, SOCKS5, scanning, and meek polling; added an ignored live end-to-end compatibility test.

…orshid CDN-fronting)

Add a self-bootstrapping, domain-fronted meek POLLING transport. This is distinct
from the existing h2-streaming MeekStream (a different wire protocol the deployed
meek-server does not speak); only the fronting/TLS/config layer is reused.

- meek_poll: MeekPollConn (AsyncRead+AsyncWrite) porting lantern-box protocol/meek
  PR #282 — discrete POSTs keyed by X-Session-Id + a monotonic server-deduped
  X-Meek-Seq (lost request/response replays, no gap/dup), 256KB negotiated body
  (X-Meek-Max-Body), retriable polls. Both h1 and h2 backends. FrontedMeekPollDialer
  + open_meek_poll glue; app<->poll-task bridged via tokio::io::duplex.
- sys_dns: SystemResolver (getaddrinfo) — local/ISP DNS for Akamai-edge discovery.
  A censor returns truthful, geo-local edge IPs; flint's DoH resolver would bypass
  exactly the local answers we want.
- scanner: vantage-point front discovery — Akamai edge local-DNS resolution +
  CloudFront & Aliyun/Alibaba prefix sampling, two-stage ranked probe.
- socks5: minimal SOCKS5 CONNECT over the tunnel (meek-server upstream is microsocks).
- dial_fronts: race scanner-discovered fronts without a Config.

37 hermetic tests pass (incl. multi-poll reassembly + retry-replay-without-gap/dup).
Live test (tests/meek_live.rs, run with --features boring --ignored): a real Akamai
edge (system-resolved) -> meek over h1 -> the deployed meek-server -> SOCKS5 ->
example.com -> 200.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds public fronted-transport APIs, ALPN-aware dialing, a meek polling client, DNS and CDN scanner utilities, a SOCKS5 CONNECT client, and an ignored live integration test that exercises the end-to-end path.

Changes

Fronted transport stack

Layer / File(s) Summary
Public exports and front dialing
crates/flint-fronted/src/lib.rs, crates/flint-dial/src/lib.rs, crates/flint-dial/src/engine.rs
lib.rs re-exports the new fronted transport, resolver, scanner, and SOCKS5 APIs, and adds dial_fronts and dial_fronts_alpn to race materialized fronts through flint_dial::dial and flint_dial::dial_alpn; flint-dial adds ALPN-aware dialing and AlpnStream.
Front discovery and scan pipeline
crates/flint-fronted/src/sys_dns.rs, crates/flint-fronted/src/scanner.rs
SystemResolver resolves via spawn_blocking; scanner adds Candidate, ScanTargets, Akamai/CloudFront/Aliyun candidate generation, deterministic sampling, scan, and unit tests.
Meek polling core
crates/flint-fronted/src/meek_poll.rs
MeekPollConfig, MeekPollConn, the poll loop, retry/backoff, backend selection, h1/h2 transport, and session-id generation are added, along with fronted meek dialer glue.
SOCKS5 CONNECT client
crates/flint-fronted/src/socks5.rs
Target and connect implement SOCKS5 no-auth CONNECT for IP and domain targets, with a mock server and protocol tests for the handshake and echo path.
Live end-to-end flow
crates/flint-fronted/tests/meek_live.rs
The ignored integration test resolves Akamai candidates, races ALPN-aware front dialing, opens meek polling, tunnels SOCKS5 to example.com:80, sends a GET request, and checks the response.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~90 minutes

Possibly related issues

  • getlantern/engineering#3526 — The new meek polling transport, scanner, SOCKS5 client, and ALPN-aware dialing align with the fronted transport work described there.

Possibly related PRs

  • getlantern/flint#4 — Introduces related fronted dialing infrastructure that the new dial_fronts/dial_fronts_alpn helpers build on.

Poem

A rabbit hopped through fronted rain,
With meek polls pulsing down the lane.
SOCKS5 tunnels, swift and neat,
Led to Example Domain’s bright treat.
🐇✨ I found my packet carrots there.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly captures the main changes: a meek polling client and a vantage-point scanner for CDN fronting.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fisk/fronted-meek

Comment @coderabbitai help to get the list of available commands.

The boring Chrome dial offers h2,http/1.1 and the CDN edge picks one, but boxing
the SslStream into BoxedTlsStream erased which. Capture it and thread it up so the
meek client speaks whatever was negotiated per connection instead of a fixed guess.

- flint-dial: new AlpnStream (wraps BoxedTlsStream + the negotiated ALPN) and
  dial_alpn / dial_over_alpn; dial / dial_over keep their old signatures (thin
  wrappers that drop the ALPN). dial_boring captures
  ssl().selected_alpn_protocol() before boxing.
- flint-fronted: dial_fronts_alpn (race scanner fronts, keep the ALPN) and
  open_meek_poll_auto (set MeekPollConfig.http_version = h2 iff the edge negotiated
  h2, else h1).
- meek_live test: drive the full auto path (scan -> dial_fronts_alpn ->
  open_meek_poll_auto). Live against the deployed server it logs negotiated
  ALPN "http/1.1" and auto-selects h1; SOCKS5 -> example.com -> 200.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 11

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/flint-fronted/src/meek_poll.rs`:
- Around line 239-243: The retry path in roundtrip_with_retry is attempting to
recover by calling Backend::reconnect, but that method currently does nothing,
so failed h1/h2 connections are reused instead of replaced. Update the retry
flow around roundtrip_with_retry and Backend::reconnect to obtain a genuinely
fresh backend after connection failures, or restrict the retry logic to
stream-level resets only; use the reconnect call site and the Backend
implementation to ensure poisoned connections are not retried in place.
- Line 230: The per-poll request in Meek polling can stall indefinitely because
backend.roundtrip waits forever, preventing the retry path from running. Add a
timeout field to MeekPollConfig, then wrap the backend.roundtrip call in
meek_poll’s polling logic with tokio::time::timeout and convert elapsed time
into a retryable TimedOut error so the same seq/body can be retried.
- Around line 405-411: The chunked-transfer parsing in `meek_poll.rs` only
accepts a bare hex size and stops after a single line when `size == 0`, so
update the chunk-size handling to parse only the part before any `;` extension
in the chunk header and then drain trailer lines in a loop until the blank line
terminator is reached. Keep the change localized around the
`usize::from_str_radix` parsing and the zero-size chunk branch in the poll/read
loop so `MeekPoll` correctly handles HTTP/1.1 chunk extensions and multiple
trailers.
- Around line 330-340: The h1 request construction in meek_poll currently
formats cfg.path and cfg.inner_host directly into the request line and headers,
so add validation in this path before building the request. Update the logic
around the head format in meek_poll to reject CR/LF and other control characters
in cfg.path and cfg.inner_host, or switch to an HTTP encoder that safely
serializes these fields before composing the POST request.
- Around line 192-201: The hunk in meek_poll’s polling match arm is failing
rustfmt checks, so update the formatting in the timeout/read handling block to
match standard rustfmt output. Keep the logic in the polling loop and the
app_write_open/read_buf handling unchanged, and use the existing match on
tokio::time::timeout in meek_poll::poll to make the formatter pass.
- Around line 388-390: The response body handling in the meek poll flow is
truncating when content_length exceeds cfg.max_body_bytes, which can corrupt the
stream and leave unread bytes behind; update the logic around the read_exact
path and seq advancement to fail fast with an error before reading or appending
any over-limit body data. Apply the same guard to the other body-read branches
referenced in this area so the tunnel never returns a partial prefix for
oversized responses.
- Around line 114-117: MeekPollConn currently stores a JoinHandle in _task, but
dropping it only detaches the poll task instead of stopping it. Update
MeekPollConn so the background poll task is explicitly aborted when the
connection is dropped, using the existing MeekPollConn and _task ownership to
implement a Drop-based cleanup or equivalent cancellation path that guarantees
the HTTP poll loop cannot outlive the connection.

In `@crates/flint-fronted/src/scanner.rs`:
- Around line 31-35: The file has formatting drift that is causing CI to fail,
so run cargo fmt --all and keep the scanner.rs formatting aligned with rustfmt
in the DEFAULT_AKAMAI_EDGE_HOSTS constant, the scan function signature, the
cloudfront_prefixes and aliyun_prefixes tables, and the test assertions; use the
existing symbols DEFAULT_AKAMAI_EDGE_HOSTS and scan to locate the affected
sections and apply the formatter’s output without changing behavior.

In `@crates/flint-fronted/src/socks5.rs`:
- Around line 160-167: The formatting in the socks5 test is out of sync with
rustfmt, causing the cargo fmt check to fail. Re-run rustfmt on the crate so the
connect call in socks5_connect_ip_then_tunnels is wrapped according to standard
formatting, and commit the resulting formatting-only changes without altering
behavior.
- Around line 35-38: The SOCKS5 handshake in the stream write/read flow can
stall because write_all on a buffered AsyncWrite does not guarantee the peer has
received the greeting or CONNECT request before read_exact waits for a reply.
Update the SOCKS5 logic in the greeting and CONNECT paths in socks5.rs, using
the existing stream write calls around the handshake, to flush the writer after
each write_all and before each corresponding read_exact so the data is actually
sent to the proxy.

In `@crates/flint-fronted/tests/meek_live.rs`:
- Around line 63-69: The SOCKS5 handshake and HTTP request write in meek_live.rs
are unbounded and can hang the test if the tunnel stalls; add explicit timeouts
around the socks5::connect call and the conn.write_all request write. Use the
existing async flow in the test to wrap those awaits with a timeout mechanism,
keeping the fronted dial/read loop behavior unchanged while preventing the live
test from waiting indefinitely.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 56309dd5-ed13-4997-b862-d0f2a334d7bc

📥 Commits

Reviewing files that changed from the base of the PR and between 76c5cd3 and 3751fd5.

📒 Files selected for processing (6)
  • crates/flint-fronted/src/lib.rs
  • crates/flint-fronted/src/meek_poll.rs
  • crates/flint-fronted/src/scanner.rs
  • crates/flint-fronted/src/socks5.rs
  • crates/flint-fronted/src/sys_dns.rs
  • crates/flint-fronted/tests/meek_live.rs

Comment thread crates/flint-fronted/src/meek_poll.rs
Comment thread crates/flint-fronted/src/meek_poll.rs Outdated
Comment thread crates/flint-fronted/src/meek_poll.rs Outdated
Comment thread crates/flint-fronted/src/meek_poll.rs Outdated
Comment thread crates/flint-fronted/src/meek_poll.rs
Comment thread crates/flint-fronted/src/meek_poll.rs Outdated
Comment thread crates/flint-fronted/src/scanner.rs Outdated
Comment thread crates/flint-fronted/src/socks5.rs Outdated
Comment thread crates/flint-fronted/src/socks5.rs
Comment thread crates/flint-fronted/tests/meek_live.rs Outdated

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.

Pull request overview

This PR adds a new “meek-v1 polling” transport stack to flint-fronted (distinct from the existing h2-bidi MeekStream) to support domain-fronted bootstrapping from the user’s vantage point, plus supporting components for discovery (scanner), DNS (system resolver), and upstream tunneling (SOCKS5). It also extends flint-dial to surface negotiated ALPN so the meek client can auto-select HTTP/1.1 vs HTTP/2 per connection.

Changes:

  • Introduces meek_poll (MeekPollConn, FrontedMeekPollDialer, open_meek_poll[_auto]) with retry/replay semantics and both h1/h2 backends.
  • Adds SystemResolver (OS getaddrinfo) + scanner for local-DNS and prefix-sampled front candidate generation and ranked probing.
  • Adds minimal SOCKS5 CONNECT client and a live ignored integration test; adds ALPN-carrying AlpnStream + dial_*_alpn in flint-dial.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/flint-fronted/tests/meek_live.rs Ignored live e2e test covering scan → dial (ALPN) → meek polling → SOCKS5 → example.com.
crates/flint-fronted/src/sys_dns.rs Adds OS stub-resolver (getaddrinfo) FrontResolver implementation for vantage-point DNS.
crates/flint-fronted/src/socks5.rs Adds minimal no-auth SOCKS5 CONNECT client and duplex-based unit tests.
crates/flint-fronted/src/scanner.rs Adds candidate generation (Akamai via system DNS, CloudFront/Aliyun via prefix sampling) + bounded concurrent probe/ranking.
crates/flint-fronted/src/meek_poll.rs Adds meek polling protocol implementation with h1/h2 backends, retry/replay semantics, and extensive unit tests.
crates/flint-fronted/src/lib.rs Exposes new modules/exports; adds dial_fronts and dial_fronts_alpn for scanner-materialized fronts.
crates/flint-dial/src/lib.rs Adds AlpnStream wrapper and exports dial_alpn/dial_over_alpn.
crates/flint-dial/src/engine.rs Captures negotiated ALPN from boring TLS and returns it via AlpnStream.

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

Comment thread crates/flint-fronted/src/meek_poll.rs Outdated
Comment thread crates/flint-fronted/src/meek_poll.rs Outdated
Comment thread crates/flint-fronted/src/meek_poll.rs Outdated
Comment thread crates/flint-fronted/src/sys_dns.rs Outdated
…t, honest retry)

Review fixes from CodeRabbit + Copilot on the fronted-meek PR:
- MeekPollConn aborts its poll task on Drop (was detached, could outlive the conn).
- Per-poll request timeout (MeekPollConfig.request_timeout) wraps each roundtrip;
  an elapsed request is retryable so a stalled response can't block forever.
- Retry semantics made honest: removed the no-op Backend::reconnect; retry only
  in-place when safe (h2: fresh stream + server seq-dedupe), h1 ends the session
  on failure (single keep-alive connection a failed request leaves desynced).
- h1 hardening: reject CR/LF/control chars in inner_host/path (raw request line);
  require Content-Length on keep-alive and error on over-cap instead of leaving
  unread bytes; chunked decode now parses the size token before `;extensions`,
  drains all trailers to the blank line, and keeps draining framing past the cap
  (stops appending only) so the connection stays aligned.
- h2: error on an over-cap response instead of truncating + advancing seq.
- SystemResolver: bound getaddrinfo with a 5s timeout (a stuck resolver can't hang
  the scan).
- meek_live: timeout the SOCKS5 handshake + request write too.
- cargo fmt (unblocks CI); fix a stale MeekHttpVersion doc (ALPN is now exposed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
@myleshorton myleshorton requested a review from Copilot June 25, 2026 20:11
@myleshorton

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread crates/flint-fronted/src/meek_poll.rs
Comment thread crates/flint-fronted/tests/meek_live.rs
…200"

Review (Copilot): "200" could appear anywhere in the bytes. Assert the response
starts with "HTTP/1.1 200" and contains "Example Domain".

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
@myleshorton

Copy link
Copy Markdown
Contributor Author

Round 2 addressed in 8851e04:

  • meek_live assert — now checks the HTTP/1.1 200 status line + Example Domain body marker (not a bare 200 substring).
  • BufReader "doesn't impl AsyncWrite" — false positive: tokio's BufReader passes AsyncWrite through to the inner stream, so it compiles and all 44 tests pass (replied inline).
  • The earlier getaddrinfo/Content-Length/chunked-trailer comments were already fixed in 827981c.

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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Comment thread crates/flint-fronted/tests/meek_live.rs
Comment thread crates/flint-fronted/src/sys_dns.rs Outdated
Comment thread crates/flint-fronted/src/socks5.rs
Comment thread crates/flint-fronted/src/meek_poll.rs Outdated
Comment thread crates/flint-fronted/src/meek_poll.rs
Comment thread crates/flint-fronted/src/scanner.rs
- Reject space/non-ASCII (not just CR/LF/control) in the h1 inner_host/path, which
  would otherwise break the request-target / Host tokenization.
- Validate the chunk-terminating CRLF in chunked decode (error on malformed
  framing instead of silently desyncing the keep-alive connection).
- Validate the SOCKS5 reply RSV byte is 0x00.
- sys_dns test: bind the resolver to a local before awaiting.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
@myleshorton

Copy link
Copy Markdown
Contributor Author

Round 3 addressed in 55552b3:

  • h1 host/path validation — now also rejects space + non-ASCII (not just CR/LF/control).
  • chunked terminator — validate each chunk's trailing CRLF (error on malformed framing).
  • SOCKS5 RSV — validate the reply RSV byte is 0x00.
  • sys_dns test — bind the resolver to a local before awaiting.
  • scan 'static — replied inline (intentional; not on the hot path).

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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread crates/flint-fronted/src/scanner.rs Outdated
Comment thread crates/flint-fronted/src/meek_poll.rs
- read_chunked: reject a chunk whose size exceeds max_body_bytes before
  allocating, so an attacker-controlled chunk-size line can't force an OOM
  allocation (the server caps responses at the advertised max-body anyway).
- Candidate::to_front sets Front.domain to the edge's verify hostname (e.g.
  a248.e.akamai.net) rather than duplicating the inner fronted_host.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
@myleshorton

Copy link
Copy Markdown
Contributor Author

Round 4 addressed in fb51533:

  • chunked OOM (good catch) — reject a chunk whose size exceeds max_body_bytes before allocating (the server caps responses at the advertised max-body).
  • Front.domainCandidate::to_front now sets the edge's verify hostname (e.g. a248.e.akamai.net), not the inner fronted_host.

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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread crates/flint-fronted/src/meek_poll.rs Outdated
Comment thread crates/flint-fronted/src/meek_poll.rs
…hority)

- read_chunked errors when the total exceeds max_body_bytes (matching the
  non-chunked + h2 paths) instead of silently truncating and advancing the seq.
- inner_host is validated as a bare authority — also reject `/ ? # @ \` (not just
  control/space/non-ASCII), which would break the Host header / request target.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
@myleshorton

Copy link
Copy Markdown
Contributor Author

Round 5 addressed in 3d8b4bf:

  • chunked over-cap — now errors when the total exceeds max_body_bytes (matching the non-chunked + h2 paths), no silent truncation + seq advance.
  • inner_host — validated as a bare authority; also rejects / ? # @ \ beyond control/space/non-ASCII.

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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment thread crates/flint-fronted/src/meek_poll.rs
Comment thread crates/flint-fronted/src/scanner.rs
Comment thread crates/flint-fronted/src/scanner.rs
- H2Backend holds the h2 connection-driver JoinHandle and aborts it on Drop, so
  the driver (and the TLS stream it holds) can't outlive the backend — matching
  MeekPollConn's drop semantics.
- ScanTargets gains cloudfront_verify_hostname / aliyun_verify_hostname: prefix-
  sampled (empty-SNI) candidates verify against the edge's cert identity rather
  than always the inner fronted_host (which only matches for an Akamai endpoint).
  Defaults preserve current behavior; documented for real CloudFront/Aliyun meek.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
@myleshorton

Copy link
Copy Markdown
Contributor Author

Round 6 addressed in c57d112:

  • h2 driver leak (good catch) — H2Backend now holds the connection-driver JoinHandle and aborts it on Drop, so it can't outlive the backend/conn.
  • CloudFront/Aliyun verify hostnameScanTargets gains cloudfront_verify_hostname/aliyun_verify_hostname so empty-SNI prefix candidates verify against the edge cert identity, not always the inner host (defaults preserve current behavior; the deployed meek is Akamai-only).

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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread crates/flint-fronted/src/socks5.rs
Comment thread crates/flint-fronted/src/socks5.rs
Flush after the method-select greeting and the CONNECT request; a buffered
AsyncWrite could otherwise hold the bytes while we block on read_exact.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
@myleshorton

Copy link
Copy Markdown
Contributor Author

Round 7 addressed in bddb187: socks5 now flushes after the greeting + CONNECT writes, so a buffered AsyncWrite can't hold the bytes while we block on the reply.

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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment thread crates/flint-fronted/src/sys_dns.rs
Comment thread crates/flint-dial/src/engine.rs
Comment thread crates/flint-fronted/src/meek_poll.rs
- Add a hermetic HTTP/1.1 meek echo test so the h1 backend's Content-Length path
  has CI coverage (previously only the live test exercised h1).
- SystemResolver short-circuits IP literals and sorts+dedups results, matching
  FlintDnsResolver and avoiding redundant scan/dial attempts.

(Skipped the dial() ALPN-Vec micro-alloc nit — negligible, and dial() is not the
meek hot path; the race uses dial_alpn.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
@myleshorton

Copy link
Copy Markdown
Contributor Author

Round 8 addressed in 2cfdc70 — and wrapping the review loop here (comments have converged to nits):

  • h1 hermetic test — added (the h1 Content-Length path now has CI coverage, not just the live test).
  • SystemResolver — short-circuits IP literals + sorts/dedups results (matches FlintDnsResolver).
  • dial() ALPN Vec alloc — skipped intentionally: it's one tiny allocation, and the meek race uses dial_alpn (which needs the ALPN), not dial; not worth special-casing.

Thanks @coderabbitai + Copilot — 8 rounds, all substantive findings addressed. 45 hermetic tests pass + the live end-to-end test against the deployed meek-server.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@myleshorton Thanks for the thorough follow-through — this sounds in a good place to wrap.

The added h1 hermetic coverage, the SystemResolver parity update, and the rationale for skipping the tiny dial() ALPN allocation special-case all make sense. Glad to hear the full 45 hermetic tests plus the live end-to-end test are passing.

Appreciate the careful iteration across all 8 rounds. 🐇


✏️ Learnings added
Learnt from: myleshorton
URL: https://github.com/getlantern/flint/pull/10

Timestamp: 2026-06-25T21:13:37.672Z
Learning: In the Rust `flint-fronted` crate, `crates/flint-fronted/src/sys_dns.rs` `SystemResolver` is expected to match `FlintDnsResolver` behavior by short-circuiting IP literals and returning sorted, deduplicated resolution results.

Learnt from: myleshorton
URL: https://github.com/getlantern/flint/pull/10

Timestamp: 2026-06-25T21:13:37.672Z
Learning: In the Rust `flint-dial` crate, the tiny negotiated-ALPN `Vec` allocation in `dial()` is intentionally accepted and should not be re-raised as a performance nit, because the meek connection race uses `dial_alpn` on the relevant path and the extra allocation is not considered worth special-casing.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

CI runs `cargo clippy --all-targets -- -D warnings`; the manual Default impl
tripped clippy::derivable_impls. Replace with #[derive(Default)] + #[default] on
H2 (unchanged behavior). Ran the exact CI clippy command locally to confirm clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
@myleshorton myleshorton merged commit 95f4cd7 into main Jun 25, 2026
2 checks passed
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.

2 participants