@@ -3,6 +3,8 @@ package proxy
33import (
44 "bufio"
55 "bytes"
6+ "context"
7+ "crypto/tls"
68 "fmt"
79 "io"
810 "math/rand"
@@ -479,3 +481,222 @@ func TestAuditLogging(t *testing.T) {
479481 }
480482 }))
481483}
484+
485+ // ── Dial-timeout regression tests ────────────────────────────────────────────
486+ // These tests satisfy the gating requirements raised by maintainer szuecs:
487+ // 1. Show the bug (unbounded dial) — TestDialBackendHTTPSTimesOutOnStalledHandshake
488+ // 2. Prove the fix (timeout fires) — same test + context-cancel variant
489+ // 3. Prove configurability — TestDialBackendTimeoutViaParams
490+
491+ // TestEffectiveDialTimeoutDefault verifies that a zero dialTimeout falls back
492+ // to defaultUpgradeDialTimeout rather than 0 (which would mean no deadline at
493+ // all and recreate the original goroutine-leak bug).
494+ func TestEffectiveDialTimeoutDefault (t * testing.T ) {
495+ p := getUpgradeProxy () // dialTimeout is zero-value
496+ got := p .effectiveDialTimeout ()
497+ if got != defaultUpgradeDialTimeout {
498+ t .Errorf ("effectiveDialTimeout() = %v; want defaultUpgradeDialTimeout (%v)" ,
499+ got , defaultUpgradeDialTimeout )
500+ }
501+ }
502+
503+ // TestEffectiveDialTimeoutConfigured verifies that an explicit non-zero
504+ // dialTimeout is returned as-is, proving the operator-supplied value wins.
505+ func TestEffectiveDialTimeoutConfigured (t * testing.T ) {
506+ const want = 5 * time .Second
507+ u , _ := url .ParseRequestURI ("http://127.0.0.1:8080/foo" )
508+ p := & upgradeProxy {
509+ backendAddr : u ,
510+ dialTimeout : want ,
511+ }
512+ if got := p .effectiveDialTimeout (); got != want {
513+ t .Errorf ("effectiveDialTimeout() = %v; want %v" , got , want )
514+ }
515+ }
516+
517+ // TestDialBackendHTTPSTimesOutOnStalledHandshake is the primary regression
518+ // test for the P0 availability fix.
519+ //
520+ // It proves that dialBackend for the HTTPS path:
521+ // - does NOT hang indefinitely when the backend accepts TCP but never
522+ // completes the TLS handshake (the exact bug pattern from the original PR),
523+ // - returns an error within [dialTimeout, 3×dialTimeout], and
524+ // - does not leak the goroutine or the file descriptor.
525+ //
526+ // tls.Dialer.DialContext applies its deadline to both TCP connect and the full
527+ // TLS handshake, so a server that accepts TCP but sends no ServerHello will
528+ // cause the dial to abort at exactly the configured deadline.
529+ func TestDialBackendHTTPSTimesOutOnStalledHandshake (t * testing.T ) {
530+ // Start a TCP listener that accepts connections but then stalls —
531+ // never sending TLS ServerHello — simulating a dead backend at TLS layer.
532+ ln , err := net .Listen ("tcp" , "127.0.0.1:0" )
533+ require .NoError (t , err )
534+ defer ln .Close ()
535+
536+ accepted := make (chan struct {})
537+ go func () {
538+ conn , err := ln .Accept ()
539+ if err != nil {
540+ return
541+ }
542+ close (accepted )
543+ defer conn .Close ()
544+ // Stall: hold the TCP connection open but never write TLS data.
545+ time .Sleep (30 * time .Second )
546+ }()
547+
548+ const dialTimeout = 250 * time .Millisecond
549+
550+ backendURL , _ := url .Parse ("https://" + ln .Addr ().String ())
551+ p := & upgradeProxy {
552+ backendAddr : backendURL ,
553+ /* #nosec G402 – test-only, no production TLS */
554+ tlsClientConfig : & tls.Config {InsecureSkipVerify : true },
555+ insecure : true ,
556+ dialTimeout : dialTimeout ,
557+ }
558+
559+ req , err := http .NewRequestWithContext (
560+ context .Background (),
561+ http .MethodGet ,
562+ "https://" + ln .Addr ().String ()+ "/ws" ,
563+ nil ,
564+ )
565+ require .NoError (t , err )
566+ req .URL = backendURL
567+
568+ start := time .Now ()
569+ conn , err := p .dialBackend (req )
570+ elapsed := time .Since (start )
571+
572+ // Backend must have accepted the TCP connection — confirms the stall
573+ // happened at the TLS layer, not at TCP connect.
574+ select {
575+ case <- accepted :
576+ case <- time .After (2 * time .Second ):
577+ t .Fatal ("backend never accepted TCP connection — test precondition failed" )
578+ }
579+
580+ require .Error (t , err , "dialBackend to a stalled TLS backend must return an error" )
581+ assert .Nil (t , conn , "conn must be nil on dial timeout" )
582+
583+ // Tight bounds: elapsed must be in [dialTimeout, 3×dialTimeout].
584+ // Lower bound proves the timeout wasn't zero. Upper bound proves the
585+ // dial didn't fall through to the 30 s default (which would fail CI).
586+ assert .GreaterOrEqual (t , elapsed , dialTimeout ,
587+ "dial returned before dialTimeout (%v); got %v — timeout may be wired to zero" , dialTimeout , elapsed )
588+ assert .Less (t , elapsed , 3 * dialTimeout ,
589+ "dial took %v, expected < 3×dialTimeout (%v) — default 30 s may have been used instead" , elapsed , 3 * dialTimeout )
590+ }
591+
592+ // TestDialBackendRespectsContextCancellation proves that cancelling the request
593+ // context aborts an in-flight dial immediately, independently of dialTimeout.
594+ // This validates the second bound: req.Context() is correctly threaded through
595+ // DialContext so client disconnects / upstream cancellations take effect.
596+ func TestDialBackendRespectsContextCancellation (t * testing.T ) {
597+ ln , err := net .Listen ("tcp" , "127.0.0.1:0" )
598+ require .NoError (t , err )
599+ defer ln .Close ()
600+
601+ go func () {
602+ conn , err := ln .Accept ()
603+ if err != nil {
604+ return
605+ }
606+ defer conn .Close ()
607+ time .Sleep (30 * time .Second ) // stall TLS handshake
608+ }()
609+
610+ backendURL , _ := url .Parse ("https://" + ln .Addr ().String ())
611+ p := & upgradeProxy {
612+ backendAddr : backendURL ,
613+ /* #nosec G402 – test-only */
614+ tlsClientConfig : & tls.Config {InsecureSkipVerify : true },
615+ insecure : true ,
616+ dialTimeout : 10 * time .Second , // long hard ceiling — context fires first
617+ }
618+
619+ const ctxTimeout = 150 * time .Millisecond
620+ ctx , cancel := context .WithTimeout (context .Background (), ctxTimeout )
621+ defer cancel ()
622+
623+ req , err := http .NewRequestWithContext (ctx , http .MethodGet ,
624+ "https://" + ln .Addr ().String ()+ "/ws" , nil )
625+ require .NoError (t , err )
626+ req .URL = backendURL
627+
628+ start := time .Now ()
629+ _ , err = p .dialBackend (req )
630+ elapsed := time .Since (start )
631+
632+ require .Error (t , err , "dialBackend must fail when context is cancelled" )
633+ assert .Less (t , elapsed , 3 * ctxTimeout ,
634+ "dial must abort close to context deadline (%v), not wait for dialTimeout (10 s); took %v" ,
635+ ctxTimeout , elapsed )
636+ }
637+
638+ // TestDialBackendTimeoutViaParams is an end-to-end integration smoke test that
639+ // verifies Params.Timeout flows the full chain:
640+ //
641+ // Params.Timeout → WithParams → Proxy.upgradeDialTimeout
642+ // → makeUpgradeRequest → upgradeProxy.dialTimeout → effectiveDialTimeout()
643+ //
644+ // It uses a stalled-TLS-handshake backend and a sub-second Params.Timeout to
645+ // confirm the entire wiring is functional and the 503 response arrives within
646+ // the configured window — not after the 30 s default.
647+ func TestDialBackendTimeoutViaParams (t * testing.T ) {
648+ ln , err := net .Listen ("tcp" , "127.0.0.1:0" )
649+ require .NoError (t , err )
650+ defer ln .Close ()
651+
652+ go func () {
653+ conn , err := ln .Accept ()
654+ if err != nil {
655+ return
656+ }
657+ defer conn .Close ()
658+ time .Sleep (30 * time .Second )
659+ }()
660+
661+ const customTimeout = 300 * time .Millisecond
662+ backendAddr := "https://" + ln .Addr ().String ()
663+ routes := fmt .Sprintf (`route: Path("/ws") -> "%s";` , backendAddr )
664+
665+ tp , err := newTestProxyWithParams (routes , Params {
666+ ExperimentalUpgrade : true ,
667+ Timeout : customTimeout ,
668+ })
669+ require .NoError (t , err )
670+ defer tp .close ()
671+
672+ skipper := httptest .NewServer (tp .proxy )
673+ defer skipper .Close ()
674+
675+ skipperURL , _ := url .Parse (skipper .URL )
676+ clientConn , err := net .Dial ("tcp" , skipperURL .Host )
677+ require .NoError (t , err )
678+ defer clientConn .Close ()
679+
680+ u , _ := url .ParseRequestURI ("wss://www.example.org/ws" )
681+ r := & http.Request {
682+ URL : u ,
683+ Method : http .MethodGet ,
684+ Header : http.Header {
685+ "Connection" : []string {"Upgrade" },
686+ "Upgrade" : []string {"websocket" },
687+ },
688+ }
689+ require .NoError (t , r .Write (clientConn ))
690+
691+ start := time .Now ()
692+ reader := bufio .NewReader (clientConn )
693+ resp , err := http .ReadResponse (reader , r )
694+ elapsed := time .Since (start )
695+ require .NoError (t , err )
696+
697+ assert .Equal (t , http .StatusServiceUnavailable , resp .StatusCode ,
698+ "stalled backend must yield 503 ServiceUnavailable" )
699+ assert .Less (t , elapsed , 3 * customTimeout ,
700+ "proxy must fail within 3×Params.Timeout (%v); took %v — wiring may be broken" ,
701+ customTimeout , elapsed )
702+ }
0 commit comments