From 06165cb3d3d20420d1dacca265a8130b0f5c45dc Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Tue, 26 Dec 2017 22:16:47 -0800 Subject: [PATCH] Many linter fixes for smtpd pkg --- smtpd/filemsg.go | 2 +- smtpd/filestore.go | 12 +++++++----- smtpd/filestore_test.go | 2 +- smtpd/handler.go | 18 +++++++----------- smtpd/handler_test.go | 12 +++--------- smtpd/listener.go | 8 +++----- smtpd/utils.go | 35 ++++++++++++++++++++++++++++------- 7 files changed, 50 insertions(+), 39 deletions(-) diff --git a/smtpd/filemsg.go b/smtpd/filemsg.go index 308746a..9030dbe 100644 --- a/smtpd/filemsg.go +++ b/smtpd/filemsg.go @@ -72,7 +72,7 @@ func (m *FileMessage) From() string { return m.Ffrom } -// From returns the value of the Message To header +// To returns the value of the Message To header func (m *FileMessage) To() []string { return m.Fto } diff --git a/smtpd/filestore.go b/smtpd/filestore.go index 4717e3a..900c480 100644 --- a/smtpd/filestore.go +++ b/smtpd/filestore.go @@ -150,10 +150,12 @@ type FileMailbox struct { messages []*FileMessage } +// Name of the mailbox func (mb *FileMailbox) Name() string { return mb.name } +// String renders the name and directory path of the mailbox func (mb *FileMailbox) String() string { return mb.name + "[" + mb.dirName + "]" } @@ -184,11 +186,11 @@ func (mb *FileMailbox) GetMessage(id string) (datastore.Message, error) { if id == "latest" && len(mb.messages) != 0 { return mb.messages[len(mb.messages)-1], nil - } else { - for _, m := range mb.messages { - if m.Fid == id { - return m, nil - } + } + + for _, m := range mb.messages { + if m.Fid == id { + return m, nil } } diff --git a/smtpd/filestore_test.go b/smtpd/filestore_test.go index 2ca3104..a27e5ca 100644 --- a/smtpd/filestore_test.go +++ b/smtpd/filestore_test.go @@ -496,7 +496,7 @@ func TestGetLatestMessage(t *testing.T) { assert.True(t, msg.ID() == id3, "Expected %q to be equal to %q", msg.ID(), id3) // Test wrong id - msg, err = mb.GetMessage("wrongid") + _, err = mb.GetMessage("wrongid") assert.Error(t, err) if t.Failed() { diff --git a/smtpd/handler.go b/smtpd/handler.go index a5dd34b..fdb0177 100644 --- a/smtpd/handler.go +++ b/smtpd/handler.go @@ -511,20 +511,16 @@ func (ss *Session) send(msg string) { // readByteLine reads a line of input into the provided buffer. Does // not reset the Buffer - please do so prior to calling. -func (ss *Session) readByteLine(buf *bytes.Buffer) error { +func (ss *Session) readByteLine(buf io.Writer) error { if err := ss.conn.SetReadDeadline(ss.nextDeadline()); err != nil { return err } - for { - line, err := ss.reader.ReadBytes('\n') - if err != nil { - return err - } - if _, err = buf.Write(line); err != nil { - return err - } - return nil + line, err := ss.reader.ReadBytes('\n') + if err != nil { + return err } + _, err = buf.Write(line) + return err } // Reads a line of input @@ -573,7 +569,7 @@ func (ss *Session) parseCmd(line string) (cmd string, arg string, ok bool) { // The leading space is mandatory. func (ss *Session) parseArgs(arg string) (args map[string]string, ok bool) { args = make(map[string]string) - re := regexp.MustCompile(" (\\w+)=(\\w+)") + re := regexp.MustCompile(` (\w+)=(\w+)`) pm := re.FindAllStringSubmatch(arg, -1) if pm == nil { ss.logWarn("Failed to parse arg string: %q") diff --git a/smtpd/handler_test.go b/smtpd/handler_test.go index 2a65d14..e541f4e 100644 --- a/smtpd/handler_test.go +++ b/smtpd/handler_test.go @@ -31,10 +31,8 @@ func TestGreetState(t *testing.T) { server, logbuf, teardown := setupSMTPServer(mds) defer teardown() - var script []scriptStep - // Test out some mangled HELOs - script = []scriptStep{ + script := []scriptStep{ {"HELO", 501}, {"EHLO", 501}, {"HELLO", 500}, @@ -90,10 +88,8 @@ func TestReadyState(t *testing.T) { server, logbuf, teardown := setupSMTPServer(mds) defer teardown() - var script []scriptStep - // Test out some mangled READY commands - script = []scriptStep{ + script := []scriptStep{ {"HELO localhost", 250}, {"FOOB", 500}, {"HELO", 503}, @@ -165,10 +161,8 @@ func TestMailState(t *testing.T) { server, logbuf, teardown := setupSMTPServer(mds) defer teardown() - var script []scriptStep - // Test out some mangled READY commands - script = []scriptStep{ + script := []scriptStep{ {"HELO localhost", 250}, {"MAIL FROM:", 250}, {"FOOB", 500}, diff --git a/smtpd/listener.go b/smtpd/listener.go index 6b4634e..60953b6 100644 --- a/smtpd/listener.go +++ b/smtpd/listener.go @@ -131,10 +131,8 @@ func (s *Server) Start(ctx context.Context) { go s.serve(ctx) // Wait for shutdown - select { - case <-ctx.Done(): - log.Tracef("SMTP shutdown requested, connections will be drained") - } + <-ctx.Done() + log.Tracef("SMTP shutdown requested, connections will be drained") // Closing the listener will cause the serve() go routine to exit if err := s.listener.Close(); err != nil { @@ -186,7 +184,7 @@ func (s *Server) serve(ctx context.Context) { func (s *Server) emergencyShutdown() { // Shutdown Inbucket select { - case _ = <-s.globalShutdown: + case <-s.globalShutdown: default: close(s.globalShutdown) } diff --git a/smtpd/utils.go b/smtpd/utils.go index bd57e3c..9b3eb17 100644 --- a/smtpd/utils.go +++ b/smtpd/utils.go @@ -130,15 +130,24 @@ LOOP: switch { case ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'): // Letters are OK - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false case '0' <= c && c <= '9': // Numbers are OK - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false case bytes.IndexByte([]byte("!#$%&'*+-/=?^_`{|}~"), c) >= 0: // These specials can be used unquoted - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false case c == '.': // A single period is OK @@ -146,13 +155,19 @@ LOOP: // Sequence of periods is not permitted return "", "", fmt.Errorf("Sequence of periods is not permitted") } - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false case c == '\\': inCharQuote = true case c == '"': if inCharQuote { - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false } else if inStringQuote { inStringQuote = false @@ -165,7 +180,10 @@ LOOP: } case c == '@': if inCharQuote || inStringQuote { - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false } else { // End of local-part @@ -182,7 +200,10 @@ LOOP: return "", "", fmt.Errorf("Characters outside of US-ASCII range not permitted") default: if inCharQuote || inStringQuote { - _ = buf.WriteByte(c) + err = buf.WriteByte(c) + if err != nil { + return + } inCharQuote = false } else { return "", "", fmt.Errorf("Character %q must be quoted", c)