From bb24346e049677d64039d3dfddf7f17222752673 Mon Sep 17 00:00:00 2001 From: Anis Eleuch Date: Mon, 12 Jun 2023 17:09:28 +0100 Subject: [PATCH] listen: Only error out if not able to bind any interface (#17353) --- cmd/server-main.go | 9 ++- internal/http/listener.go | 68 ++++++--------------- internal/http/listener_test.go | 106 ++++++++++++++++++--------------- internal/http/server.go | 32 +++++++--- 4 files changed, 108 insertions(+), 107 deletions(-) diff --git a/cmd/server-main.go b/cmd/server-main.go index 270c5c65f..27a714bbc 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -612,7 +612,14 @@ func serverMain(ctx *cli.Context) { httpServer.TCPOptions.Trace = bootstrapTrace go func() { - globalHTTPServerErrorCh <- httpServer.Start(GlobalContext) + serveFn, err := httpServer.Init(GlobalContext, func(listenAddr string, err error) { + logger.LogIf(GlobalContext, fmt.Errorf("Unable to listen on `%s`: %v", listenAddr, err)) + }) + if err != nil { + globalHTTPServerErrorCh <- err + return + } + globalHTTPServerErrorCh <- serveFn() }() bootstrapTrace("setHTTPServer") diff --git a/internal/http/listener.go b/internal/http/listener.go index 49bd6a316..c5e8d5343 100644 --- a/internal/http/listener.go +++ b/internal/http/listener.go @@ -21,7 +21,6 @@ import ( "context" "fmt" "net" - "strings" "syscall" ) @@ -129,61 +128,36 @@ type TCPOptions struct { // httpListener is capable to // * listen to multiple addresses // * controls incoming connections only doing HTTP protocol -func newHTTPListener(ctx context.Context, serverAddrs []string, opts TCPOptions) (listener *httpListener, err error) { - var tcpListeners []*net.TCPListener - - // Close all opened listeners on error - defer func() { - if err == nil { - return - } - - for _, tcpListener := range tcpListeners { - // Ignore error on close. - tcpListener.Close() - } - }() - - isLocalhost := false - for _, serverAddr := range serverAddrs { - host, _, err := net.SplitHostPort(serverAddr) - if err == nil { - if strings.EqualFold(host, "localhost") { - isLocalhost = true - } - } - } +func newHTTPListener(ctx context.Context, serverAddrs []string, opts TCPOptions) (listener *httpListener, listenErrs []error) { + tcpListeners := make([]*net.TCPListener, 0, len(serverAddrs)) + listenErrs = make([]error, len(serverAddrs)) // Unix listener with special TCP options. listenCfg := net.ListenConfig{ Control: setTCPParametersFn(opts), } - // Silently ignore failure to bind on DNS cached ipv6 loopback if user specifies "localhost" - for _, serverAddr := range serverAddrs { - var l net.Listener - if l, err = listenCfg.Listen(ctx, "tcp", serverAddr); err != nil { + for i, serverAddr := range serverAddrs { + var ( + l net.Listener + e error + ) + if l, e = listenCfg.Listen(ctx, "tcp", serverAddr); e != nil { if opts.Trace != nil { - opts.Trace(fmt.Sprint("listenCfg.Listen: ", err.Error())) + opts.Trace(fmt.Sprint("listenCfg.Listen: ", e.Error())) } - if isLocalhost && strings.HasPrefix(serverAddr, "[::1") { - continue - } - return nil, err + listenErrs[i] = e + continue } tcpListener, ok := l.(*net.TCPListener) if !ok { - err = fmt.Errorf("unexpected listener type found %v, expected net.TCPListener", l) + listenErrs[i] = fmt.Errorf("unexpected listener type found %v, expected net.TCPListener", l) if opts.Trace != nil { - opts.Trace(fmt.Sprint("net.TCPListener: ", err.Error())) + opts.Trace(fmt.Sprint("net.TCPListener: ", listenErrs[i].Error())) } - - if isLocalhost && strings.HasPrefix(serverAddr, "[::1") { - continue - } - return nil, err + continue } if opts.Trace != nil { opts.Trace(fmt.Sprint("adding listener to ", tcpListener.Addr())) @@ -191,15 +165,9 @@ func newHTTPListener(ctx context.Context, serverAddrs []string, opts TCPOptions) tcpListeners = append(tcpListeners, tcpListener) } - // Fail if no listeners found if len(tcpListeners) == 0 { - // Report specific issue - if err != nil { - return nil, err - } - // Report general issue - err = fmt.Errorf("%v listeners found, expected at least 1", 0) - return nil, err + // No listeners initialized, no need to continue + return } listener = &httpListener{ @@ -212,5 +180,5 @@ func newHTTPListener(ctx context.Context, serverAddrs []string, opts TCPOptions) } listener.start() - return listener, nil + return } diff --git a/internal/http/listener_test.go b/internal/http/listener_test.go index 979273692..1c0f55a58 100644 --- a/internal/http/listener_test.go +++ b/internal/http/listener_test.go @@ -136,34 +136,37 @@ func TestNewHTTPListener(t *testing.T) { tcpKeepAliveTimeout time.Duration readTimeout time.Duration writeTimeout time.Duration - expectedErr bool + expectedListenErrs []bool }{ - {[]string{"93.184.216.34:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), true}, - {[]string{"example.org:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), true}, - {[]string{"unknown-host"}, time.Duration(0), time.Duration(0), time.Duration(0), true}, - {[]string{"unknown-host:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), true}, - {[]string{"localhost:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), false}, - {[]string{"localhost:65432", "93.184.216.34:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), true}, - {[]string{"localhost:65432", "unknown-host:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), true}, - {[]string{"localhost:0"}, time.Duration(0), time.Duration(0), time.Duration(0), false}, - {[]string{"localhost:0"}, time.Duration(0), time.Duration(0), time.Duration(0), false}, - {[]string{"[::1]:9090", "localhost:0"}, time.Duration(0), time.Duration(0), time.Duration(0), false}, + {[]string{"93.184.216.34:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{true}}, // 1 + {[]string{"example.org:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{true}}, // 2 + {[]string{"unknown-host"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{true}}, // 3 + {[]string{"unknown-host:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{true}}, // 4 + {[]string{"localhost:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{false}}, // 5 + {[]string{"localhost:65432", "93.184.216.34:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{false, true}}, // 6 + {[]string{"localhost:65432", "unknown-host:65432"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{false, true}}, // 7 + {[]string{"[::1:65432", "unknown-host:-1"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{true, true}}, // 7 + {[]string{"localhost:0"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{false}}, // 8 + {[]string{"localhost:0"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{false}}, // 9 + {[]string{"[::1]:9090", "127.0.0.1:90900"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{false}}, // 10 + {[]string{"[::1]:9090", "localhost:0"}, time.Duration(0), time.Duration(0), time.Duration(0), []bool{false}}, // 10 } - for _, testCase := range testCases { - listener, err := newHTTPListener(context.Background(), + for testIdx, testCase := range testCases { + listener, listenErrs := newHTTPListener(context.Background(), testCase.serverAddrs, TCPOptions{}, ) - if !testCase.expectedErr { - if err != nil { - t.Fatalf("error: expected = , got = %v", err) + for i, expectedListenErr := range testCase.expectedListenErrs { + if !expectedListenErr { + if listenErrs[i] != nil { + t.Fatalf("Test %d:, listenErrs[%d] error: expected = , got = %v", testIdx+1, i, listenErrs[i]) + } + } else if listenErrs[i] == nil { + t.Fatalf("Test %d: listenErrs[%d]: expected = %v, got = ", testIdx+1, i, expectedListenErr) } - } else if err == nil { - t.Fatalf("error: expected = %v, got = ", testCase.expectedErr) } - - if err == nil { + if listener != nil { listener.Close() } } @@ -187,20 +190,23 @@ func TestHTTPListenerStartClose(t *testing.T) { {[]string{"127.0.0.1:0", nonLoopBackIP + ":0"}}, } +nextTest: for i, testCase := range testCases { - listener, err := newHTTPListener(context.Background(), + listener, errs := newHTTPListener(context.Background(), testCase.serverAddrs, TCPOptions{}, ) - if err != nil { - if strings.Contains(err.Error(), "The requested address is not valid in its context") { - // Ignore if IP is unbindable. - continue + for _, err := range errs { + if err != nil { + if strings.Contains(err.Error(), "The requested address is not valid in its context") { + // Ignore if IP is unbindable. + continue nextTest + } + if strings.Contains(err.Error(), "bind: address already in use") { + continue nextTest + } + t.Fatalf("Test %d: error: expected = , got = %v", i+1, err) } - if strings.Contains(err.Error(), "bind: address already in use") { - continue - } - t.Fatalf("Test %d: error: expected = , got = %v", i+1, err) } for _, serverAddr := range listener.Addrs() { @@ -238,20 +244,23 @@ func TestHTTPListenerAddr(t *testing.T) { {[]string{"127.0.0.1:" + casePorts[5], nonLoopBackIP + ":" + casePorts[5]}, "0.0.0.0:" + casePorts[5]}, } +nextTest: for i, testCase := range testCases { - listener, err := newHTTPListener(context.Background(), + listener, errs := newHTTPListener(context.Background(), testCase.serverAddrs, TCPOptions{}, ) - if err != nil { - if strings.Contains(err.Error(), "The requested address is not valid in its context") { - // Ignore if IP is unbindable. - continue + for _, err := range errs { + if err != nil { + if strings.Contains(err.Error(), "The requested address is not valid in its context") { + // Ignore if IP is unbindable. + continue nextTest + } + if strings.Contains(err.Error(), "bind: address already in use") { + continue nextTest + } + t.Fatalf("Test %d: error: expected = , got = %v", i+1, err) } - if strings.Contains(err.Error(), "bind: address already in use") { - continue - } - t.Fatalf("Test %d: error: expected = , got = %v", i+1, err) } addr := listener.Addr() @@ -286,20 +295,23 @@ func TestHTTPListenerAddrs(t *testing.T) { {[]string{"127.0.0.1:" + casePorts[5], nonLoopBackIP + ":" + casePorts[5]}, set.CreateStringSet("127.0.0.1:"+casePorts[5], nonLoopBackIP+":"+casePorts[5])}, } +nextTest: for i, testCase := range testCases { - listener, err := newHTTPListener(context.Background(), + listener, errs := newHTTPListener(context.Background(), testCase.serverAddrs, TCPOptions{}, ) - if err != nil { - if strings.Contains(err.Error(), "The requested address is not valid in its context") { - // Ignore if IP is unbindable. - continue + for _, err := range errs { + if err != nil { + if strings.Contains(err.Error(), "The requested address is not valid in its context") { + // Ignore if IP is unbindable. + continue nextTest + } + if strings.Contains(err.Error(), "bind: address already in use") { + continue nextTest + } + t.Fatalf("Test %d: error: expected = , got = %v", i+1, err) } - if strings.Contains(err.Error(), "bind: address already in use") { - continue - } - t.Fatalf("Test %d: error: expected = , got = %v", i+1, err) } addrs := listener.Addrs() diff --git a/internal/http/server.go b/internal/http/server.go index 5549a444a..0c68ccb4b 100644 --- a/internal/http/server.go +++ b/internal/http/server.go @@ -74,8 +74,8 @@ func (srv *Server) GetRequestCount() int { return int(atomic.LoadInt32(&srv.requestCount)) } -// Start - start HTTP server -func (srv *Server) Start(ctx context.Context) (err error) { +// Init - init HTTP server +func (srv *Server) Init(listenCtx context.Context, listenErrCallback func(listenAddr string, err error)) (serve func() error, err error) { // Take a copy of server fields. var tlsConfig *tls.Config if srv.TLSConfig != nil { @@ -85,13 +85,22 @@ func (srv *Server) Start(ctx context.Context) (err error) { // Create new HTTP listener. var listener *httpListener - listener, err = newHTTPListener( - ctx, + listener, listenErrs := newHTTPListener( + listenCtx, srv.Addrs, srv.TCPOptions, ) - if err != nil { - return err + + var interfaceFound bool + for i := range listenErrs { + if listenErrs[i] != nil { + listenErrCallback(srv.Addrs[i], listenErrs[i]) + } else { + interfaceFound = true + } + } + if !interfaceFound { + return nil, errors.New("no available interface found") } // Wrap given handler to do additional @@ -118,11 +127,16 @@ func (srv *Server) Start(ctx context.Context) (err error) { srv.listener = listener srv.listenerMutex.Unlock() - // Start servicing with listener. + var l net.Listener = listener if tlsConfig != nil { - return srv.Server.Serve(tls.NewListener(listener, tlsConfig)) + l = tls.NewListener(listener, tlsConfig) } - return srv.Server.Serve(listener) + + serve = func() error { + return srv.Server.Serve(l) + } + + return } // Shutdown - shuts down HTTP server.