Skip to content

Allow callers to plug a custom dialer into the smart strategy#34

Merged
myleshorton merged 2 commits into
mainfrom
fisk/configurable-stream-dialer
May 16, 2026
Merged

Allow callers to plug a custom dialer into the smart strategy#34
myleshorton merged 2 commits into
mainfrom
fisk/configurable-stream-dialer

Conversation

@myleshorton
Copy link
Copy Markdown
Contributor

The Outline-SDK smart strategy in `WithProxyless` / `NewSmartHTTPTransport` was hardcoding `transport.TCPDialer{}` / `transport.UDPDialer{}` as its base dialers. Both wrap the stdlib `net.Dialer`, so on a host with an active VPN TUN claiming the default route the smart-strategy probes (and the eventual production dial) get captured by the TUN and loop through the caller's own tunnel.

Radiance currently works around this by overwriting the returned `http.Transport`'s `DialContext` with a bypass dial — but that throws away the entire smart strategy. The smart-dialer cost is paid at construction and the strategy is then never actually invoked at request time, so kindling traffic gets neither DPI evasion nor the VPN-loop fix together.

API additions

Two new options on the Kindling instance:

```go
WithStreamDialer(d transport.StreamDialer) Option
WithPacketDialer(d transport.PacketDialer) Option
```

And a new standalone helper that mirrors `NewSmartHTTPTransport`:

```go
NewSmartHTTPTransportWithDialer(
logWriter io.Writer,
stream transport.StreamDialer,
packet transport.PacketDialer,
domains ...string,
) (*http.Transport, error)
```

When the supplied dialer is `nil`, `newSmartDialer` falls back to the original `TCPDialer{}` / `UDPDialer{}`, so existing callers (anything using `WithProxyless(domains...)` or `NewSmartHTTPTransport(logWriter, domains...)` without setting the new options) get exactly today's behavior.

Why now

Surfaced in a Slack thread from Patrick on 2026-05-15: radiance can't actually feed its bypass dialer into the smart strategy. The workaround it uses (override DialContext post-hoc) suppresses the strategy entirely.

After this PR, radiance can wire `bypass.DialContext` into a `transport.StreamDialer` adapter and pass it via `NewSmartHTTPTransportWithDialer`, restoring the smart strategy on top of bypass.

Other strategies

strategy bypass plumbing status before this PR
domainfront `domainfront.WithDialer(...)` already correct
amp `amp.WithDialer(...)` already correct
smart / proxyless hardcoded `transport.TCPDialer{}` fixed here
dnstt private `dohDialer` / `dotDialer` types same problem; needs a separate fix in dnstt itself, out of scope here

