From 3c12fb6254df3fcf5dced7e7add312083bc486d9 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 6 Aug 2018 05:02:47 -0400 Subject: [PATCH] fix #283 (remove unnecessary log.Fatal) The server should never crash during rehash, even if the config is invalid. --- irc/client.go | 19 +++++++++---------- irc/config.go | 6 +++--- irc/logger/logger.go | 11 +++-------- irc/nickserv.go | 7 ++++++- irc/server.go | 36 ++++++++++++++++++++++++------------ 5 files changed, 45 insertions(+), 34 deletions(-) diff --git a/irc/client.go b/irc/client.go index ad3717f2..d5b6a3f3 100644 --- a/irc/client.go +++ b/irc/client.go @@ -7,7 +7,6 @@ package irc import ( "fmt" - "log" "net" "runtime/debug" "strconv" @@ -82,8 +81,8 @@ type Client struct { vhost string } -// NewClient returns a client with all the appropriate info setup. -func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { +// NewClient sets up a new client and starts its goroutine. +func NewClient(server *Server, conn net.Conn, isTLS bool) { now := time.Now() config := server.Config() fullLineLenLimit := config.Limits.LineLen.Tags + config.Limits.LineLen.Rest @@ -114,15 +113,17 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { } if config.Server.CheckIdent && !utils.AddrIsUnix(conn.RemoteAddr()) { _, serverPortString, err := net.SplitHostPort(conn.LocalAddr().String()) + if err != nil { + server.logger.Error("internal", "bad server address", err.Error()) + return + } serverPort, _ := strconv.Atoi(serverPortString) - if err != nil { - log.Fatal(err) - } clientHost, clientPortString, err := net.SplitHostPort(conn.RemoteAddr().String()) - clientPort, _ := strconv.Atoi(clientPortString) if err != nil { - log.Fatal(err) + server.logger.Error("internal", "bad client address", err.Error()) + return } + clientPort, _ := strconv.Atoi(clientPortString) client.Notice(client.t("*** Looking up your username")) resp, err := ident.Query(clientHost, serverPort, clientPort, IdentTimeoutSeconds) @@ -141,8 +142,6 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { } } go client.run() - - return client } func (client *Client) resetFakelag() { diff --git a/irc/config.go b/irc/config.go index e3680ad1..b27891b6 100644 --- a/irc/config.go +++ b/irc/config.go @@ -410,17 +410,17 @@ func (conf *Config) Operators(oc map[string]*OperClass) (map[string]*Oper, error } // TLSListeners returns a list of TLS listeners and their configs. -func (conf *Config) TLSListeners() map[string]*tls.Config { +func (conf *Config) TLSListeners() (map[string]*tls.Config, error) { tlsListeners := make(map[string]*tls.Config) for s, tlsListenersConf := range conf.Server.TLSListeners { config, err := tlsListenersConf.Config() if err != nil { - log.Fatal(err) + return nil, err } config.ClientAuth = tls.RequestClientCert tlsListeners[s] = config } - return tlsListeners + return tlsListeners, nil } // LoadConfig loads the given YAML configuration file. diff --git a/irc/logger/logger.go b/irc/logger/logger.go index 48cbb097..c6014482 100644 --- a/irc/logger/logger.go +++ b/irc/logger/logger.go @@ -124,7 +124,9 @@ func (logger *Manager) ApplyConfig(config []LoggingConfig) error { stdoutWriteLock: &logger.stdoutWriteLock, fileWriteLock: &logger.fileWriteLock, } - if typeMap["userinput"] || typeMap["useroutput"] || (typeMap["*"] && !(excludedTypeMap["userinput"] && excludedTypeMap["useroutput"])) { + ioEnabled := typeMap["userinput"] || typeMap["useroutput"] || (typeMap["*"] && !(excludedTypeMap["userinput"] && excludedTypeMap["useroutput"])) + // raw I/O is only logged at level debug; + if ioEnabled && logConfig.Level == LogDebug { atomic.StoreUint32(&logger.loggingRawIO, 1) } if sLogger.MethodFile.Enabled { @@ -177,13 +179,6 @@ func (logger *Manager) Error(logType string, messageParts ...string) { logger.Log(LogError, logType, messageParts...) } -// Fatal logs the given message as an error message, then exits. -func (logger *Manager) Fatal(logType string, messageParts ...string) { - logger.Error(logType, messageParts...) - logger.Error("FATAL", "Fatal error encountered, application exiting") - os.Exit(1) -} - type fileMethod struct { Enabled bool Filename string diff --git a/irc/nickserv.go b/irc/nickserv.go index fda9669d..addccd96 100644 --- a/irc/nickserv.go +++ b/irc/nickserv.go @@ -20,6 +20,11 @@ func servCmdRequiresAuthEnabled(server *Server) bool { return server.AccountConfig().AuthenticationEnabled } +func nsGroupEnabled(server *Server) bool { + conf := server.Config() + return conf.Accounts.AuthenticationEnabled && conf.Accounts.NickReservation.Enabled +} + const nickservHelp = `NickServ lets you register and login to an account. To see in-depth help for a specific NickServ command, try: @@ -55,7 +60,7 @@ same user account, letting you reclaim your nickname.`, GROUP links your current nickname with your logged-in account, preventing other users from changing to it (or forcing them to rename).`, helpShort: `$bGROUP$b links your current nickname to your user account.`, - enabled: servCmdRequiresAccreg, + enabled: nsGroupEnabled, authRequired: true, }, diff --git a/irc/server.go b/irc/server.go index c05767ab..cb53f3f8 100644 --- a/irc/server.go +++ b/irc/server.go @@ -10,7 +10,6 @@ import ( "crypto/tls" "encoding/base64" "fmt" - "log" "math/rand" "net" "net/http" @@ -328,8 +327,8 @@ func (server *Server) checkBans(ipaddr net.IP) (banned bool, message string) { // IRC protocol listeners // -// createListener starts the given listeners. -func (server *Server) createListener(addr string, tlsConfig *tls.Config) *ListenerWrapper { +// createListener starts a given listener. +func (server *Server) createListener(addr string, tlsConfig *tls.Config) (*ListenerWrapper, error) { // make listener var listener net.Listener var err error @@ -342,7 +341,7 @@ func (server *Server) createListener(addr string, tlsConfig *tls.Config) *Listen listener, err = net.Listen("tcp", addr) } if err != nil { - log.Fatal(server, "listen error: ", err) + return nil, err } // throw our details to the server so we can be modified/killed later @@ -385,7 +384,7 @@ func (server *Server) createListener(addr string, tlsConfig *tls.Config) *Listen } }() - return &wrapper + return &wrapper, nil } // generateMessageID returns a network-unique message ID. @@ -899,7 +898,7 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { } // we are now open for business - server.setupListeners(config) + err = server.setupListeners(config) if !initial { // push new info to all of our clients @@ -914,7 +913,7 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { } } - return nil + return err } func (server *Server) setupPprofListener(config *Config) { @@ -1024,15 +1023,20 @@ func (server *Server) loadDatastore(config *Config) error { return nil } -func (server *Server) setupListeners(config *Config) { +func (server *Server) setupListeners(config *Config) (err error) { logListener := func(addr string, tlsconfig *tls.Config) { server.logger.Info("listeners", fmt.Sprintf("now listening on %s, tls=%t.", addr, (tlsconfig != nil)), ) } + tlsListeners, err := config.TLSListeners() + if err != nil { + server.logger.Error("rehash", "failed to reload TLS certificates, aborting rehash", err.Error()) + return + } + // update or destroy all existing listeners - tlsListeners := config.TLSListeners() for addr := range server.listeners { currentListener := server.listeners[addr] var stillConfigured bool @@ -1068,7 +1072,13 @@ func (server *Server) setupListeners(config *Config) { if !exists { // make new listener tlsConfig := tlsListeners[newaddr] - server.listeners[newaddr] = server.createListener(newaddr, tlsConfig) + listener, listenerErr := server.createListener(newaddr, tlsConfig) + if listenerErr != nil { + server.logger.Error("rehash", "couldn't listen on", newaddr, listenerErr.Error()) + err = listenerErr + continue + } + server.listeners[newaddr] = listener logListener(newaddr, tlsConfig) } } @@ -1078,8 +1088,8 @@ func (server *Server) setupListeners(config *Config) { } var usesStandardTLSPort bool - for addr := range config.TLSListeners() { - if strings.Contains(addr, "6697") { + for addr := range tlsListeners { + if strings.HasSuffix(addr, ":6697") { usesStandardTLSPort = true break } @@ -1087,6 +1097,8 @@ func (server *Server) setupListeners(config *Config) { if 0 < len(tlsListeners) && !usesStandardTLSPort { server.logger.Warning("startup", "Port 6697 is the standard TLS port for IRC. You should (also) expose port 6697 as a TLS port to ensure clients can connect securely") } + + return } // elistMatcher takes and matches ELIST conditions