Fix for issue #66: If loading the certificate fails (#67)

* Fix for issue #66: If loading the certificate fails

New behaviour is:

guerrillad will not start if it can't load a certificate

If already started and the config is reloaded with a bad certificate, it will ignore the new config and continue with the old one.

cmd/guerrillad/serve.go

@@ -64,7 +64,10 @@ func sigHandler(app guerrilla.Guerrilla) {
 			newConfig := CmdConfig{}
 			err := readConfig(configPath, pidFile, &newConfig)
 			if err != nil {
+				// new config will not be applied
 				mainlog.WithError(err).Error("Error while ReadConfig (reload)")
+				// re-open logs
+				cmdConfig.EmitLogReopenEvents(app)
 			} else {
 				cmdConfig = newConfig
 				mainlog.Infof("Configuration was reloaded at %s", guerrilla.ConfigLoadTime)
@@ -82,7 +85,7 @@ func sigHandler(app guerrilla.Guerrilla) {
 	}
 }
 
-func subscribeBackendEvent(event string, backend backends.Backend, app guerrilla.Guerrilla) {
+func subscribeBackendEvent(event guerrilla.Event, backend backends.Backend, app guerrilla.Guerrilla) {
 
 	app.Subscribe(event, func(cmdConfig *CmdConfig) {
 		logger, _ := log.GetLogger(cmdConfig.LogFile)
@@ -141,12 +144,12 @@ func serve(cmd *cobra.Command, args []string) {
 	if err != nil {
 		mainlog.WithError(err).Error("Error(s) when starting server(s)")
 	}
-	subscribeBackendEvent("config_change:backend_config", backend, app)
-	subscribeBackendEvent("config_change:backend_name", backend, app)
+	subscribeBackendEvent(guerrilla.EvConfigBackendConfig, backend, app)
+	subscribeBackendEvent(guerrilla.EvConfigBackendName, backend, app)
 	// Write out our PID
 	writePid(cmdConfig.PidFile)
 	// ...and write out our pid whenever the file name changes in the config
-	app.Subscribe("config_change:pid_file", func(ac *guerrilla.AppConfig) {
+	app.Subscribe(guerrilla.EvConfigPidFile, func(ac *guerrilla.AppConfig) {
 		writePid(ac.PidFile)
 	})
 	// change the logger from stdrerr to one from config
@@ -169,21 +172,22 @@ type CmdConfig struct {
 }
 
 func (c *CmdConfig) load(jsonBytes []byte) error {
-	c.AppConfig.Load(jsonBytes)
 	err := json.Unmarshal(jsonBytes, &c)
 	if err != nil {
 		return fmt.Errorf("Could not parse config file: %s", err.Error())
+	} else {
+		// load in guerrilla.AppConfig
+		return c.AppConfig.Load(jsonBytes)
 	}
-	return nil
 }
 
 func (c *CmdConfig) emitChangeEvents(oldConfig *CmdConfig, app guerrilla.Guerrilla) {
 	// has backend changed?
 	if !reflect.DeepEqual((*c).BackendConfig, (*oldConfig).BackendConfig) {
-		app.Publish("config_change:backend_config", c)
+		app.Publish(guerrilla.EvConfigBackendConfig, c)
 	}
 	if c.BackendName != oldConfig.BackendName {
-		app.Publish("config_change:backend_name", c)
+		app.Publish(guerrilla.EvConfigBackendName, c)
 	}
 	// call other emitChangeEvents
 	c.AppConfig.EmitChangeEvents(&oldConfig.AppConfig, app)

cmd/guerrillad/serve_test.go

@@ -3,6 +3,13 @@ package main
 import (
 	"crypto/tls"
 	"encoding/json"
+	"fmt"
+	"github.com/flashmob/go-guerrilla"
+	"github.com/flashmob/go-guerrilla/backends"
+	"github.com/flashmob/go-guerrilla/log"
+	test "github.com/flashmob/go-guerrilla/tests"
+	"github.com/flashmob/go-guerrilla/tests/testcert"
+	"github.com/spf13/cobra"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -12,13 +19,6 @@ import (
 	"sync"
 	"testing"
 	"time"
-
-	"github.com/flashmob/go-guerrilla"
-	"github.com/flashmob/go-guerrilla/backends"
-	"github.com/flashmob/go-guerrilla/log"
-	test "github.com/flashmob/go-guerrilla/tests"
-	"github.com/flashmob/go-guerrilla/tests/testcert"
-	"github.com/spf13/cobra"
 )
 
 var configJsonA = `
@@ -252,10 +252,10 @@ func TestCmdConfigChangeEvents(t *testing.T) {
 	newerconf := &CmdConfig{}
 	newerconf.load([]byte(configJsonC))
 
-	expectedEvents := map[string]bool{
-		"config_change:backend_config": false,
-		"config_change:backend_name":   false,
-		"server_change:new_server":     false,
+	expectedEvents := map[guerrilla.Event]bool{
+		guerrilla.EvConfigBackendConfig: false,
+		guerrilla.EvConfigBackendName:   false,
+		guerrilla.EvConfigEvServerNew:   false,
 	}
 	mainlog, _ = log.GetLogger("off")
 
@@ -265,14 +265,14 @@ func TestCmdConfigChangeEvents(t *testing.T) {
 	if err != nil {
 		//log.Info("Failed to create new app", err)
 	}
-	toUnsubscribe := map[string]func(c *CmdConfig){}
-	toUnsubscribeS := map[string]func(c *guerrilla.ServerConfig){}
+	toUnsubscribe := map[guerrilla.Event]func(c *CmdConfig){}
+	toUnsubscribeS := map[guerrilla.Event]func(c *guerrilla.ServerConfig){}
 
 	for event := range expectedEvents {
 		// Put in anon func since range is overwriting event
-		func(e string) {
+		func(e guerrilla.Event) {
 
-			if strings.Index(e, "server_change") == 0 {
+			if strings.Index(e.String(), "server_change") == 0 {
 				f := func(c *guerrilla.ServerConfig) {
 					expectedEvents[e] = true
 				}
@@ -689,7 +689,8 @@ func TestAllowedHostsEvent(t *testing.T) {
 		logOutput := string(read)
 		//fmt.Println(logOutput)
 		if i := strings.Index(logOutput, "allowed_hosts config changed, a new list was set"); i < 0 {
-			t.Error("did not change allowed_hosts, most likely because Bus.Subscribe(\"config_change:allowed_hosts\" didnt fire")
+			t.Errorf("did not change allowed_hosts, most likely because Bus.Subscribe(\"%s\" didnt fire",
+				guerrilla.EvConfigAllowedHosts)
 		}
 	}
 	// cleanup
@@ -805,20 +806,63 @@ func TestTLSConfigEvent(t *testing.T) {
 
 }
 
-// Test for missing TLS certificate, when starting or config reload
+// Testing starting a server with a bad TLS config
+// It should not start, return exit code 1
+func TestBadTLSStart(t *testing.T) {
+	// Need to run the test in a different process by executing a command
+	// because the serve() does os.Exit when starting with a bad TLS config
+	if os.Getenv("BE_CRASHER") == "1" {
+		// do the test
+		// first, remove the good certs, if any
+		if err := os.Remove("./../../tests/mail2.guerrillamail.com.cert.pem"); err != nil {
+			mainlog.WithError(err).Error("could not remove ./../../tests/mail2.guerrillamail.com.cert.pem")
+		} else {
+			mainlog.Info("removed ./../../tests/mail2.guerrillamail.com.cert.pem")
+		}
+		// next run the server
+		ioutil.WriteFile("configJsonD.json", []byte(configJsonD), 0644)
+		conf := &CmdConfig{}           // blank one
+		conf.load([]byte(configJsonD)) // load configJsonD
+
+		cmd := &cobra.Command{}
+		configPath = "configJsonD.json"
+		var serveWG sync.WaitGroup
+
+		serveWG.Add(1)
+		go func() {
+			serve(cmd, []string{})
+			serveWG.Done()
+		}()
+		time.Sleep(testPauseDuration)
+
+		sigKill()
+		serveWG.Wait()
+
+		return
+	}
+	cmd := exec.Command(os.Args[0], "-test.run=TestBadTLSStart")
+	cmd.Env = append(os.Environ(), "BE_CRASHER=1")
+	err := cmd.Run()
+	if e, ok := err.(*exec.ExitError); ok && !e.Success() {
+		return
+	}
+	t.Error("Server started with a bad TLS config, was expecting exit status 1")
+	// cleanup
+	os.Truncate("../../tests/testlog", 0)
+	os.Remove("configJsonD.json")
+	os.Remove("./pidfile.pid")
+}
 
-func TestBadTLS(t *testing.T) {
+// Test config reload with a bad TLS config
+// It should ignore the config reload, keep running with old settings
+func TestBadTLSReload(t *testing.T) {
 	mainlog, _ = log.GetLogger("../../tests/testlog")
-	if err := os.Remove("./../../tests/mail2.guerrillamail.com.cert.pem"); err != nil {
-		mainlog.WithError(err).Error("could not remove ./../../tests/mail2.guerrillamail.com.cert.pem")
-	} else {
-		mainlog.Info("removed ./../../tests/mail2.guerrillamail.com.cert.pem")
-	}
+	// start with a good vert
+	testcert.GenerateCert("mail2.guerrillamail.com", "", 365*24*time.Hour, false, 2048, "P256", "../../tests/")
 	// start the server by emulating the serve command
 	ioutil.WriteFile("configJsonD.json", []byte(configJsonD), 0644)
 	conf := &CmdConfig{}           // blank one
 	conf.load([]byte(configJsonD)) // load configJsonD
-	conf.Servers[0].Timeout = 1
 	cmd := &cobra.Command{}
 	configPath = "configJsonD.json"
 	var serveWG sync.WaitGroup
@@ -830,79 +874,67 @@ func TestBadTLS(t *testing.T) {
 	}()
 	time.Sleep(testPauseDuration)
 
-	// Test STARTTLS handshake
-	testTlsHandshake := func() {
-		if conn, buffin, err := test.Connect(conf.AppConfig.Servers[0], 20); err != nil {
-			t.Error("Could not connect to server", conf.AppConfig.Servers[0].ListenInterface, err)
-		} else {
-			conn.SetDeadline(time.Now().Add(time.Second))
-			if result, err := test.Command(conn, buffin, "HELO"); err == nil {
-				expect := "250 mail.test.com Hello"
-				if strings.Index(result, expect) != 0 {
-					t.Error("Expected", expect, "but got", result)
-				} else {
-					if result, err = test.Command(conn, buffin, "STARTTLS"); err == nil {
-						expect := "220 2.0.0 Ready to start TLS"
-						if strings.Index(result, expect) != 0 {
-							t.Error("Expected:", expect, "but got:", result)
-						} else {
-							tlsConn := tls.Client(conn, &tls.Config{
-								InsecureSkipVerify: true,
-								ServerName:         "127.0.0.1",
-							})
-							if err := tlsConn.Handshake(); err != nil {
-								mainlog.Info("TLS Handshake failed")
-							} else {
-								t.Error("Handshake succeeded, expected it to fail", conf.AppConfig.Servers[0].ListenInterface)
-								conn = tlsConn
-
-							}
-
-						}
-					}
-				}
+	if conn, buffin, err := test.Connect(conf.AppConfig.Servers[0], 20); err != nil {
+		t.Error("Could not connect to server", conf.AppConfig.Servers[0].ListenInterface, err)
+	} else {
+		if result, err := test.Command(conn, buffin, "HELO"); err == nil {
+			expect := "250 mail.test.com Hello"
+			if strings.Index(result, expect) != 0 {
+				t.Error("Expected", expect, "but got", result)
 			}
-			conn.Close()
 		}
 	}
-	testTlsHandshake()
-
 	// write some trash data
 	ioutil.WriteFile("./../../tests/mail2.guerrillamail.com.cert.pem", []byte("trash data"), 0664)
 	ioutil.WriteFile("./../../tests/mail2.guerrillamail.com.key.pem", []byte("trash data"), 0664)
 
-	// generate a new cert
-	//testcert.GenerateCert("mail2.guerrillamail.com", "", 365 * 24 * time.Hour, false, 2048, "P256", "../../tests/")
-	sigHup()
+	newConf := conf // copy the cmdConfg
 
+	if jsonbytes, err := json.Marshal(newConf); err == nil {
+		ioutil.WriteFile("configJsonD.json", []byte(jsonbytes), 0644)
+	} else {
+		t.Error(err)
+	}
+	// send a sighup signal to the server to reload config
+	sigHup()
 	time.Sleep(testPauseDuration) // pause for config to reload
-	testTlsHandshake()
 
-	time.Sleep(testPauseDuration)
-	// send kill signal and wait for exit
+	// we should still be able to to talk to it
+
+	if conn, buffin, err := test.Connect(conf.AppConfig.Servers[0], 20); err != nil {
+		t.Error("Could not connect to server", conf.AppConfig.Servers[0].ListenInterface, err)
+	} else {
+		if result, err := test.Command(conn, buffin, "HELO"); err == nil {
+			expect := "250 mail.test.com Hello"
+			if strings.Index(result, expect) != 0 {
+				t.Error("Expected", expect, "but got", result)
+			}
+		}
+	}
+
 	sigKill()
 	serveWG.Wait()
-	// did backend started as expected?
+
+	// did config reload fail as expected?
 	fd, _ := os.Open("../../tests/testlog")
 	if read, err := ioutil.ReadAll(fd); err == nil {
 		logOutput := string(read)
 		//fmt.Println(logOutput)
-		if i := strings.Index(logOutput, "failed to load the new TLS configuration"); i < 0 {
-			t.Error("did not detect TLS load failure")
+		if i := strings.Index(logOutput, "cannot use TLS config for"); i < 0 {
+			t.Error("[127.0.0.1:2552] did not reject our tls config as expected")
 		}
 	}
 	// cleanup
 	os.Truncate("../../tests/testlog", 0)
 	os.Remove("configJsonD.json")
 	os.Remove("./pidfile.pid")
-
 }
 
 // Test for when the server config Timeout value changes
 // Start with configJsonD.json
 
 func TestSetTimeoutEvent(t *testing.T) {
-	//mainlog, _ = log.GetLogger("../../tests/testlog")
+	mainlog, _ = log.GetLogger("../../tests/testlog")
 	testcert.GenerateCert("mail2.guerrillamail.com", "", 365*24*time.Hour, false, 2048, "P256", "../../tests/")
 	// start the server by emulating the serve command
 	ioutil.WriteFile("configJsonD.json", []byte(configJsonD), 0644)
@@ -919,16 +951,6 @@ func TestSetTimeoutEvent(t *testing.T) {
 	}()
 	time.Sleep(testPauseDuration)
 
-	if conn, buffin, err := test.Connect(conf.AppConfig.Servers[0], 20); err != nil {
-		t.Error("Could not connect to server", conf.AppConfig.Servers[0].ListenInterface, err)
-	} else {
-		if result, err := test.Command(conn, buffin, "HELO"); err == nil {
-			expect := "250 mail.test.com Hello"
-			if strings.Index(result, expect) != 0 {
-				t.Error("Expected", expect, "but got", result)
-			}
-		}
-	}
 	// set the timeout to 1 second
 
 	newConf := conf // copy the cmdConfg
@@ -938,9 +960,32 @@ func TestSetTimeoutEvent(t *testing.T) {
 	} else {
 		t.Error(err)
 	}
+
 	// send a sighup signal to the server to reload config
 	sigHup()
-	time.Sleep(time.Millisecond * 1200) // pause for connection to timeout
+	time.Sleep(testPauseDuration) // config reload
+
+	var waitTimeout sync.WaitGroup
+	if conn, buffin, err := test.Connect(conf.AppConfig.Servers[0], 20); err != nil {
+		t.Error("Could not connect to server", conf.AppConfig.Servers[0].ListenInterface, err)
+	} else {
+		waitTimeout.Add(1)
+		go func() {
+			if result, err := test.Command(conn, buffin, "HELO"); err == nil {
+				expect := "250 mail.test.com Hello"
+				if strings.Index(result, expect) != 0 {
+					t.Error("Expected", expect, "but got", result)
+				} else {
+					b := make([]byte, 1024)
+					conn.Read(b)
+				}
+			}
+			waitTimeout.Done()
+		}()
+	}
+
+	// wait for timeout
+	waitTimeout.Wait()
 
 	// so the connection we have opened should timeout by now
 
@@ -951,7 +996,7 @@ func TestSetTimeoutEvent(t *testing.T) {
 	fd, _ := os.Open("../../tests/testlog")
 	if read, err := ioutil.ReadAll(fd); err == nil {
 		logOutput := string(read)
-		//fmt.Println(logOutput)
+		fmt.Println(logOutput)
 		if i := strings.Index(logOutput, "i/o timeout"); i < 0 {
 			t.Error("Connection to 127.0.0.1:2552 didn't timeout as expected")
 		}

config.go

@@ -1,6 +1,7 @@
 package guerrilla
 
 import (
+	"crypto/tls"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -36,7 +37,74 @@ type ServerConfig struct {
 	_publicKeyFile_mtime  int
 }
 
+type Event int
+
+const (
+	// when a new config was loaded
+	EvConfigNewConfig Event = iota
+	// when allowed_hosts changed
+	EvConfigAllowedHosts
+	// when pid_file changed
+	EvConfigPidFile
+	// when log_file changed
+	EvConfigLogFile
+	// when it's time to reload the main log file
+	EvConfigLogReopen
+	// when log level changed
+	EvConfigLogLevel
+	// when the backend changed
+	EvConfigBackendName
+	// when the backend's config changed
+	EvConfigBackendConfig
+	// when a new server was added
+	EvConfigEvServerNew
+	// when an existing server was removed
+	EvConfigServerRemove
+	// when a new server config was detected (general event)
+	EvConfigServerConfig
+	// when a server was enabled
+	EvConfigServerStart
+	// when a server was disabled
+	EvConfigServerStop
+	// when a server's log file changed
+	EvConfigServerLogFile
+	// when it's time to reload the server's log
+	EvConfigServerLogReopen
+	// when a server's timeout changed
+	EvConfigServerTimeout
+	// when a server's max clients changed
+	EvConfigServerMaxClients
+	// when a server's TLS config changed
+	EvConfigServerTLSConfig
+)
+
+var configEvents = [...]string{
+	"config_change:new_config",
+	"config_change:allowed_hosts",
+	"config_change:pid_file",
+	"config_change:log_file",
+	"config_change:reopen_log_file",
+	"config_change:log_level",
+	"config_change:backend_config",
+	"config_change:backend_name",
+	"server_change:new_server",
+	"server_change:remove_server",
+	"server_change:update_config",
+	"server_change:start_server",
+	"server_change:stop_server",
+	"server_change:new_log_file",
+	"server_change:reopen_log_file",
+	"server_change:timeout",
+	"server_change:max_clients",
+	"server_change:tls_config",
+}
+
+func (e Event) String() string {
+	return configEvents[e]
+}
+
 // Unmarshalls json data into AppConfig struct and any other initialization of the struct
+// also does validation, returns error if validation failed or something went wrong
 func (c *AppConfig) Load(jsonBytes []byte) error {
 	err := json.Unmarshal(jsonBytes, c)
 	if err != nil {
@@ -46,35 +114,44 @@ func (c *AppConfig) Load(jsonBytes []byte) error {
 		return errors.New("empty AllowedHosts is not allowed")
 	}
 
+	// all servers must be valid in order to continue
+	for _, server := range c.Servers {
+		if errs := server.Validate(); errs != nil {
+			return errs
+		}
+	}
+
 	// read the timestamps for the ssl keys, to determine if they need to be reloaded
 	for i := 0; i < len(c.Servers); i++ {
-		if err := c.Servers[i].loadTlsKeyTimestamps(); err != nil {
-			return err
-		}
+		c.Servers[i].loadTlsKeyTimestamps()
 	}
 	return nil
 }
 
 // Emits any configuration change events onto the event bus.
 func (c *AppConfig) EmitChangeEvents(oldConfig *AppConfig, app Guerrilla) {
+	// has config changed, general check
+	if !reflect.DeepEqual(oldConfig, c) {
+		app.Publish(EvConfigNewConfig, c)
+	}
 	// has 'allowed hosts' changed?
 	if !reflect.DeepEqual(oldConfig.AllowedHosts, c.AllowedHosts) {
-		app.Publish("config_change:allowed_hosts", c)
+		app.Publish(EvConfigAllowedHosts, c)
 	}
 	// has pid file changed?
 	if strings.Compare(oldConfig.PidFile, c.PidFile) != 0 {
-		app.Publish("config_change:pid_file", c)
+		app.Publish(EvConfigPidFile, c)
 	}
 	// has mainlog log changed?
 	if strings.Compare(oldConfig.LogFile, c.LogFile) != 0 {
-		app.Publish("config_change:log_file", c)
+		app.Publish(EvConfigLogFile, c)
 	} else {
 		// since config file has not changed, we reload it
-		app.Publish("config_change:reopen_log_file", c)
+		app.Publish(EvConfigLogReopen, c)
 	}
 	// has log level changed?
 	if strings.Compare(oldConfig.LogLevel, c.LogLevel) != 0 {
-		app.Publish("config_change:log_level", c)
+		app.Publish(EvConfigLogLevel, c)
 	}
 	// server config changes
 	oldServers := oldConfig.getServers()
@@ -83,16 +160,25 @@ func (c *AppConfig) EmitChangeEvents(oldConfig *AppConfig, app Guerrilla) {
 		if oldServer, ok := oldServers[iface]; ok {
 			// since old server exists in the new config, we do not track it anymore
 			delete(oldServers, iface)
+			// so we know the server exists in both old & new configs
 			newServer.emitChangeEvents(oldServer, app)
 		} else {
 			// start new server
-			app.Publish("server_change:new_server", newServer)
+			app.Publish(EvConfigEvServerNew, newServer)
 		}
 
 	}
 	// remove any servers that don't exist anymore
 	for _, oldserver := range oldServers {
-		app.Publish("server_change:remove_server", oldserver)
+		app.Publish(EvConfigServerRemove, oldserver)
+	}
+}
+
+// EmitLogReopen emits log reopen events using existing config
+func (c *AppConfig) EmitLogReopenEvents(app Guerrilla) {
+	app.Publish(EvConfigLogReopen, c)
+	for _, sc := range c.getServers() {
+		app.Publish(EvConfigServerLogReopen, sc)
 	}
 }
 
@@ -115,33 +201,33 @@ func (sc *ServerConfig) emitChangeEvents(oldServer *ServerConfig, app Guerrilla)
 	)
 	if len(changes) > 0 {
 		// something changed in the server config
-		app.Publish("server_change:update_config", sc)
+		app.Publish(EvConfigServerConfig, sc)
 	}
 
 	// enable or disable?
 	if _, ok := changes["IsEnabled"]; ok {
 		if sc.IsEnabled {
-			app.Publish("server_change:start_server", sc)
+			app.Publish(EvConfigServerStart, sc)
 		} else {
-			app.Publish("server_change:stop_server", sc)
+			app.Publish(EvConfigServerStop, sc)
 		}
 		// do not emit any more events when IsEnabled changed
 		return
 	}
 	// log file change?
 	if _, ok := changes["LogFile"]; ok {
-		app.Publish("server_change:new_log_file", sc)
+		app.Publish(EvConfigServerLogFile, sc)
 	} else {
 		// since config file has not changed, we reload it
-		app.Publish("server_change:reopen_log_file", sc)
+		app.Publish(EvConfigServerLogReopen, sc)
 	}
 	// timeout changed
 	if _, ok := changes["Timeout"]; ok {
-		app.Publish("server_change:timeout", sc)
+		app.Publish(EvConfigServerTimeout, sc)
 	}
 	// max_clients changed
 	if _, ok := changes["MaxClients"]; ok {
-		app.Publish("server_change:max_clients", sc)
+		app.Publish(EvConfigServerMaxClients, sc)
 	}
 
 	// tls changed
@@ -160,7 +246,7 @@ func (sc *ServerConfig) emitChangeEvents(oldServer *ServerConfig, app Guerrilla)
 		}
 		return false
 	}(); ok {
-		app.Publish("server_change:tls_config", sc)
+		app.Publish(EvConfigServerTLSConfig, sc)
 	}
 }
 
@@ -192,6 +278,19 @@ func (sc *ServerConfig) getTlsKeyTimestamps() (int, int) {
 	return sc._privateKeyFile_mtime, sc._publicKeyFile_mtime
 }
 
+// Validate validates the server's configuration.
+func (sc *ServerConfig) Validate() Errors {
+	var errs Errors
+	if _, err := tls.LoadX509KeyPair(sc.PublicKeyFile, sc.PrivateKeyFile); err != nil {
+		if sc.StartTLSOn || sc.TLSAlwaysOn {
+			errs = append(errs,
+				errors.New(fmt.Sprintf("cannot use TLS config for [%s], %v", sc.ListenInterface, err)))
+		}
+
+	}
+	return errs
+}
+
 // Returns a diff between struct a & struct b.
 // Results are returned in a map, where each key is the name of the field that was different.
 // a and b are struct values, must not be pointer

config_test.go

@@ -38,7 +38,7 @@ var configJsonA = `
             "public_key_file":"config_test.go",
             "timeout":160,
             "listen_interface":"127.0.0.1:2526",
-            "start_tls_on":true,
+            "start_tls_on":false,
             "tls_always_on":false,
             "max_clients": 2
         },
@@ -64,7 +64,7 @@ var configJsonA = `
             "public_key_file":"config_test.go",
             "timeout":160,
             "listen_interface":"127.0.0.1:9999",
-            "start_tls_on":true,
+            "start_tls_on":false,
             "tls_always_on":false,
             "max_clients": 2
         },
@@ -77,7 +77,7 @@ var configJsonA = `
             "public_key_file":"config_test.go",
             "timeout":160,
             "listen_interface":"127.0.0.1:3333",
-            "start_tls_on":true,
+            "start_tls_on":false,
             "tls_always_on":false,
             "max_clients": 2
         }
@@ -182,7 +182,7 @@ func TestSampleConfig(t *testing.T) {
 		ac := &AppConfig{}
 		if err := ac.Load(jsonBytes); err != nil {
 			// sample config can have broken tls certs
-			if strings.Index(err.Error(), "could not stat key") != 0 {
+			if strings.Index(err.Error(), "cannot use TLS config for [127.0.0.1:25") != 0 {
 				t.Error("Cannot load config", fileName, "|", err)
 				t.FailNow()
 			}
@@ -207,31 +207,31 @@ func TestConfigChangeEvents(t *testing.T) {
 	os.Chtimes(oldconf.Servers[1].PublicKeyFile, time.Now(), time.Now())
 	newconf := &AppConfig{}
 	newconf.Load([]byte(configJsonB))
-	newconf.Servers[0].LogFile = "/dev/stderr" // test for log file change
+	newconf.Servers[0].LogFile = "off" // test for log file change
 	newconf.LogLevel = "off"
 	newconf.LogFile = "off"
-	expectedEvents := map[string]bool{
-		"config_change:pid_file":        false,
-		"config_change:log_file":        false,
-		"config_change:log_level":       false,
-		"config_change:allowed_hosts":   false,
-		"server_change:new_server":      false, // 127.0.0.1:4654 will be added
-		"server_change:remove_server":   false, // 127.0.0.1:9999 server removed
-		"server_change:stop_server":     false, // 127.0.0.1:3333: server (disabled)
-		"server_change:new_log_file":    false, // 127.0.0.1:2526
-		"server_change:reopen_log_file": false, // 127.0.0.1:2527
-		"server_change:timeout":         false, // 127.0.0.1:2526 timeout
+	expectedEvents := map[Event]bool{
+		EvConfigPidFile:         false,
+		EvConfigLogFile:         false,
+		EvConfigLogLevel:        false,
+		EvConfigAllowedHosts:    false,
+		EvConfigEvServerNew:     false, // 127.0.0.1:4654 will be added
+		EvConfigServerRemove:    false, // 127.0.0.1:9999 server removed
+		EvConfigServerStop:      false, // 127.0.0.1:3333: server (disabled)
+		EvConfigServerLogFile:   false, // 127.0.0.1:2526
+		EvConfigServerLogReopen: false, // 127.0.0.1:2527
+		EvConfigServerTimeout:   false, // 127.0.0.1:2526 timeout
 		//"server_change:tls_config":    false, // 127.0.0.1:2526
-		"server_change:max_clients": false, // 127.0.0.1:2526
-		"server_change:tls_config":  false, // 127.0.0.1:2527 timestamp changed on certificates
+		EvConfigServerMaxClients: false, // 127.0.0.1:2526
+		EvConfigServerTLSConfig:  false, // 127.0.0.1:2527 timestamp changed on certificates
 	}
-	toUnsubscribe := map[string]func(c *AppConfig){}
-	toUnsubscribeS := map[string]func(c *ServerConfig){}
+	toUnsubscribe := map[Event]func(c *AppConfig){}
+	toUnsubscribeSrv := map[Event]func(c *ServerConfig){}
 
 	for event := range expectedEvents {
 		// Put in anon func since range is overwriting event
-		func(e string) {
-			if strings.Index(e, "config_change") != -1 {
+		func(e Event) {
+			if strings.Index(e.String(), "config_change") != -1 {
 				f := func(c *AppConfig) {
 					expectedEvents[e] = true
 				}
@@ -243,7 +243,7 @@ func TestConfigChangeEvents(t *testing.T) {
 					expectedEvents[e] = true
 				}
 				app.Subscribe(event, f)
-				toUnsubscribeS[event] = f
+				toUnsubscribeSrv[event] = f
 			}
 
 		}(event)
@@ -255,7 +255,7 @@ func TestConfigChangeEvents(t *testing.T) {
 	for unevent, unfun := range toUnsubscribe {
 		app.Unsubscribe(unevent, unfun)
 	}
-	for unevent, unfun := range toUnsubscribeS {
+	for unevent, unfun := range toUnsubscribeSrv {
 		app.Unsubscribe(unevent, unfun)
 	}
 	for event, val := range expectedEvents {

guerrilla.go

@@ -36,9 +36,9 @@ func (e Errors) Error() string {
 type Guerrilla interface {
 	Start() error
 	Shutdown()
-	Subscribe(topic string, fn interface{}) error
-	Publish(topic string, args ...interface{})
-	Unsubscribe(topic string, handler interface{}) error
+	Subscribe(topic Event, fn interface{}) error
+	Publish(topic Event, args ...interface{})
+	Unsubscribe(topic Event, handler interface{}) error
 	SetLogger(log.Logger)
 }
 
@@ -47,10 +47,10 @@ type guerrilla struct {
 	servers map[string]*server
 	backend backends.Backend
 	// guard controls access to g.servers
-	guard   sync.Mutex
-	state   int8
-	bus     *evbus.EventBus
-	mainlog logStore
+	guard sync.Mutex
+	state int8
+	bus   *evbus.EventBus
+	logStore
 }
 
 type logStore struct {
@@ -58,7 +58,7 @@ type logStore struct {
 }
 
 // Get loads the log.logger in an atomic operation. Returns a stderr logger if not able to load
-func (ls *logStore) Get() log.Logger {
+func (ls *logStore) mainlog() log.Logger {
 	if v, ok := ls.Load().(log.Logger); ok {
 		return v
 	}
@@ -66,6 +66,11 @@ func (ls *logStore) Get() log.Logger {
 	return l
 }
 
+// storeMainlog stores the log value in an atomic operation
+func (ls *logStore) storeMainlog(log log.Logger) {
+	ls.Store(log)
+}
+
 // Returns a new instance of Guerrilla with the given config, not yet running.
 func New(ac *AppConfig, b backends.Backend, l log.Logger) (Guerrilla, error) {
 	g := &guerrilla{
@@ -74,10 +79,10 @@ func New(ac *AppConfig, b backends.Backend, l log.Logger) (Guerrilla, error) {
 		backend: b,
 		bus:     evbus.New(),
 	}
-	g.mainlog.Store(l)
+	g.storeMainlog(l)
 
 	if ac.LogLevel != "" {
-		g.mainlog.Get().SetLevel(ac.LogLevel)
+		g.mainlog().SetLevel(ac.LogLevel)
 	}
 
 	g.state = GuerrillaStateNew
@@ -90,23 +95,28 @@ func New(ac *AppConfig, b backends.Backend, l log.Logger) (Guerrilla, error) {
 
 // Instantiate servers
 func (g *guerrilla) makeServers() error {
-	g.mainlog.Get().Debug("making servers")
+	g.mainlog().Debug("making servers")
 	var errs Errors
 	for _, sc := range g.Config.Servers {
 		if _, ok := g.servers[sc.ListenInterface]; ok {
 			// server already instantiated
 			continue
 		}
-		server, err := newServer(&sc, g.backend, g.mainlog.Get())
-		if err != nil {
-			g.mainlog.Get().WithError(err).Errorf("Failed to create server [%s]", sc.ListenInterface)
-			errs = append(errs, err)
-		}
-		if server != nil {
-			g.servers[sc.ListenInterface] = server
-			server.setAllowedHosts(g.Config.AllowedHosts)
+		if errs := sc.Validate(); errs != nil {
+			g.mainlog().WithError(errs).Errorf("Failed to create server [%s]", sc.ListenInterface)
+			errs = append(errs, errs...)
+			continue
+		} else {
+			server, err := newServer(&sc, g.backend, g.mainlog())
+			if err != nil {
+				g.mainlog().WithError(err).Errorf("Failed to create server [%s]", sc.ListenInterface)
+				errs = append(errs, err)
+			}
+			if server != nil {
+				g.servers[sc.ListenInterface] = server
+				server.setAllowedHosts(g.Config.AllowedHosts)
+			}
 		}
-
 	}
 	if len(g.servers) == 0 {
 		errs = append(errs, errors.New("There are no servers that can start, please check your config"))
@@ -117,41 +127,36 @@ func (g *guerrilla) makeServers() error {
 	return errs
 }
 
-// find a server by interface, retuning the index of the config and instance of server
-func (g *guerrilla) findServer(iface string) (int, *server) {
+// find a server by interface, retuning the server or err
+func (g *guerrilla) findServer(iface string) (*server, error) {
 	g.guard.Lock()
 	defer g.guard.Unlock()
-	ret := -1
-	for i := range g.Config.Servers {
-		if g.Config.Servers[i].ListenInterface == iface {
-			server := g.servers[iface]
-			ret = i
-			return ret, server
-		}
+	if server, ok := g.servers[iface]; ok {
+		return server, nil
 	}
-	return ret, nil
+	return nil, errors.New("server not found in g.servers")
 }
 
-func (g *guerrilla) removeServer(serverConfigIndex int, iface string) {
+func (g *guerrilla) removeServer(iface string) {
 	g.guard.Lock()
 	defer g.guard.Unlock()
 	delete(g.servers, iface)
-	// cut out from the slice
-	g.Config.Servers = append(g.Config.Servers[:serverConfigIndex], g.Config.Servers[1:]...)
 }
 
-func (g *guerrilla) addServer(sc *ServerConfig) {
+// setConfig sets the app config
+func (g *guerrilla) setConfig(c *AppConfig) {
 	g.guard.Lock()
 	defer g.guard.Unlock()
-	g.Config.Servers = append(g.Config.Servers, *sc)
-	g.makeServers()
+	g.Config = *c
 }
 
-func (g *guerrilla) setConfig(i int, sc *ServerConfig) {
+// setServerConfig config updates the server's config, which will update for the next connected client
+func (g *guerrilla) setServerConfig(sc *ServerConfig) {
 	g.guard.Lock()
 	defer g.guard.Unlock()
-	g.Config.Servers[i] = *sc
-	g.servers[sc.ListenInterface].setConfig(sc)
+	if _, ok := g.servers[sc.ListenInterface]; ok {
+		g.servers[sc.ListenInterface].setConfig(sc)
+	}
 }
 
 // mapServers calls a callback on each server in g.servers map
@@ -168,132 +173,140 @@ func (g *guerrilla) mapServers(callback func(*server)) map[string]*server {
 // subscribeEvents subscribes event handlers for configuration change events
 func (g *guerrilla) subscribeEvents() {
 
+	// main config changed
+	g.Subscribe(EvConfigNewConfig, func(c *AppConfig) {
+		g.setConfig(c)
+	})
+
 	// allowed_hosts changed, set for all servers
-	g.Subscribe("config_change:allowed_hosts", func(c *AppConfig) {
+	g.Subscribe(EvConfigAllowedHosts, func(c *AppConfig) {
 		g.mapServers(func(server *server) {
 			server.setAllowedHosts(c.AllowedHosts)
 		})
-		g.mainlog.Get().Infof("allowed_hosts config changed, a new list was set")
+		g.mainlog().Infof("allowed_hosts config changed, a new list was set")
 	})
 
 	// the main log file changed
-	g.Subscribe("config_change:log_file", func(c *AppConfig) {
+	g.Subscribe(EvConfigLogFile, func(c *AppConfig) {
 		var err error
 		var l log.Logger
 		if l, err = log.GetLogger(c.LogFile); err == nil {
-			g.mainlog.Store(l)
+			g.storeMainlog(l)
 			g.mapServers(func(server *server) {
-				server.mainlogStore.Store(l) // it will change to hl on the next accepted client
+				// it will change server's logger when the next client gets accepted
+				server.mainlogStore.Store(l)
 			})
-			g.mainlog.Get().Infof("main log for new clients changed to to [%s]", c.LogFile)
+			g.mainlog().Infof("main log for new clients changed to to [%s]", c.LogFile)
 		} else {
-			g.mainlog.Get().WithError(err).Errorf("main logging change failed [%s]", c.LogFile)
+			g.mainlog().WithError(err).Errorf("main logging change failed [%s]", c.LogFile)
 		}
 
 	})
 
 	// re-open the main log file (file not changed)
-	g.Subscribe("config_change:reopen_log_file", func(c *AppConfig) {
-		g.mainlog.Get().Reopen()
-		g.mainlog.Get().Infof("re-opened main log file [%s]", c.LogFile)
+	g.Subscribe(EvConfigLogReopen, func(c *AppConfig) {
+		g.mainlog().Reopen()
+		g.mainlog().Infof("re-opened main log file [%s]", c.LogFile)
 	})
 
 	// when log level changes, apply to mainlog and server logs
-	g.Subscribe("config_change:log_level", func(c *AppConfig) {
-		g.mainlog.Get().SetLevel(c.LogLevel)
+	g.Subscribe(EvConfigLogLevel, func(c *AppConfig) {
+		g.mainlog().SetLevel(c.LogLevel)
 		g.mapServers(func(server *server) {
 			server.log.SetLevel(c.LogLevel)
 		})
-		g.mainlog.Get().Infof("log level changed to [%s]", c.LogLevel)
+		g.mainlog().Infof("log level changed to [%s]", c.LogLevel)
 	})
 
 	// server config was updated
-	g.Subscribe("server_change:update_config", func(sc *ServerConfig) {
-		if i, _ := g.findServer(sc.ListenInterface); i != -1 {
-			g.setConfig(i, sc)
-		}
+	g.Subscribe(EvConfigServerConfig, func(sc *ServerConfig) {
+		g.setServerConfig(sc)
 	})
 
 	// add a new server to the config & start
-	g.Subscribe("server_change:new_server", func(sc *ServerConfig) {
-		if i, _ := g.findServer(sc.ListenInterface); i == -1 {
+	g.Subscribe(EvConfigEvServerNew, func(sc *ServerConfig) {
+		if _, err := g.findServer(sc.ListenInterface); err != nil {
 			// not found, lets add it
-			g.addServer(sc)
-			g.mainlog.Get().Infof("New server added [%s]", sc.ListenInterface)
+			if err := g.makeServers(); err != nil {
+				g.mainlog().WithError(err).Error("cannot add server [%s]", sc.ListenInterface)
+				return
+			}
+			g.mainlog().Infof("New server added [%s]", sc.ListenInterface)
 			if g.state == GuerrillaStateStarted {
 				err := g.Start()
 				if err != nil {
-					g.mainlog.Get().WithError(err).Info("Event server_change:new_server returned errors when starting")
+					g.mainlog().WithError(err).Info("Event server_change:new_server returned errors when starting")
 				}
 			}
 		}
 	})
-	// start a server that already exists in the config and has been instantiated
-	g.Subscribe("server_change:start_server", func(sc *ServerConfig) {
-		if i, server := g.findServer(sc.ListenInterface); i != -1 {
+	// start a server that already exists in the config and has been enabled
+	g.Subscribe(EvConfigServerStart, func(sc *ServerConfig) {
+		if server, err := g.findServer(sc.ListenInterface); err == nil {
 			if server.state == ServerStateStopped || server.state == ServerStateNew {
-				g.mainlog.Get().Infof("Starting server [%s]", server.listenInterface)
+				g.mainlog().Infof("Starting server [%s]", server.listenInterface)
 				err := g.Start()
 				if err != nil {
-					g.mainlog.Get().WithError(err).Info("Event server_change:start_server returned errors when starting")
+					g.mainlog().WithError(err).Info("Event server_change:start_server returned errors when starting")
 				}
 			}
 		}
 	})
 	// stop running a server
-	g.Subscribe("server_change:stop_server", func(sc *ServerConfig) {
-		if i, server := g.findServer(sc.ListenInterface); i != -1 {
+	g.Subscribe(EvConfigServerStop, func(sc *ServerConfig) {
+		if server, err := g.findServer(sc.ListenInterface); err == nil {
 			if server.state == ServerStateRunning {
 				server.Shutdown()
-				g.mainlog.Get().Infof("Server [%s] stopped.", sc.ListenInterface)
+				g.mainlog().Infof("Server [%s] stopped.", sc.ListenInterface)
 			}
 		}
 	})
 	// server was removed from config
-	g.Subscribe("server_change:remove_server", func(sc *ServerConfig) {
-		if i, server := g.findServer(sc.ListenInterface); i != -1 {
+	g.Subscribe(EvConfigServerRemove, func(sc *ServerConfig) {
+		if server, err := g.findServer(sc.ListenInterface); err == nil {
 			server.Shutdown()
-			g.removeServer(i, sc.ListenInterface)
-			g.mainlog.Get().Infof("Server [%s] removed from config, stopped it.", sc.ListenInterface)
+			g.removeServer(sc.ListenInterface)
+			g.mainlog().Infof("Server [%s] removed from config, stopped it.", sc.ListenInterface)
 		}
 	})
 
 	// TLS changes
-	g.Subscribe("server_change:tls_config", func(sc *ServerConfig) {
-		if i, server := g.findServer(sc.ListenInterface); i != -1 {
+	g.Subscribe(EvConfigServerTLSConfig, func(sc *ServerConfig) {
+		if server, err := g.findServer(sc.ListenInterface); err == nil {
 			if err := server.configureSSL(); err == nil {
-				g.mainlog.Get().Infof("Server [%s] new TLS configuration loaded", sc.ListenInterface)
+				g.mainlog().Infof("Server [%s] new TLS configuration loaded", sc.ListenInterface)
 			} else {
-				g.mainlog.Get().WithError(err).Errorf("Server [%s] failed to load the new TLS configuration", sc.ListenInterface)
+				g.mainlog().WithError(err).Errorf("Server [%s] failed to load the new TLS configuration", sc.ListenInterface)
 			}
 		}
 	})
 	// when server's timeout change.
-	g.Subscribe("server_change:timeout", func(sc *ServerConfig) {
+	g.Subscribe(EvConfigServerTimeout, func(sc *ServerConfig) {
 		g.mapServers(func(server *server) {
 			server.setTimeout(sc.Timeout)
 		})
 	})
 	// when server's max clients change.
-	g.Subscribe("server_change:max_clients", func(sc *ServerConfig) {
+	g.Subscribe(EvConfigServerMaxClients, func(sc *ServerConfig) {
 		g.mapServers(func(server *server) {
 			// TODO resize the pool somehow
 		})
 	})
 	// when a server's log file changes
-	g.Subscribe("server_change:new_log_file", func(sc *ServerConfig) {
-		if i, server := g.findServer(sc.ListenInterface); i != -1 {
+	g.Subscribe(EvConfigServerLogFile, func(sc *ServerConfig) {
+		if server, err := g.findServer(sc.ListenInterface); err == nil {
 			var err error
 			var l log.Logger
 			if l, err = log.GetLogger(sc.LogFile); err == nil {
-				g.mainlog.Store(l)
-				server.logStore.Store(l) // it will change to l on the next accepted client
-				g.mainlog.Get().Infof("Server [%s] changed, new clients will log to: [%s]",
+				g.storeMainlog(l)
+				// it will change to the new logger on the next accepted client
+				server.logStore.Store(l)
+				g.mainlog().Infof("Server [%s] changed, new clients will log to: [%s]",
 					sc.ListenInterface,
 					sc.LogFile,
 				)
 			} else {
-				g.mainlog.Get().WithError(err).Errorf(
+				g.mainlog().WithError(err).Errorf(
 					"Server [%s] log change failed to: [%s]",
 					sc.ListenInterface,
 					sc.LogFile,
@@ -301,11 +314,11 @@ func (g *guerrilla) subscribeEvents() {
 			}
 		}
 	})
-	// when the daemon caught a sighup
-	g.Subscribe("server_change:reopen_log_file", func(sc *ServerConfig) {
-		if i, server := g.findServer(sc.ListenInterface); i != -1 {
+	// when the daemon caught a sighup, event for individual server
+	g.Subscribe(EvConfigServerLogReopen, func(sc *ServerConfig) {
+		if server, err := g.findServer(sc.ListenInterface); err == nil {
 			server.log.Reopen()
-			g.mainlog.Get().Infof("Server [%s] re-opened log file [%s]", sc.ListenInterface, sc.LogFile)
+			g.mainlog().Infof("Server [%s] re-opened log file [%s]", sc.ListenInterface, sc.LogFile)
 		}
 	})
 
@@ -328,6 +341,7 @@ func (g *guerrilla) Start() error {
 
 	// start servers, send any errors back to errs channel
 	for ListenInterface := range g.servers {
+		g.mainlog().Infof("Starting: %s", ListenInterface)
 		if !g.servers[ListenInterface].isEnabled() {
 			// not enabled
 			continue
@@ -374,28 +388,28 @@ func (g *guerrilla) Shutdown() {
 	for ListenInterface, s := range g.servers {
 		if s.state == ServerStateRunning {
 			s.Shutdown()
-			g.mainlog.Get().Infof("shutdown completed for [%s]", ListenInterface)
+			g.mainlog().Infof("shutdown completed for [%s]", ListenInterface)
 		}
 	}
 	if err := g.backend.Shutdown(); err != nil {
-		g.mainlog.Get().WithError(err).Warn("Backend failed to shutdown")
+		g.mainlog().WithError(err).Warn("Backend failed to shutdown")
 	} else {
-		g.mainlog.Get().Infof("Backend shutdown completed")
+		g.mainlog().Infof("Backend shutdown completed")
 	}
 }
 
-func (g *guerrilla) Subscribe(topic string, fn interface{}) error {
-	return g.bus.Subscribe(topic, fn)
+func (g *guerrilla) Subscribe(topic Event, fn interface{}) error {
+	return g.bus.Subscribe(topic.String(), fn)
 }
 
-func (g *guerrilla) Publish(topic string, args ...interface{}) {
-	g.bus.Publish(topic, args...)
+func (g *guerrilla) Publish(topic Event, args ...interface{}) {
+	g.bus.Publish(topic.String(), args...)
 }
 
-func (g *guerrilla) Unsubscribe(topic string, handler interface{}) error {
-	return g.bus.Unsubscribe(topic, handler)
+func (g *guerrilla) Unsubscribe(topic Event, handler interface{}) error {
+	return g.bus.Unsubscribe(topic.String(), handler)
 }
 
 func (g *guerrilla) SetLogger(l log.Logger) {
-	g.mainlog.Store(l)
+	g.storeMainlog(l)
 }

server.go

@@ -65,8 +65,8 @@ type server struct {
 }
 
 type allowedHosts struct {
-	table map[string]bool // host lookup table
-	m     sync.Mutex      // guard access to the map
+	table      map[string]bool // host lookup table
+	sync.Mutex                 // guard access to the map
 }
 
 // Creates and returns a new ready-to-run Server from a configuration
@@ -156,8 +156,8 @@ func (server *server) isEnabled() bool {
 
 // Set the allowed hosts for the server
 func (server *server) setAllowedHosts(allowedHosts []string) {
-	defer server.hosts.m.Unlock()
-	server.hosts.m.Lock()
+	server.hosts.Lock()
+	defer server.hosts.Unlock()
 	server.hosts.table = make(map[string]bool, len(allowedHosts))
 	for _, h := range allowedHosts {
 		server.hosts.table[strings.ToLower(h)] = true
@@ -239,8 +239,8 @@ func (server *server) GetActiveClientsCount() int {
 
 // Verifies that the host is a valid recipient.
 func (server *server) allowsHost(host string) bool {
-	defer server.hosts.m.Unlock()
-	server.hosts.m.Lock()
+	server.hosts.Lock()
+	defer server.hosts.Unlock()
 	if _, ok := server.hosts.table[strings.ToLower(host)]; ok {
 		return true
 	}

server_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	"bufio"
-	"fmt"
 	"net/textproto"
 	"strings"
 	"sync"
@@ -76,14 +75,14 @@ func TestHandleClient(t *testing.T) {
 	// Wait for the greeting from the server
 	r := textproto.NewReader(bufio.NewReader(conn.Client))
 	line, _ := r.ReadLine()
-	fmt.Println(line)
+	//	fmt.Println(line)
 	w := textproto.NewWriter(bufio.NewWriter(conn.Client))
 	w.PrintfLine("HELO test.test.com")
 	line, _ = r.ReadLine()
-	fmt.Println(line)
+	//fmt.Println(line)
 	w.PrintfLine("QUIT")
 	line, _ = r.ReadLine()
-	fmt.Println("line is:", line)
+	//fmt.Println("line is:", line)
 	expected := "221 2.0.0 Bye"
 	if strings.Index(line, expected) != 0 {
 		t.Error("expected", expected, "but got:", line)