Test plan

  • `go build ./...`
  • `go test ./...` (existing tests pass)
  • Follow-up radiance PR rewires `kindling/smart/client.go` to use `NewSmartHTTPTransportWithDialer` with a `bypass.DialContext`-backed `StreamDialer` adapter
  • Verify on a real device with VPN active: smart-strategy probes appear in logs (currently they don't because radiance overrides DialContext) and connection counters on the bypass listener increment

🤖 Generated with Claude Code

The Outline-SDK smart strategy in WithProxyless / NewSmartHTTPTransport
was hardcoding transport.TCPDialer{} / transport.UDPDialer{} as its base
dialers. Both wrap the stdlib net.Dialer, so on a host with an active
VPN TUN claiming the default route the smart-strategy probes (and the
eventual production dial) get captured by the TUN and loop through the
caller's own tunnel. Radiance currently works around this by
overwriting the returned http.Transport's DialContext with a bypass
dial — but that throws away the entire smart strategy.

Expose two new options:

  WithStreamDialer(transport.StreamDialer)
  WithPacketDialer(transport.PacketDialer)

and a new standalone helper:

  NewSmartHTTPTransportWithDialer(logWriter, stream, packet, domains...)

Both default to the original stdlib-backed behavior when the dialer is
nil, so existing callers are unaffected. The motivating consumer
(radiance, via its kindling/smart/client.go) can now hand kindling a
bypass-aware dialer and keep the smart strategy intact instead of
short-circuiting it.

Domainfront and amp already accept caller-supplied dialers through
their own packages; this change closes the same gap for the smart
strategy. dnstt also dials its own DoH/DoT resolver via a private
dialer type and has the same problem — out of scope here, separate
fix in the dnstt repo.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 16, 2026 11:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 support for supplying custom base dialers to the Outline SDK smart/proxyless strategy so callers can bypass host routing, such as VPN TUN routing, while still using smart dialing.

Changes:

  • Adds WithStreamDialer and WithPacketDialer options for Kindling smart/proxyless transports.
  • Updates smart dialer construction to use custom stream/packet dialers with default fallbacks.
  • Adds NewSmartHTTPTransportWithDialer as a standalone helper mirroring NewSmartHTTPTransport.

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

Comment thread kindling.go
…ndent

WithProxyless constructed the smart dialer at option-application time,
which meant WithStreamDialer / WithPacketDialer only took effect when
callers happened to pass them BEFORE WithProxyless. The reverse order
silently fell back to the default stdlib dialer — exactly the bug the
new options were meant to make impossible.

Move the smart-dialer construction into a deferred slice on the
kindling struct that runs after every other option has had a chance
to mutate the struct. WithProxyless now just registers a closure; the
NewKindling option loop runs the closures last. Both option orderings
now produce a smart transport using the supplied dialers.

Added a regression test (TestNewKindling/ProxylessDialerOrderIndependent)
that swaps a package-level newSmartDialerFn to avoid touching the
network and asserts both options end up at the strategy constructor
regardless of the order the caller passes them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@myleshorton myleshorton merged commit 2e2b417 into main May 16, 2026
1 check passed
myleshorton added a commit to getlantern/radiance that referenced this pull request May 16, 2026
* kindling: route smart strategy probes through bypass dialer

The smart strategy was paying for DNS/TLS strategy probes that the
existing wrapTransportWithBypass call then threw away by overwriting
http.Transport.DialContext post-construction. After this change the
strategy actually uses the bypass dialer, so kindling traffic stays
out of the VPN TUN even when the smart path wins the race.

Two pieces, matching getlantern/kindling#34 + #35:

- bypass.StreamDialer() wraps bypass.DialContext as a
  transport.StreamDialer for outline-sdk consumers. halfCloseConn
  adapts net.Conn → transport.StreamConn (the bufferedConn returned
  on the proxy path doesn't expose CloseRead/CloseWrite via embedding).

- kindling/smart/smart_dialer_config.yml is a copy of kindling's
  embedded YAML with `system: {}` removed. The outline-sdk strategy
  rejects a non-*transport.TCPDialer base with "cannot use system
  resolver with base dialer of type %T" (smart/stream_dialer.go:350)
  whenever the system resolver is selected — semantically correct
  since system DNS goes through OS routing tables and would loop back
  through the very TUN we're trying to bypass.

Wiring:

- kindling/smart/client.go uses NewSmartHTTPTransportWithConfig with
  the bypass dialer + system-less YAML, replacing the old
  wrapTransportWithBypass DialContext override.
- kindling/client.go's WithProxyless path adds WithStreamDialer +
  WithSmartDialerConfig for the same reason.

Bumps kindling to v0.0.0-20260516120759-a9712f95df03 for the new
NewSmartHTTPTransportWithConfig + WithSmartDialerConfig surface.

* Address Copilot review: drop cross-repo code-location refs from doc comments

Per AGENTS.md, comments must not reference code locations in other
repos (those rot when upstream shifts and belong in commit messages).

- DialerConfig doc on kindling/smart/client.go drops the
  outline-sdk:line reference; states the constraint as a contract.
- halfCloseConn doc on bypass/streamdialer.go drops the concrete-type
  enumeration; documents the half-close contract instead.
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