commit ea7b9c02231cc03f1f0a879528310a93681b80ff Author: David Fifield <david@bamsoftware.com> Date: Mon Mar 12 14:21:11 2018 -0700 Wait briefly after calling ListenAndServe{TLS} to see if it errors. This is a port of commit e3f3054f8b74caa639a6d9be09702693af9a70e7 from meek. In the previous commit, we changed from separate Listen and Serve steps to always calling ListenAndServe. However, we would really like to immediately get feedback if any errors happen in the Listen step inside the call, because it's much better for debugging if those errors get reported to tor through SMETHOD-ERROR--rather than reporting success to tor and actually logging an error only in the snowflake log. So we wait 100 ms for an error to occur before deciding that the Listen succeeded. We don't need to apply this hack to the ACME HTTP-01 listener, because it's a plaintext listener. Unlike in the TLS case, there isn't any internal magic that the net library does that we have to rely on. We just call net.ListenTCP and check for an error. --- server/server.go | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/server/server.go b/server/server.go index 8fd703a..0136fc0 100644 --- a/server/server.go +++ b/server/server.go @@ -30,6 +30,10 @@ const requestTimeout = 10 * time.Second const maxMessageSize = 64 * 1024 +// How long to wait for ListenAndServe or ListenAndServeTLS to return an error +// before deciding that it's not going to return. +const listenAndServeErrorTimeout = 100 * time.Millisecond + var ptInfo pt.ServerInfo // When a connection handler starts, +1 is written to this channel; when it @@ -174,7 +178,7 @@ func webSocketHandler(ws *websocket.WebSocket) { func initServer(addr *net.TCPAddr, getCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error), - listenAndServe func(*http.Server)) (*http.Server, error) { + listenAndServe func(*http.Server, chan<- error)) (*http.Server, error) { // We're not capable of listening on port 0 (i.e., an ephemeral port // unknown in advance). The reason is that while the net/http package // exposes ListenAndServe and ListenAndServeTLS, those functions never @@ -206,28 +210,44 @@ func initServer(addr *net.TCPAddr, } server.TLSConfig.GetCertificate = getCertificate - go listenAndServe(server) + // Another unfortunate effect of the inseparable net/http ListenAndServe + // is that we can't check for Listen errors like "permission denied" and + // "address already in use" without potentially entering the infinite + // loop of Serve. The hack we apply here is to wait a short time, + // listenAndServeErrorTimeout, to see if an error is returned (because + // it's better if the error message goes to the tor log through + // SMETHOD-ERROR than if it only goes to the snowflake log). + errChan := make(chan error) + go listenAndServe(server, errChan) + select { + case err = <-errChan: + break + case <-time.After(listenAndServeErrorTimeout): + break + } - return server, nil + return server, err } func startServer(addr *net.TCPAddr) (*http.Server, error) { - return initServer(addr, nil, func(server *http.Server) { + return initServer(addr, nil, func(server *http.Server, errChan chan<- error) { log.Printf("listening with plain HTTP on %s", addr) err := server.ListenAndServe() if err != nil { log.Printf("error in ListenAndServe: %s", err) } + errChan <- err }) } func startServerTLS(addr *net.TCPAddr, getCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error)) (*http.Server, error) { - return initServer(addr, getCertificate, func(server *http.Server) { + return initServer(addr, getCertificate, func(server *http.Server, errChan chan<- error) { log.Printf("listening with HTTPS on %s", addr) err := server.ListenAndServeTLS("", "") if err != nil { log.Printf("error in ListenAndServeTLS: %s", err) } + errChan <- err }) }