Skip to content

Commit

Permalink
Code style fixes (flashmob#131)
Browse files Browse the repository at this point in the history 
* Codebase: fix code style issues, error checking where possible, fix flashmob#96 
* Tests: fix race conditions in tests, which caused random failures in the past, fix flashmob#96 
* Cross-platform compatibility: Open-file-limit test, capture os.kill signal for maximal compatibility
* Header parsing: increase max header size to 4kb
* Header parsing: avoid extra copy when parsing headers
  • Loading branch information
@flashmob
flashmob authored Jan 30, 2019
1 parent a910a48 commit 3493a86
Show file tree
Hide file tree
Showing 35 changed files with 1,553 additions and 837 deletions.
6 changes: 2 additions & 4 deletions api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ type deferredSub struct {
fn interface{}
}

const defaultInterface = "127.0.0.1:2525"

// AddProcessor adds a processor constructor to the backend.
// name is the identifier to be used in the config. See backends docs for more info.
func (d *Daemon) AddProcessor(name string, pc backends.ProcessorConstructor) {
Expand Down Expand Up @@ -64,7 +62,7 @@ func (d *Daemon) Start() (err error) {
return err
}
for i := range d.subs {
d.Subscribe(d.subs[i].topic, d.subs[i].fn)
_ = d.Subscribe(d.subs[i].topic, d.subs[i].fn)

}
d.subs = make([]deferredSub, 0)
Expand Down Expand Up @@ -164,10 +162,10 @@ func (d *Daemon) ReopenLogs() error {
// Subscribe for subscribing to config change events
func (d *Daemon) Subscribe(topic Event, fn interface{}) error {
if d.g == nil {
// defer the subscription until the daemon is started
d.subs = append(d.subs, deferredSub{topic, fn})
return nil
}

return d.g.Subscribe(topic, fn)
}

Expand Down
166 changes: 131 additions & 35 deletions api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,9 @@ func TestSMTPLoadFile(t *testing.T) {
return
}

d.ReloadConfigFile("goguerrilla.conf.api")
if err = d.ReloadConfigFile("goguerrilla.conf.api"); err != nil {
t.Error(err)
}

if d.Config.LogFile != "./tests/testlog2" {
t.Error("d.Config.LogFile != \"./tests/testlog\"")
Expand All @@ -227,7 +229,9 @@ func TestSMTPLoadFile(t *testing.T) {
}

func TestReopenLog(t *testing.T) {
os.Truncate("test/testlog", 0)
if err := os.Truncate("tests/testlog", 0); err != nil {
t.Error(err)
}
cfg := &AppConfig{LogFile: "tests/testlog"}
sc := ServerConfig{
ListenInterface: "127.0.0.1:2526",
Expand All @@ -240,7 +244,9 @@ func TestReopenLog(t *testing.T) {
if err != nil {
t.Error("start error", err)
} else {
d.ReopenLogs()
if err = d.ReopenLogs(); err != nil {
t.Error(err)
}
time.Sleep(time.Second * 2)

d.Shutdown()
Expand All @@ -261,7 +267,9 @@ func TestReopenLog(t *testing.T) {

func TestSetConfig(t *testing.T) {

os.Truncate("test/testlog", 0)
if err := os.Truncate("tests/testlog", 0); err != nil {
t.Error(err)
}
cfg := AppConfig{LogFile: "tests/testlog"}
sc := ServerConfig{
ListenInterface: "127.0.0.1:2526",
Expand Down Expand Up @@ -305,7 +313,9 @@ func TestSetConfig(t *testing.T) {

func TestSetConfigError(t *testing.T) {

os.Truncate("tests/testlog", 0)
if err := os.Truncate("tests/testlog", 0); err != nil {
t.Error(err)
}
cfg := AppConfig{LogFile: "tests/testlog"}
sc := ServerConfig{
ListenInterface: "127.0.0.1:2526",
Expand Down Expand Up @@ -369,7 +379,9 @@ var funkyLogger = func() backends.Decorator {

// How about a custom processor?
func TestSetAddProcessor(t *testing.T) {
os.Truncate("tests/testlog", 0)
if err := os.Truncate("tests/testlog", 0); err != nil {
t.Error(err)
}
cfg := &AppConfig{
LogFile: "tests/testlog",
AllowedHosts: []string{"grr.la"},
Expand All @@ -381,9 +393,13 @@ func TestSetAddProcessor(t *testing.T) {
d := Daemon{Config: cfg}
d.AddProcessor("FunkyLogger", funkyLogger)

d.Start()
if err := d.Start(); err != nil {
t.Error(err)
}
// lets have a talk with the server
talkToServer("127.0.0.1:2525")
if err := talkToServer("127.0.0.1:2525"); err != nil {
t.Error(err)
}

d.Shutdown()

Expand All @@ -410,38 +426,87 @@ func TestSetAddProcessor(t *testing.T) {

}

func talkToServer(address string) {
func talkToServer(address string) (err error) {

conn, err := net.Dial("tcp", address)
if err != nil {

return
}
in := bufio.NewReader(conn)
str, err := in.ReadString('\n')
fmt.Fprint(conn, "HELO maildiranasaurustester\r\n")
if err != nil {
return err
}
_, err = fmt.Fprint(conn, "HELO maildiranasaurustester\r\n")
if err != nil {
return err
}
str, err = in.ReadString('\n')
fmt.Fprint(conn, "MAIL FROM:<[email protected]>r\r\n")
if err != nil {
return err
}
_, err = fmt.Fprint(conn, "MAIL FROM:<[email protected]>r\r\n")
if err != nil {
return err
}
str, err = in.ReadString('\n')
fmt.Fprint(conn, "RCPT TO:<[email protected]>\r\n")
if err != nil {
return err
}
if err != nil {
return err
}
_, err = fmt.Fprint(conn, "RCPT TO:<[email protected]>\r\n")
if err != nil {
return err
}
str, err = in.ReadString('\n')
fmt.Fprint(conn, "DATA\r\n")
if err != nil {
return err
}
_, err = fmt.Fprint(conn, "DATA\r\n")
if err != nil {
return err
}
str, err = in.ReadString('\n')
fmt.Fprint(conn, "Subject: Test subject\r\n")
fmt.Fprint(conn, "\r\n")
fmt.Fprint(conn, "A an email body\r\n")
fmt.Fprint(conn, ".\r\n")
if err != nil {
return err
}
_, err = fmt.Fprint(conn, "Subject: Test subject\r\n")
if err != nil {
return err
}
_, err = fmt.Fprint(conn, "\r\n")
if err != nil {
return err
}
_, err = fmt.Fprint(conn, "A an email body\r\n")
if err != nil {
return err
}
_, err = fmt.Fprint(conn, ".\r\n")
if err != nil {
return err
}
str, err = in.ReadString('\n')
if err != nil {
return err
}
_ = str
return nil
}

// Test hot config reload
// Here we forgot to add FunkyLogger so backend will fail to init

func TestReloadConfig(t *testing.T) {
os.Truncate("tests/testlog", 0)
if err := os.Truncate("tests/testlog", 0); err != nil {
t.Error(err)
}
d := Daemon{}
d.Start()
if err := d.Start(); err != nil {
t.Error(err)
}
defer d.Shutdown()
cfg := AppConfig{
LogFile: "tests/testlog",
Expand All @@ -452,16 +517,22 @@ func TestReloadConfig(t *testing.T) {
},
}
// Look mom, reloading the config without shutting down!
d.ReloadConfig(cfg)
if err := d.ReloadConfig(cfg); err != nil {
t.Error(err)
}

}

func TestPubSubAPI(t *testing.T) {

os.Truncate("tests/testlog", 0)
if err := os.Truncate("tests/testlog", 0); err != nil {
t.Error(err)
}

d := Daemon{Config: &AppConfig{LogFile: "tests/testlog"}}
d.Start()
if err := d.Start(); err != nil {
t.Error(err)
}
defer d.Shutdown()
// new config
cfg := AppConfig{
Expand All @@ -482,14 +553,22 @@ func TestPubSubAPI(t *testing.T) {
}
d.Logger.Info("number", i)
}
d.Subscribe(EventConfigPidFile, pidEvHandler)
if err := d.Subscribe(EventConfigPidFile, pidEvHandler); err != nil {
t.Error(err)
}

d.ReloadConfig(cfg)
if err := d.ReloadConfig(cfg); err != nil {
t.Error(err)
}

d.Unsubscribe(EventConfigPidFile, pidEvHandler)
if err := d.Unsubscribe(EventConfigPidFile, pidEvHandler); err != nil {
t.Error(err)
}
cfg.PidFile = "tests/pidfile2.pid"
d.Publish(EventConfigPidFile, &cfg)
d.ReloadConfig(cfg)
if err := d.ReloadConfig(cfg); err != nil {
t.Error(err)
}

b, err := ioutil.ReadFile("tests/testlog")
if err != nil {
Expand All @@ -504,7 +583,9 @@ func TestPubSubAPI(t *testing.T) {
}

func TestAPILog(t *testing.T) {
os.Truncate("tests/testlog", 0)
if err := os.Truncate("tests/testlog", 0); err != nil {
t.Error(err)
}
d := Daemon{}
l := d.Log()
l.Info("logtest1") // to stderr
Expand Down Expand Up @@ -540,18 +621,29 @@ func TestSkipAllowsHost(t *testing.T) {
defer d.Shutdown()
// setting the allowed hosts to a single entry with a dot will let any host through
d.Config = &AppConfig{AllowedHosts: []string{"."}, LogFile: "off"}
d.Start()
if err := d.Start(); err != nil {
t.Error(err)
}

conn, err := net.Dial("tcp", d.Config.Servers[0].ListenInterface)
if err != nil {
t.Error(t)
return
}
in := bufio.NewReader(conn)
fmt.Fprint(conn, "HELO test\r\n")
fmt.Fprint(conn, "RCPT TO:<[email protected]>\r\n")
in.ReadString('\n')
in.ReadString('\n')
if _, err := fmt.Fprint(conn, "HELO test\r\n"); err != nil {
t.Error(err)
}
if _, err := fmt.Fprint(conn, "RCPT TO:<[email protected]>\r\n"); err != nil {
t.Error(err)
}

if _, err := in.ReadString('\n'); err != nil {
t.Error(err)
}
if _, err := in.ReadString('\n'); err != nil {
t.Error(err)
}
str, _ := in.ReadString('\n')
if strings.Index(str, "250") != 0 {
t.Error("expected 250 reply, got:", str)
Expand All @@ -578,7 +670,9 @@ var customBackend2 = func() backends.Decorator {

// Test a custom backend response
func TestCustomBackendResult(t *testing.T) {
os.Truncate("tests/testlog", 0)
if err := os.Truncate("tests/testlog", 0); err != nil {
t.Error(err)
}
cfg := &AppConfig{
LogFile: "tests/testlog",
AllowedHosts: []string{"grr.la"},
Expand All @@ -594,7 +688,9 @@ func TestCustomBackendResult(t *testing.T) {
t.Error(err)
}
// lets have a talk with the server
talkToServer("127.0.0.1:2525")
if err := talkToServer("127.0.0.1:2525"); err != nil {
t.Error(err)
}

d.Shutdown()

Expand Down
Loading

0 comments on commit 3493a86

Please sign in to comment.