Skip to content

Commit 3db598b

Browse files
refactor(proxy): complete tls-client migration with fixes and tests
The migration to tls-client (commit c6ee11d) introduced direct fhttp dependencies. These changes complete the migration: Bug fixes: - Replace naive prefix-matching in dumbResponseWriter with state-aware CONNECT header handling. The old code swallowed any "HTTP/1.0 200" prefix, including in response bodies. New approach tracks WriteHeader state to correctly swallow only the CONNECT OK header. - Remove TOCTOU-prone RWMutex double-check pattern in TransportCache. The old RLock/RUnlock then Lock double-check had a race window between lock upgrade calls. New features: - Graceful HTTPS connection draining on shutdown using sync.WaitGroup. Active connections are tracked and waited on (with 10s timeout). TCP listener is closed immediately on shutdown signal. - Add client remote address to non-SNI warning for better diagnostics. Code quality: - Simplify fingerprintRoundTripperWrapper to pass ctx.Req directly instead of creating a request copy via WithContext. - Add comprehensive unit tests for net/http <-> fhttp conversion layer. - Rewrite dumbResponseWriter tests with proper subtests (fragmented headers, 502 error passthrough, post-CONNECT data passthrough). - Improve mockConn test helper with proper write buffer tracking. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
1 parent b3a7e8c commit 3db598b

2 files changed

Lines changed: 283 additions & 79 deletions

File tree

main.go

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -86,29 +86,14 @@ func cacheKey(profileName, proxyURL string) string {
8686
func (tc *TransportCache) GetOrCreate(profileName, proxyURL string) (http.RoundTripper, error) {
8787
key := cacheKey(profileName, proxyURL)
8888

89-
// Check if already cached and valid (fast path with read lock)
90-
tc.mu.RLock()
91-
if entry, ok := tc.transports[key]; ok {
92-
if time.Since(entry.lastUsed) < tc.ttl {
93-
entry.lastUsed = time.Now()
94-
tc.mu.RUnlock()
95-
return entry.transport, nil
96-
}
97-
// Entry expired, will be evicted (but we need write lock for deletion)
98-
}
99-
tc.mu.RUnlock()
100-
101-
// Slow path: create new transport with write lock
10289
tc.mu.Lock()
10390
defer tc.mu.Unlock()
10491

105-
// Double-check after acquiring write lock
10692
if entry, ok := tc.transports[key]; ok {
10793
if time.Since(entry.lastUsed) < tc.ttl {
10894
entry.lastUsed = time.Now()
10995
return entry.transport, nil
11096
}
111-
// Expired, remove it
11297
delete(tc.transports, key)
11398
}
11499

@@ -243,9 +228,7 @@ type fingerprintRoundTripperWrapper struct {
243228
}
244229

245230
func (w *fingerprintRoundTripperWrapper) RoundTrip(req *http.Request, ctx *goproxy.ProxyCtx) (*http.Response, error) {
246-
// Create a new request with context to support cancellation
247-
newReq := req.WithContext(ctx.Req.Context())
248-
return w.rt.RoundTrip(newReq)
231+
return w.rt.RoundTrip(ctx.Req)
249232
}
250233

251234
// tlsClientRoundTripper wraps tls_client.HttpClient to implement http.RoundTripper.
@@ -349,6 +332,7 @@ type fingerprintProxy struct {
349332
insecureSkipVerify bool
350333
httpAddr string
351334
httpsAddr string
335+
connWg sync.WaitGroup
352336
}
353337

354338
// NewFingerprintProxy creates a new fingerprint proxy with configurable options.
@@ -471,13 +455,31 @@ func (fp *fingerprintProxy) Run(httpAddr, httpsAddr string) error {
471455
continue
472456
}
473457
}
474-
go fp.handleHTTPS(c)
458+
fp.connWg.Add(1)
459+
go func() {
460+
defer fp.connWg.Done()
461+
fp.handleHTTPS(c)
462+
}()
475463
}
476464
}()
477465

478466
<-ctx.Done()
479467
log.Printf("[Server] Shutdown signal received, stopping servers...")
480468

469+
_ = ln.Close()
470+
471+
done := make(chan struct{})
472+
go func() {
473+
fp.connWg.Wait()
474+
close(done)
475+
}()
476+
select {
477+
case <-done:
478+
log.Printf("[Server] All HTTPS connections drained")
479+
case <-time.After(10 * time.Second):
480+
log.Printf("[Server] Timed out waiting for HTTPS connections to drain")
481+
}
482+
481483
shutdownCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
482484
defer cancel()
483485

@@ -504,7 +506,7 @@ func (fp *fingerprintProxy) handleHTTPS(c net.Conn) {
504506
}
505507

506508
if tlsConn.Host() == "" {
507-
log.Printf("[Warning] Cannot support non-SNI enabled clients")
509+
log.Printf("[Warning] Cannot support non-SNI enabled clients from %s", c.RemoteAddr())
508510
return
509511
}
510512

@@ -525,7 +527,11 @@ func (fp *fingerprintProxy) handleHTTPS(c net.Conn) {
525527

526528
type dumbResponseWriter struct {
527529
net.Conn
528-
header http.Header
530+
header http.Header
531+
connectBuf bytes.Buffer
532+
headerSent bool
533+
connectComplete bool
534+
statusCode int
529535
}
530536

531537
func (dumb *dumbResponseWriter) Header() http.Header {
@@ -536,14 +542,23 @@ func (dumb *dumbResponseWriter) Header() http.Header {
536542
}
537543

538544
func (dumb *dumbResponseWriter) Write(buf []byte) (int, error) {
539-
if bytes.HasPrefix(buf, []byte("HTTP/1.0 200")) || bytes.HasPrefix(buf, []byte("HTTP/1.1 200")) {
540-
return len(buf), nil
545+
if dumb.connectComplete {
546+
return dumb.Conn.Write(buf)
547+
}
548+
if !dumb.headerSent || dumb.statusCode != http.StatusOK {
549+
return dumb.Conn.Write(buf)
550+
}
551+
dumb.connectBuf.Write(buf)
552+
if bytes.Index(dumb.connectBuf.Bytes(), []byte("\r\n\r\n")) >= 0 {
553+
dumb.connectComplete = true
554+
dumb.connectBuf.Reset()
541555
}
542-
return dumb.Conn.Write(buf)
556+
return len(buf), nil
543557
}
544558

545559
func (dumb *dumbResponseWriter) WriteHeader(code int) {
546-
// Silently accept any status code — goproxy handles the protocol write.
560+
dumb.headerSent = true
561+
dumb.statusCode = code
547562
}
548563

549564
func (dumb *dumbResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) {

0 commit comments

Comments
 (0)