diff --git a/.goxc.json b/.goxc.json index 132e5c5..e342a9c 100644 --- a/.goxc.json +++ b/.goxc.json @@ -8,5 +8,11 @@ "ResourcesInclude": "README*,LICENSE*,inbucket.bat,etc,themes", "PackageVersion": "1.1.0", "PrereleaseInfo": "alpha", - "ConfigVersion": "0.9" -} + "ConfigVersion": "0.9", + "BuildSettings": { + "LdFlagsXVars": { + "TimeNow": "main.BUILDDATE", + "Version": "main.VERSION" + } + } +} \ No newline at end of file diff --git a/.travis.yml b/.travis.yml index d0598d9..5731319 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,5 @@ language: go go: - - 1.5 + - 1.5.3 - tip - -install: - - go get -v ./... - - go get github.com/stretchr/testify diff --git a/config/config.go b/config/config.go index 280bd38..9cb4f4a 100644 --- a/config/config.go +++ b/config/config.go @@ -10,11 +10,11 @@ import ( "github.com/robfig/config" ) -// SmtpConfig houses the SMTP server configuration - not using pointers -// so that I can pass around copies of the object safely. -type SmtpConfig struct { - Ip4address net.IP - Ip4port int +// SMTPConfig contains the SMTP server configuration - not using pointers +// so that we can pass around copies of the object safely. +type SMTPConfig struct { + IP4address net.IP + IP4port int Domain string DomainNoStore string MaxRecipients int @@ -23,22 +23,25 @@ type SmtpConfig struct { StoreMessages bool } -type Pop3Config struct { - Ip4address net.IP - Ip4port int +// POP3Config contains the POP3 server configuration +type POP3Config struct { + IP4address net.IP + IP4port int Domain string MaxIdleSeconds int } +// WebConfig contains the HTTP server configuration type WebConfig struct { - Ip4address net.IP - Ip4port int + IP4address net.IP + IP4port int TemplateDir string TemplateCache bool PublicDir string GreetingFile string } +// DataStoreConfig contains the mail store configuration type DataStoreConfig struct { Path string RetentionMinutes int @@ -47,27 +50,29 @@ type DataStoreConfig struct { } var ( - // Build info, set by main - VERSION = "" - BUILD_DATE = "" + // Version of this build, set by main + Version = "" - // Global goconfig object + // BuildDate for this build, set by main + BuildDate = "" + + // Config is our global robfig/config object Config *config.Config // Parsed specific configs - smtpConfig *SmtpConfig - pop3Config *Pop3Config + smtpConfig *SMTPConfig + pop3Config *POP3Config webConfig *WebConfig dataStoreConfig *DataStoreConfig ) -// GetSmtpConfig returns a copy of the SmtpConfig object -func GetSmtpConfig() SmtpConfig { +// GetSMTPConfig returns a copy of the SmtpConfig object +func GetSMTPConfig() SMTPConfig { return *smtpConfig } -// GetPop3Config returns a copy of the Pop3Config object -func GetPop3Config() Pop3Config { +// GetPOP3Config returns a copy of the Pop3Config object +func GetPOP3Config() POP3Config { return *pop3Config } @@ -138,11 +143,11 @@ func LoadConfig(filename string) error { return fmt.Errorf("Failed to validate configuration") } - if err = parseSmtpConfig(); err != nil { + if err = parseSMTPConfig(); err != nil { return err } - if err = parsePop3Config(); err != nil { + if err = parsePOP3Config(); err != nil { return err } @@ -174,9 +179,9 @@ func parseLoggingConfig() error { return nil } -// parseSmtpConfig trying to catch config errors early -func parseSmtpConfig() error { - smtpConfig = new(SmtpConfig) +// parseSMTPConfig trying to catch config errors early +func parseSMTPConfig() error { + smtpConfig = new(SMTPConfig) section := "smtp" // Parse IP4 address only, error on IP6. @@ -193,10 +198,10 @@ func parseSmtpConfig() error { if addr == nil { return fmt.Errorf("Failed to parse [%v]%v: '%v' not IPv4!", section, option, err) } - smtpConfig.Ip4address = addr + smtpConfig.IP4address = addr option = "ip4.port" - smtpConfig.Ip4port, err = Config.Int(section, option) + smtpConfig.IP4port, err = Config.Int(section, option) if err != nil { return fmt.Errorf("Failed to parse [%v]%v: '%v'", section, option, err) } @@ -245,9 +250,9 @@ func parseSmtpConfig() error { return nil } -// parsePop3Config trying to catch config errors early -func parsePop3Config() error { - pop3Config = new(Pop3Config) +// parsePOP3Config trying to catch config errors early +func parsePOP3Config() error { + pop3Config = new(POP3Config) section := "pop3" // Parse IP4 address only, error on IP6. @@ -264,10 +269,10 @@ func parsePop3Config() error { if addr == nil { return fmt.Errorf("Failed to parse [%v]%v: '%v' not IPv4!", section, option, err) } - pop3Config.Ip4address = addr + pop3Config.IP4address = addr option = "ip4.port" - pop3Config.Ip4port, err = Config.Int(section, option) + pop3Config.IP4port, err = Config.Int(section, option) if err != nil { return fmt.Errorf("Failed to parse [%v]%v: '%v'", section, option, err) } @@ -307,10 +312,10 @@ func parseWebConfig() error { if addr == nil { return fmt.Errorf("Failed to parse [%v]%v: '%v' not IPv4!", section, option, err) } - webConfig.Ip4address = addr + webConfig.IP4address = addr option = "ip4.port" - webConfig.Ip4port, err = Config.Int(section, option) + webConfig.IP4port, err = Config.Int(section, option) if err != nil { return fmt.Errorf("Failed to parse [%v]%v: '%v'", section, option, err) } diff --git a/etc/mailbox-list.sh b/etc/mailbox-list.sh index fa0cb78..b0f20bb 100755 --- a/etc/mailbox-list.sh +++ b/etc/mailbox-list.sh @@ -1 +1,2 @@ +#!/bin/sh curl -i -H "Accept: application/json" --noproxy localhost "http://localhost:9000/mailbox/$1" diff --git a/etc/mailbox-purge.sh b/etc/mailbox-purge.sh index a8d3808..4d15336 100755 --- a/etc/mailbox-purge.sh +++ b/etc/mailbox-purge.sh @@ -1 +1,2 @@ +#!/bin/sh curl -i -H "Accept: application/json" --noproxy localhost -X DELETE http://localhost:9000/mailbox/$1 diff --git a/etc/message-body.sh b/etc/message-body.sh index ed8a0b4..3e593ed 100755 --- a/etc/message-body.sh +++ b/etc/message-body.sh @@ -1 +1,2 @@ +#!/bin/sh curl -i -H "Accept: application/json" --noproxy localhost http://localhost:9000/mailbox/$1/$2 diff --git a/etc/message-delete.sh b/etc/message-delete.sh index c3c8b86..c2fae1a 100755 --- a/etc/message-delete.sh +++ b/etc/message-delete.sh @@ -1 +1,2 @@ +#!/bin/sh curl -i -H "Accept: application/json" --noproxy localhost -X DELETE http://localhost:9000/mailbox/$1/$2 diff --git a/etc/message-source.sh b/etc/message-source.sh index 21714cc..c4f52ec 100755 --- a/etc/message-source.sh +++ b/etc/message-source.sh @@ -1 +1,2 @@ +#!/bin/sh curl -i -H "Accept: application/json" --noproxy localhost http://localhost:9000/mailbox/$1/$2/source diff --git a/inbucket.go b/inbucket.go index 9751729..d158712 100644 --- a/inbucket.go +++ b/inbucket.go @@ -1,6 +1,4 @@ -/* - This is the inbucket daemon launcher -*/ +// main is the inbucket daemon launcher package main import ( @@ -21,9 +19,11 @@ import ( ) var ( - // Build info, populated during linking by goxc - VERSION = "1.1.0.snapshot" - BUILD_DATE = "undefined" + // VERSION contains the build version number, populated during linking by goxc + VERSION = "1.1.0.snapshot" + + // BUILDDATE contains the build date, populated during linking by goxc + BUILDDATE = "undefined" // Command line flags help = flag.Bool("help", false, "Displays this help") @@ -42,8 +42,8 @@ var ( ) func main() { - config.VERSION = VERSION - config.BUILD_DATE = BUILD_DATE + config.Version = VERSION + config.BuildDate = BUILDDATE flag.Parse() if *help { @@ -84,27 +84,38 @@ func main() { } defer closeLogFile() - // close std* streams - os.Stdout.Close() - os.Stderr.Close() // Warning: this will hide panic() output - os.Stdin.Close() + // Close std* streams to prevent accidental output, they will be redirected to + // our logfile below + if err := os.Stdout.Close(); err != nil { + log.Errorf("Failed to close os.Stdout during log setup") + } + // Warning: this will hide panic() output + // TODO Replace with syscall.Dup2 per https://github.com/golang/go/issues/325 + if err := os.Stderr.Close(); err != nil { + log.Errorf("Failed to close os.Stderr during log setup") + } + if err := os.Stdin.Close(); err != nil { + log.Errorf("Failed to close os.Stdin during log setup") + } os.Stdout = logf os.Stderr = logf } } - log.LogInfo("Inbucket %v (%v) starting...", config.VERSION, config.BUILD_DATE) + log.Infof("Inbucket %v (%v) starting...", config.Version, config.BuildDate) // Write pidfile if requested // TODO: Probably supposed to remove pidfile during shutdown if *pidfile != "none" { pidf, err := os.Create(*pidfile) if err != nil { - log.LogError("Failed to create %v: %v", *pidfile, err) + log.Errorf("Failed to create %v: %v", *pidfile, err) os.Exit(1) } - defer pidf.Close() fmt.Fprintf(pidf, "%v\n", os.Getpid()) + if err := pidf.Close(); err != nil { + log.Errorf("Failed to close PID file %v: %v", *pidfile, err) + } } // Grab our datastore @@ -119,7 +130,7 @@ func main() { go pop3Server.Start() // Startup SMTP server, block until it exits - smtpServer = smtpd.NewSmtpServer(config.GetSmtpConfig(), ds) + smtpServer = smtpd.NewServer(config.GetSMTPConfig(), ds) smtpServer.Start() // Wait for active connections to finish @@ -136,14 +147,15 @@ func openLogFile() error { return fmt.Errorf("Failed to create %v: %v\n", *logfile, err) } golog.SetOutput(logf) - log.LogTrace("Opened new logfile") + log.Tracef("Opened new logfile") return nil } // closeLogFile closes the current logfile -func closeLogFile() error { - log.LogTrace("Closing logfile") - return logf.Close() +func closeLogFile() { + log.Tracef("Closing logfile") + // We are never in a situation where we can do anything about failing to close + _ = logf.Close() } // signalProcessor is a goroutine that handles OS signals @@ -154,21 +166,23 @@ func signalProcessor(c <-chan os.Signal) { case syscall.SIGHUP: // Rotate logs if configured if logf != nil { - log.LogInfo("Recieved SIGHUP, cycling logfile") + log.Infof("Recieved SIGHUP, cycling logfile") closeLogFile() - openLogFile() + // There is nothing we can do if the log open fails + // TODO We could panic, but that would be lame? + _ = openLogFile() } else { - log.LogInfo("Ignoring SIGHUP, logfile not configured") + log.Infof("Ignoring SIGHUP, logfile not configured") } case syscall.SIGTERM: // Initiate shutdown - log.LogInfo("Received SIGTERM, shutting down") + log.Infof("Received SIGTERM, shutting down") go timedExit() web.Stop() if smtpServer != nil { smtpServer.Stop() } else { - log.LogError("smtpServer was nil during shutdown") + log.Errorf("smtpServer was nil during shutdown") } } } @@ -177,7 +191,7 @@ func signalProcessor(c <-chan os.Signal) { // timedExit is called as a goroutine during shutdown, it will force an exit after 15 seconds func timedExit() { time.Sleep(15 * time.Second) - log.LogError("Inbucket clean shutdown timed out, forcing exit") + log.Errorf("Inbucket clean shutdown timed out, forcing exit") os.Exit(0) } diff --git a/log/logging.go b/log/logging.go index 9508e85..af11ee1 100644 --- a/log/logging.go +++ b/log/logging.go @@ -1,65 +1,71 @@ package log import ( - "log" + golog "log" "strings" ) -type LogLevel int +// Level is used to indicate the severity of a log entry +type Level int const ( - ERROR LogLevel = iota + // ERROR indicates a significant problem was encountered + ERROR Level = iota + // WARN indicates something that may be a problem WARN + // INFO indicates a purely informational log entry INFO + // TRACE entries are meant for development purposes only TRACE ) -var MaxLogLevel LogLevel = TRACE +// MaxLevel is the highest Level we will log (max TRACE, min ERROR) +var MaxLevel = TRACE -// SetLogLevel sets MaxLogLevel based on the provided string +// SetLogLevel sets MaxLevel based on the provided string func SetLogLevel(level string) (ok bool) { switch strings.ToUpper(level) { case "ERROR": - MaxLogLevel = ERROR + MaxLevel = ERROR case "WARN": - MaxLogLevel = WARN + MaxLevel = WARN case "INFO": - MaxLogLevel = INFO + MaxLevel = INFO case "TRACE": - MaxLogLevel = TRACE + MaxLevel = TRACE default: - LogError("Unknown log level requested: %v", level) + Errorf("Unknown log level requested: " + level) return false } return true } -// Error logs a message to the 'standard' Logger (always) -func LogError(msg string, args ...interface{}) { +// Errorf logs a message to the 'standard' Logger (always), accepts format strings +func Errorf(msg string, args ...interface{}) { msg = "[ERROR] " + msg - log.Printf(msg, args...) + golog.Printf(msg, args...) } -// Warn logs a message to the 'standard' Logger if MaxLogLevel is >= WARN -func LogWarn(msg string, args ...interface{}) { - if MaxLogLevel >= WARN { +// Warnf logs a message to the 'standard' Logger if MaxLevel is >= WARN, accepts format strings +func Warnf(msg string, args ...interface{}) { + if MaxLevel >= WARN { msg = "[WARN ] " + msg - log.Printf(msg, args...) + golog.Printf(msg, args...) } } -// Info logs a message to the 'standard' Logger if MaxLogLevel is >= INFO -func LogInfo(msg string, args ...interface{}) { - if MaxLogLevel >= INFO { +// Infof logs a message to the 'standard' Logger if MaxLevel is >= INFO, accepts format strings +func Infof(msg string, args ...interface{}) { + if MaxLevel >= INFO { msg = "[INFO ] " + msg - log.Printf(msg, args...) + golog.Printf(msg, args...) } } -// Trace logs a message to the 'standard' Logger if MaxLogLevel is >= TRACE -func LogTrace(msg string, args ...interface{}) { - if MaxLogLevel >= TRACE { +// Tracef logs a message to the 'standard' Logger if MaxLevel is >= TRACE, accepts format strings +func Tracef(msg string, args ...interface{}) { + if MaxLevel >= TRACE { msg = "[TRACE] " + msg - log.Printf(msg, args...) + golog.Printf(msg, args...) } } diff --git a/pop3d/handler.go b/pop3d/handler.go index 46ee2da..782cf8e 100644 --- a/pop3d/handler.go +++ b/pop3d/handler.go @@ -15,11 +15,15 @@ import ( "github.com/jhillyerd/inbucket/smtpd" ) +// State tracks the current mode of our POP3 state machine type State int const ( - AUTHORIZATION State = iota // The client must now identify and authenticate - TRANSACTION // Mailbox open, client may now issue commands + // AUTHORIZATION state: the client must now identify and authenticate + AUTHORIZATION State = iota + // TRANSACTION state: mailbox open, client may now issue commands + TRANSACTION + // QUIT state: client requests us to end session QUIT ) @@ -51,6 +55,7 @@ var commands = map[string]bool{ "CAPA": true, } +// Session defines an active POP3 session type Session struct { server *Server // Reference to the server we belong to id int // Session ID number @@ -66,6 +71,7 @@ type Session struct { msgCount int // Number of undeleted messages } +// NewSession creates a new POP3 session func NewSession(server *Server, id int, conn net.Conn) *Session { reader := bufio.NewReader(conn) host, _, _ := net.SplitHostPort(conn.RemoteAddr().String()) @@ -85,10 +91,12 @@ func (ses *Session) String() string { * 5. Goto 2 */ func (s *Server) startSession(id int, conn net.Conn) { - log.LogInfo("POP3 connection from %v, starting session <%v>", conn.RemoteAddr(), id) + log.Infof("POP3 connection from %v, starting session <%v>", conn.RemoteAddr(), id) //expConnectsCurrent.Add(1) defer func() { - conn.Close() + if err := conn.Close(); err != nil { + log.Errorf("Error closing POP3 connection for <%v>: %v", id, err) + } s.waitgroup.Done() //expConnectsCurrent.Add(-1) }() @@ -235,7 +243,7 @@ func (ses *Session) transactionHandler(cmd string, args []string) { var size int64 for i, msg := range ses.messages { if ses.retain[i] { - count += 1 + count++ size += msg.Size() } } @@ -306,12 +314,12 @@ func (ses *Session) transactionHandler(cmd string, args []string) { ses.send(fmt.Sprintf("-ERR You deleted message %v", msgNum)) return } - ses.send(fmt.Sprintf("+OK %v %v", msgNum, ses.messages[msgNum-1].Id())) + ses.send(fmt.Sprintf("+OK %v %v", msgNum, ses.messages[msgNum-1].ID())) } else { ses.send(fmt.Sprintf("+OK Listing %v messages", ses.msgCount)) for i, msg := range ses.messages { if ses.retain[i] { - ses.send(fmt.Sprintf("%v %v", i+1, msg.Id())) + ses.send(fmt.Sprintf("%v %v", i+1, msg.ID())) } } ses.send(".") @@ -340,7 +348,7 @@ func (ses *Session) transactionHandler(cmd string, args []string) { } if ses.retain[msgNum-1] { ses.retain[msgNum-1] = false - ses.msgCount -= 1 + ses.msgCount-- ses.send(fmt.Sprintf("+OK Deleted message %v", msgNum)) } else { ses.logWarn("Client tried to DELE an already deleted message") @@ -431,7 +439,12 @@ func (ses *Session) sendMessage(msg smtpd.Message) { ses.send("-ERR Failed to RETR that message, internal error") return } - defer reader.Close() + defer func() { + if err := reader.Close(); err != nil { + ses.logError("Failed to close message: %v", err) + } + }() + scanner := bufio.NewScanner(reader) for scanner.Scan() { line := scanner.Text() @@ -459,7 +472,12 @@ func (ses *Session) sendMessageTop(msg smtpd.Message, lineCount int) { ses.send("-ERR Failed to RETR that message, internal error") return } - defer reader.Close() + defer func() { + if err := reader.Close(); err != nil { + ses.logError("Failed to close message: %v", err) + } + }() + scanner := bufio.NewScanner(reader) inBody := false for scanner.Scan() { @@ -473,7 +491,7 @@ func (ses *Session) sendMessageTop(msg smtpd.Message, lineCount int) { if lineCount < 1 { break } else { - lineCount -= 1 + lineCount-- } } else { if line == "" { @@ -522,7 +540,9 @@ func (ses *Session) processDeletes() { for i, msg := range ses.messages { if !ses.retain[i] { ses.logTrace("Deleting %v", msg) - msg.Delete() + if err := msg.Delete(); err != nil { + ses.logWarn("Error deleting %v: %v", msg, err) + } } } } @@ -562,13 +582,17 @@ func (ses *Session) readByteLine(buf *bytes.Buffer) error { if err != nil { return err } - buf.Write(line) + if _, err = buf.Write(line); err != nil { + return err + } // Read the next byte looking for '\n' c, err := ses.reader.ReadByte() if err != nil { return err } - buf.WriteByte(c) + if err := buf.WriteByte(c); err != nil { + return err + } if c == '\n' { // We've reached the end of the line, return return nil @@ -612,21 +636,21 @@ func (ses *Session) ooSeq(cmd string) { // Session specific logging methods func (ses *Session) logTrace(msg string, args ...interface{}) { - log.LogTrace("POP3[%v]<%v> %v", ses.remoteHost, ses.id, fmt.Sprintf(msg, args...)) + log.Tracef("POP3[%v]<%v> %v", ses.remoteHost, ses.id, fmt.Sprintf(msg, args...)) } func (ses *Session) logInfo(msg string, args ...interface{}) { - log.LogInfo("POP3[%v]<%v> %v", ses.remoteHost, ses.id, fmt.Sprintf(msg, args...)) + log.Infof("POP3[%v]<%v> %v", ses.remoteHost, ses.id, fmt.Sprintf(msg, args...)) } func (ses *Session) logWarn(msg string, args ...interface{}) { // Update metrics //expWarnsTotal.Add(1) - log.LogWarn("POP3[%v]<%v> %v", ses.remoteHost, ses.id, fmt.Sprintf(msg, args...)) + log.Warnf("POP3[%v]<%v> %v", ses.remoteHost, ses.id, fmt.Sprintf(msg, args...)) } func (ses *Session) logError(msg string, args ...interface{}) { // Update metrics //expErrorsTotal.Add(1) - log.LogError("POP3[%v]<%v> %v", ses.remoteHost, ses.id, fmt.Sprintf(msg, args...)) + log.Errorf("POP3[%v]<%v> %v", ses.remoteHost, ses.id, fmt.Sprintf(msg, args...)) } diff --git a/pop3d/listener.go b/pop3d/listener.go index ef482c8..94c63f0 100644 --- a/pop3d/listener.go +++ b/pop3d/listener.go @@ -11,7 +11,7 @@ import ( "github.com/jhillyerd/inbucket/smtpd" ) -// Real server code starts here +// Server defines an instance of our POP3 server type Server struct { domain string maxIdleSeconds int @@ -21,30 +21,30 @@ type Server struct { waitgroup *sync.WaitGroup } -// Init a new Server object +// New creates a new Server struct func New() *Server { // TODO is two filestores better/worse than sharing w/ smtpd? ds := smtpd.DefaultFileDataStore() - cfg := config.GetPop3Config() + cfg := config.GetPOP3Config() return &Server{domain: cfg.Domain, dataStore: ds, maxIdleSeconds: cfg.MaxIdleSeconds, waitgroup: new(sync.WaitGroup)} } -// Main listener loop +// Start the server and listen for connections func (s *Server) Start() { - cfg := config.GetPop3Config() + cfg := config.GetPOP3Config() addr, err := net.ResolveTCPAddr("tcp4", fmt.Sprintf("%v:%v", - cfg.Ip4address, cfg.Ip4port)) + cfg.IP4address, cfg.IP4port)) if err != nil { - log.LogError("POP3 Failed to build tcp4 address: %v", err) + log.Errorf("POP3 Failed to build tcp4 address: %v", err) // TODO More graceful early-shutdown procedure panic(err) } - log.LogInfo("POP3 listening on TCP4 %v", addr) + log.Infof("POP3 listening on TCP4 %v", addr) s.listener, err = net.ListenTCP("tcp4", addr) if err != nil { - log.LogError("POP3 failed to start tcp4 listener: %v", err) + log.Errorf("POP3 failed to start tcp4 listener: %v", err) // TODO More graceful early-shutdown procedure panic(err) } @@ -63,12 +63,12 @@ func (s *Server) Start() { if max := 1 * time.Second; tempDelay > max { tempDelay = max } - log.LogError("POP3 accept error: %v; retrying in %v", err, tempDelay) + log.Errorf("POP3 accept error: %v; retrying in %v", err, tempDelay) time.Sleep(tempDelay) continue } else { if s.shutdown { - log.LogTrace("POP3 listener shutting down on request") + log.Tracef("POP3 listener shutting down on request") return } // TODO Implement a max error counter before shutdown? @@ -85,13 +85,15 @@ func (s *Server) Start() { // Stop requests the POP3 server closes it's listener func (s *Server) Stop() { - log.LogTrace("POP3 shutdown requested, connections will be drained") + log.Tracef("POP3 shutdown requested, connections will be drained") s.shutdown = true - s.listener.Close() + if err := s.listener.Close(); err != nil { + log.Errorf("Error closing POP3 listener: %v", err) + } } // Drain causes the caller to block until all active POP3 sessions have finished func (s *Server) Drain() { s.waitgroup.Wait() - log.LogTrace("POP3 connections drained") + log.Tracef("POP3 connections drained") } diff --git a/smtpd/datastore.go b/smtpd/datastore.go index 1bde5a2..d8dcd9b 100644 --- a/smtpd/datastore.go +++ b/smtpd/datastore.go @@ -9,13 +9,16 @@ import ( "github.com/jhillyerd/go.enmime" ) +// ErrNotExist indicates the requested message does not exist var ErrNotExist = errors.New("Message does not exist") +// DataStore is an interface to get Mailboxes stored in Inbucket type DataStore interface { MailboxFor(emailAddress string) (Mailbox, error) AllMailboxes() ([]Mailbox, error) } +// Mailbox is an interface to get and manipulate messages in a DataStore type Mailbox interface { GetMessages() ([]Message, error) GetMessage(id string) (Message, error) @@ -24,8 +27,9 @@ type Mailbox interface { String() string } +// Message is an interface for a single message in a Mailbox type Message interface { - Id() string + ID() string From() string Date() time.Time Subject() string diff --git a/smtpd/filestore.go b/smtpd/filestore.go index e081102..2fb9c5d 100644 --- a/smtpd/filestore.go +++ b/smtpd/filestore.go @@ -19,16 +19,23 @@ import ( ) // Name of index file in each mailbox -const INDEX_FILE = "index.gob" +const indexFileName = "index.gob" -// We lock this when reading/writing an index file, this is a bottleneck because -// it's a single lock even if we have a million index files -var indexLock = new(sync.RWMutex) +var ( + // indexLock is locked while reading/writing an index file + // + // NOTE: This is a bottleneck because it's a single lock even if we have a + // million index files + indexLock = new(sync.RWMutex) -var ErrNotWritable = errors.New("Message not writable") + // TODO Consider moving this to the Message interface + errNotWritable = errors.New("Message not writable") -// Global because we only want one regardless of the number of DataStore objects -var countChannel = make(chan int, 10) + // countChannel is filled with a sequential numbers (0000..9999), which are + // used by generateID() to generate unique message IDs. It's global + // because we only want one regardless of the number of DataStore objects + countChannel = make(chan int, 10) +) func init() { // Start generator @@ -42,8 +49,8 @@ func countGenerator(c chan int) { } } -// A DataStore is the root of the mail storage hiearchy. It provides access to -// Mailbox objects +// FileDataStore implements DataStore aand is the root of the mail storage +// hiearchy. It provides access to Mailbox objects type FileDataStore struct { path string mailPath string @@ -54,13 +61,15 @@ type FileDataStore struct { func NewFileDataStore(cfg config.DataStoreConfig) DataStore { path := cfg.Path if path == "" { - log.LogError("No value configured for datastore path") + log.Errorf("No value configured for datastore path") return nil } mailPath := filepath.Join(path, "mail") if _, err := os.Stat(mailPath); err != nil { // Mail datastore does not yet exist - os.MkdirAll(mailPath, 0770) + if err = os.MkdirAll(mailPath, 0770); err != nil { + log.Errorf("Error creating dir %q: %v", mailPath, err) + } } return &FileDataStore{path: path, mailPath: mailPath, messageCap: cfg.MailboxMsgCap} } @@ -72,7 +81,7 @@ func DefaultFileDataStore() DataStore { return NewFileDataStore(cfg) } -// Retrieves the Mailbox object for a specified email address, if the mailbox +// MailboxFor retrieves the Mailbox object for a specified email address, if the mailbox // does not exist, it will attempt to create it. func (ds *FileDataStore) MailboxFor(emailAddress string) (Mailbox, error) { name, err := ParseMailboxName(emailAddress) @@ -83,7 +92,7 @@ func (ds *FileDataStore) MailboxFor(emailAddress string) (Mailbox, error) { s1 := dir[0:3] s2 := dir[0:6] path := filepath.Join(ds.mailPath, s1, s2, dir) - indexPath := filepath.Join(path, INDEX_FILE) + indexPath := filepath.Join(path, indexFileName) return &FileMailbox{store: ds, name: name, dirName: dir, path: path, indexPath: indexPath}, nil @@ -117,7 +126,7 @@ func (ds *FileDataStore) AllMailboxes() ([]Mailbox, error) { if inf3.IsDir() { mbdir := inf3.Name() mbpath := filepath.Join(ds.mailPath, l1, l2, mbdir) - idx := filepath.Join(mbpath, INDEX_FILE) + idx := filepath.Join(mbpath, indexFileName) mb := &FileMailbox{store: ds, dirName: mbdir, path: mbpath, indexPath: idx} mailboxes = append(mailboxes, mb) @@ -131,8 +140,8 @@ func (ds *FileDataStore) AllMailboxes() ([]Mailbox, error) { return mailboxes, nil } -// A Mailbox manages the mail for a specific user and correlates to a particular -// directory on disk. +// FileMailbox implements Mailbox, manages the mail for a specific user and +// correlates to a particular directory on disk. type FileMailbox struct { store *FileDataStore name string @@ -180,7 +189,7 @@ func (mb *FileMailbox) GetMessage(id string) (Message, error) { return nil, ErrNotExist } -// Delete all messages in this mailbox +// Purge deletes all messages in this mailbox func (mb *FileMailbox) Purge() error { mb.messages = mb.messages[:0] return mb.writeIndex() @@ -196,7 +205,7 @@ func (mb *FileMailbox) readIndex() error { // Check if index exists if _, err := os.Stat(mb.indexPath); err != nil { // Does not exist, but that's not an error in our world - log.LogTrace("Index %v does not exist (yet)", mb.indexPath) + log.Tracef("Index %v does not exist (yet)", mb.indexPath) mb.indexLoaded = true return nil } @@ -204,7 +213,11 @@ func (mb *FileMailbox) readIndex() error { if err != nil { return err } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + log.Errorf("Failed to close %q: %v", mb.indexPath, err) + } + }() // Decode gob data dec := gob.NewDecoder(bufio.NewReader(file)) @@ -219,7 +232,7 @@ func (mb *FileMailbox) readIndex() error { return fmt.Errorf("While decoding message: %v", err) } msg.mailbox = mb - log.LogTrace("Found: %v", msg) + log.Tracef("Found: %v", msg) mb.messages = append(mb.messages, msg) } @@ -231,7 +244,7 @@ func (mb *FileMailbox) readIndex() error { func (mb *FileMailbox) createDir() error { if _, err := os.Stat(mb.path); err != nil { if err := os.MkdirAll(mb.path, 0770); err != nil { - log.LogError("Failed to create directory %v, %v", mb.path, err) + log.Errorf("Failed to create directory %v, %v", mb.path, err) return err } } @@ -253,7 +266,11 @@ func (mb *FileMailbox) writeIndex() error { if err != nil { return err } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + log.Errorf("Failed to close %q: %v", mb.indexPath, err) + } + }() writer := bufio.NewWriter(file) // Write each message and then flush @@ -264,18 +281,20 @@ func (mb *FileMailbox) writeIndex() error { return err } } - writer.Flush() + if err := writer.Flush(); err != nil { + return err + } } else { // No messages, delete index+maildir - log.LogTrace("Removing mailbox %v", mb.path) + log.Tracef("Removing mailbox %v", mb.path) return os.RemoveAll(mb.path) } return nil } -// Message contains a little bit of data about a particular email message, and -// methods to retrieve the rest of it from disk. +// FileMessage implements Message and contains a little bit of data about a +// particular email message, and methods to retrieve the rest of it from disk. type FileMessage struct { mailbox *FileMailbox // Stored in GOB @@ -290,7 +309,7 @@ type FileMessage struct { writer *bufio.Writer } -// NewMessage creates a new Message object and sets the Date and Id fields. +// NewMessage creates a new FileMessage object and sets the Date and Id fields. // It will also delete messages over messageCap if configured. func (mb *FileMailbox) NewMessage() (Message, error) { // Load index @@ -303,7 +322,7 @@ func (mb *FileMailbox) NewMessage() (Message, error) { // Delete old messages over messageCap if mb.store.messageCap > 0 { for len(mb.messages) >= mb.store.messageCap { - log.LogInfo("Mailbox %q over configured message cap", mb.name) + log.Infof("Mailbox %q over configured message cap", mb.name) if err := mb.messages[0].Delete(); err != nil { return nil, err } @@ -311,30 +330,37 @@ func (mb *FileMailbox) NewMessage() (Message, error) { } date := time.Now() - id := generateId(date) + id := generateID(date) return &FileMessage{mailbox: mb, Fid: id, Fdate: date, writable: true}, nil } -func (m *FileMessage) Id() string { +// ID gets the ID of the Message +func (m *FileMessage) ID() string { return m.Fid } +// Date returns the date of the Message +// TODO Is this the create timestamp, or from the Date header? func (m *FileMessage) Date() time.Time { return m.Fdate } +// From returns the value of the Message From header func (m *FileMessage) From() string { return m.Ffrom } +// Subject returns the value of the Message Subject header func (m *FileMessage) Subject() string { return m.Fsubject } +// String returns a string in the form: "Subject()" from From() func (m *FileMessage) String() string { return fmt.Sprintf("\"%v\" from %v", m.Fsubject, m.Ffrom) } +// Size returns the size of the Message on disk in bytes func (m *FileMessage) Size() int64 { return m.Fsize } @@ -349,10 +375,14 @@ func (m *FileMessage) ReadHeader() (msg *mail.Message, err error) { if err != nil { return nil, err } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + log.Errorf("Failed to close %q: %v", m.rawPath(), err) + } + }() + reader := bufio.NewReader(file) - msg, err = mail.ReadMessage(reader) - return msg, err + return mail.ReadMessage(reader) } // ReadBody opens the .raw portion of a Message and returns a MIMEBody object @@ -361,7 +391,12 @@ func (m *FileMessage) ReadBody() (body *enmime.MIMEBody, err error) { if err != nil { return nil, err } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + log.Errorf("Failed to close %q: %v", m.rawPath(), err) + } + }() + reader := bufio.NewReader(file) msg, err := mail.ReadMessage(reader) if err != nil { @@ -371,7 +406,7 @@ func (m *FileMessage) ReadBody() (body *enmime.MIMEBody, err error) { if err != nil { return nil, err } - return mime, err + return mime, nil } // RawReader opens the .raw portion of a Message as an io.ReadCloser @@ -386,10 +421,15 @@ func (m *FileMessage) RawReader() (reader io.ReadCloser, err error) { // ReadRaw opens the .raw portion of a Message and returns it as a string func (m *FileMessage) ReadRaw() (raw *string, err error) { reader, err := m.RawReader() - defer reader.Close() if err != nil { return nil, err } + defer func() { + if err := reader.Close(); err != nil { + log.Errorf("Failed to close %q: %v", m.rawPath(), err) + } + }() + bodyBytes, err := ioutil.ReadAll(bufio.NewReader(reader)) if err != nil { return nil, err @@ -403,7 +443,7 @@ func (m *FileMessage) ReadRaw() (raw *string, err error) { func (m *FileMessage) Append(data []byte) error { // Prevent Appending to a pre-existing Message if !m.writable { - return ErrNotWritable + return errNotWritable } // Open file for writing if we haven't yet if m.writer == nil { @@ -466,7 +506,8 @@ func (m *FileMessage) Close() error { return m.mailbox.writeIndex() } -// Delete this Message from disk by removing both the gob and raw files +// Delete this Message from disk by removing it from the index and deleting the +// raw files. func (m *FileMessage) Delete() error { messages := m.mailbox.messages for i, mm := range messages { @@ -476,16 +517,18 @@ func (m *FileMessage) Delete() error { break } } - m.mailbox.writeIndex() + if err := m.mailbox.writeIndex(); err != nil { + return err + } if len(m.mailbox.messages) == 0 { - // This was the last message, writeIndex() has removed the entire - // directory + // This was the last message, thus writeIndex() has removed the entire + // directory; we don't need to delete the raw file. return nil } // There are still messages in the index - log.LogTrace("Deleting %v", m.rawPath()) + log.Tracef("Deleting %v", m.rawPath()) return os.Remove(m.rawPath()) } @@ -498,6 +541,6 @@ func generatePrefix(date time.Time) string { // generateId adds a 4-digit unique number onto the end of the string // returned by generatePrefix() -func generateId(date time.Time) string { +func generateID(date time.Time) string { return generatePrefix(date) + "-" + fmt.Sprintf("%04d", <-countChannel) } diff --git a/smtpd/filestore_test.go b/smtpd/filestore_test.go index 7d02e06..e783e17 100644 --- a/smtpd/filestore_test.go +++ b/smtpd/filestore_test.go @@ -95,7 +95,7 @@ func TestFSDirStructure(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -122,7 +122,7 @@ func TestFSAllMailboxes(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -172,7 +172,7 @@ func TestFSDeliverMany(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -201,8 +201,8 @@ func TestFSDelete(t *testing.T) { len(subjects), len(msgs)) // Delete a couple messages - msgs[1].Delete() - msgs[3].Delete() + _ = msgs[1].Delete() + _ = msgs[3].Delete() // Confirm deletion mb, err = ds.MailboxFor(mbName) @@ -246,7 +246,7 @@ func TestFSDelete(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -294,7 +294,7 @@ func TestFSPurge(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -332,7 +332,7 @@ func TestFSSize(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -360,7 +360,7 @@ func TestFSMissing(t *testing.T) { msg, err := mb.GetMessage(sentIds[1]) assert.Nil(t, err) fmsg := msg.(*FileMessage) - os.Remove(fmsg.rawPath()) + _ = os.Remove(fmsg.rawPath()) msg, err = mb.GetMessage(sentIds[1]) assert.Nil(t, err) @@ -374,7 +374,7 @@ func TestFSMissing(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -419,7 +419,7 @@ func TestFSMessageCap(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -454,7 +454,7 @@ func TestFSNoMessageCap(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -490,7 +490,7 @@ func deliverMessage(ds *FileDataStore, mbName string, subject string, panic(err) } // Create message object - id = generateId(date) + id = generateID(date) msg, err := mb.NewMessage() if err != nil { panic(err) @@ -498,7 +498,9 @@ func deliverMessage(ds *FileDataStore, mbName string, subject string, fmsg := msg.(*FileMessage) fmsg.Fdate = date fmsg.Fid = id - msg.Append(testMsg) + if err = msg.Append(testMsg); err != nil { + panic(err) + } if err = msg.Close(); err != nil { panic(err) } diff --git a/smtpd/handler.go b/smtpd/handler.go index 4b0558e..7ade826 100644 --- a/smtpd/handler.go +++ b/smtpd/handler.go @@ -15,17 +15,23 @@ import ( "github.com/jhillyerd/inbucket/log" ) +// State tracks the current mode of our SMTP state machine type State int const ( - GREET State = iota // Waiting for HELO - READY // Got HELO, waiting for MAIL - MAIL // Got MAIL, accepting RCPTs - DATA // Got DATA, waiting for "." - QUIT // Close session + // GREET State: Waiting for HELO + GREET State = iota + // READY State: Got HELO, waiting for MAIL + READY + // MAIL State: Got MAIL, accepting RCPTs + MAIL + // DATA State: Got DATA, waiting for "." + DATA + // QUIT State: Client requested end of session + QUIT ) -const STAMP_FMT = "Mon, 02 Jan 2006 15:04:05 -0700 (MST)" +const timeStampFormat = "Mon, 02 Jan 2006 15:04:05 -0700 (MST)" func (s State) String() string { switch s { @@ -61,6 +67,7 @@ var commands = map[string]bool{ "TURN": true, } +// Session holds the state of an SMTP session type Session struct { server *Server id int @@ -74,6 +81,7 @@ type Session struct { recipients *list.List } +// NewSession creates a new Session for the given connection func NewSession(server *Server, id int, conn net.Conn) *Session { reader := bufio.NewReader(conn) host, _, _ := net.SplitHostPort(conn.RemoteAddr().String()) @@ -92,10 +100,12 @@ func (ss *Session) String() string { * 5. Goto 2 */ func (s *Server) startSession(id int, conn net.Conn) { - log.LogInfo("SMTP Connection from %v, starting session <%v>", conn.RemoteAddr(), id) + log.Infof("SMTP Connection from %v, starting session <%v>", conn.RemoteAddr(), id) expConnectsCurrent.Add(1) defer func() { - conn.Close() + if err := conn.Close(); err != nil { + log.Errorf("Error closing connection for <%v>: %v", id, err) + } s.waitgroup.Done() expConnectsCurrent.Add(-1) }() @@ -321,11 +331,10 @@ func (ss *Session) mailHandler(cmd string, arg string) { // We have recipients, go to accept data ss.enterState(DATA) return - } else { - // DATA out of sequence - ss.ooSeq(cmd) - return } + // DATA out of sequence + ss.ooSeq(cmd) + return } ss.ooSeq(cmd) } @@ -333,7 +342,7 @@ func (ss *Session) mailHandler(cmd string, arg string) { // DATA func (ss *Session) dataHandler() { // Timestamp for Received header - stamp := time.Now().Format(STAMP_FMT) + stamp := time.Now().Format(timeStampFormat) // Get a Mailbox and a new Message for each recipient mailboxes := make([]Mailbox, ss.recipients.Len()) messages := make([]Message, ss.recipients.Len()) @@ -369,9 +378,14 @@ func (ss *Session) dataHandler() { // Generate Received header recd := fmt.Sprintf("Received: from %s ([%s]) by %s\r\n for <%s>; %s\r\n", ss.remoteDomain, ss.remoteHost, ss.server.domain, recip, stamp) - messages[i].Append([]byte(recd)) + if err := messages[i].Append([]byte(recd)); err != nil { + ss.logError("Failed to write received header for %q: %s", local, err) + ss.send(fmt.Sprintf("451 Failed to create message for %v", local)) + ss.reset() + return + } } else { - log.LogTrace("Not storing message for %q", recip) + log.Tracef("Not storing message for %q", recip) } i++ } @@ -482,13 +496,17 @@ func (ss *Session) readByteLine(buf *bytes.Buffer) error { if err != nil { return err } - buf.Write(line) + if _, err = buf.Write(line); err != nil { + return err + } // Read the next byte looking for '\n' c, err := ss.reader.ReadByte() if err != nil { return err } - buf.WriteByte(c) + if err = buf.WriteByte(c); err != nil { + return err + } if c == '\n' { // We've reached the end of the line, return return nil @@ -570,21 +588,21 @@ func (ss *Session) ooSeq(cmd string) { // Session specific logging methods func (ss *Session) logTrace(msg string, args ...interface{}) { - log.LogTrace("SMTP[%v]<%v> %v", ss.remoteHost, ss.id, fmt.Sprintf(msg, args...)) + log.Tracef("SMTP[%v]<%v> %v", ss.remoteHost, ss.id, fmt.Sprintf(msg, args...)) } func (ss *Session) logInfo(msg string, args ...interface{}) { - log.LogInfo("SMTP[%v]<%v> %v", ss.remoteHost, ss.id, fmt.Sprintf(msg, args...)) + log.Infof("SMTP[%v]<%v> %v", ss.remoteHost, ss.id, fmt.Sprintf(msg, args...)) } func (ss *Session) logWarn(msg string, args ...interface{}) { // Update metrics expWarnsTotal.Add(1) - log.LogWarn("SMTP[%v]<%v> %v", ss.remoteHost, ss.id, fmt.Sprintf(msg, args...)) + log.Warnf("SMTP[%v]<%v> %v", ss.remoteHost, ss.id, fmt.Sprintf(msg, args...)) } func (ss *Session) logError(msg string, args ...interface{}) { // Update metrics expErrorsTotal.Add(1) - log.LogError("SMTP[%v]<%v> %v", ss.remoteHost, ss.id, fmt.Sprintf(msg, args...)) + log.Errorf("SMTP[%v]<%v> %v", ss.remoteHost, ss.id, fmt.Sprintf(msg, args...)) } diff --git a/smtpd/handler_test.go b/smtpd/handler_test.go index 9434aae..f762027 100644 --- a/smtpd/handler_test.go +++ b/smtpd/handler_test.go @@ -6,7 +6,6 @@ import ( "io" "github.com/jhillyerd/inbucket/config" - //"io/ioutil" "log" "net" "net/textproto" @@ -27,8 +26,8 @@ func TestGreetState(t *testing.T) { mb1 := &MockMailbox{} mds.On("MailboxFor").Return(mb1, nil) - server, logbuf := setupSmtpServer(mds) - defer teardownSmtpServer(server) + server, logbuf := setupSMTPServer(mds) + defer teardownSMTPServer(server) var script []scriptStep @@ -77,7 +76,7 @@ func TestGreetState(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -88,8 +87,8 @@ func TestReadyState(t *testing.T) { mb1 := &MockMailbox{} mds.On("MailboxFor").Return(mb1, nil) - server, logbuf := setupSmtpServer(mds) - defer teardownSmtpServer(server) + server, logbuf := setupSMTPServer(mds) + defer teardownSMTPServer(server) var script []scriptStep @@ -142,7 +141,7 @@ func TestReadyState(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -156,8 +155,8 @@ func TestMailState(t *testing.T) { mb1.On("NewMessage").Return(msg1, nil) msg1.On("Close").Return(nil) - server, logbuf := setupSmtpServer(mds) - defer teardownSmtpServer(server) + server, logbuf := setupSMTPServer(mds) + defer teardownSMTPServer(server) var script []scriptStep @@ -252,7 +251,7 @@ func TestMailState(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -266,11 +265,11 @@ func TestDataState(t *testing.T) { mb1.On("NewMessage").Return(msg1, nil) msg1.On("Close").Return(nil) - server, logbuf := setupSmtpServer(mds) - defer teardownSmtpServer(server) + server, logbuf := setupSMTPServer(mds) + defer teardownSMTPServer(server) var script []scriptStep - pipe := setupSmtpSession(server) + pipe := setupSMTPSession(server) c := textproto.NewConn(pipe) // Get us into DATA state @@ -294,8 +293,8 @@ Subject: test Hi! ` dw := c.DotWriter() - io.WriteString(dw, body) - dw.Close() + _, _ = io.WriteString(dw, body) + _ = dw.Close() if code, _, err := c.ReadCodeLine(250); err != nil { t.Errorf("Expected a 250 greeting, got %v", code) } @@ -304,13 +303,13 @@ Hi! // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } // playSession creates a new session, reads the greeting and then plays the script func playSession(t *testing.T, server *Server, script []scriptStep) error { - pipe := setupSmtpSession(server) + pipe := setupSMTPSession(server) c := textproto.NewConn(pipe) if code, _, err := c.ReadCodeLine(220); err != nil { @@ -319,8 +318,10 @@ func playSession(t *testing.T, server *Server, script []scriptStep) error { err := playScriptAgainst(t, c, script) - c.Cmd("QUIT") - c.ReadCodeLine(221) + // Not all tests leave the session in a clean state, so the following two + // calls can fail + _, _ = c.Cmd("QUIT") + _, _, _ = c.ReadCodeLine(221) return err } @@ -358,11 +359,11 @@ func (m *mockConn) SetDeadline(t time.Time) error { return nil } func (m *mockConn) SetReadDeadline(t time.Time) error { return nil } func (m *mockConn) SetWriteDeadline(t time.Time) error { return nil } -func setupSmtpServer(ds DataStore) (*Server, *bytes.Buffer) { +func setupSMTPServer(ds DataStore) (*Server, *bytes.Buffer) { // Test Server Config - cfg := config.SmtpConfig{ - Ip4address: net.IPv4(127, 0, 0, 1), - Ip4port: 2500, + cfg := config.SMTPConfig{ + IP4address: net.IPv4(127, 0, 0, 1), + IP4port: 2500, Domain: "inbucket.local", DomainNoStore: "bitbucket.local", MaxRecipients: 5, @@ -376,12 +377,12 @@ func setupSmtpServer(ds DataStore) (*Server, *bytes.Buffer) { log.SetOutput(buf) // Create a server, don't start it - return NewSmtpServer(cfg, ds), buf + return NewServer(cfg, ds), buf } var sessionNum int -func setupSmtpSession(server *Server) net.Conn { +func setupSMTPSession(server *Server) net.Conn { // Pair of pipes to communicate serverConn, clientConn := net.Pipe() // Start the session @@ -392,6 +393,6 @@ func setupSmtpSession(server *Server) net.Conn { return clientConn } -func teardownSmtpServer(server *Server) { +func teardownSMTPServer(server *Server) { //log.SetOutput(os.Stderr) } diff --git a/smtpd/listener.go b/smtpd/listener.go index 7072211..a653502 100644 --- a/smtpd/listener.go +++ b/smtpd/listener.go @@ -13,7 +13,7 @@ import ( "github.com/jhillyerd/inbucket/log" ) -// Real server code starts here +// Server holds the configuration and state of our SMTP server type Server struct { domain string domainNoStore string @@ -46,37 +46,37 @@ var expConnectsHist = new(expvar.String) var expErrorsHist = new(expvar.String) var expWarnsHist = new(expvar.String) -// Init a new Server object -func NewSmtpServer(cfg config.SmtpConfig, ds DataStore) *Server { +// NewServer creates a new Server instance with the specificed config +func NewServer(cfg config.SMTPConfig, ds DataStore) *Server { return &Server{dataStore: ds, domain: cfg.Domain, maxRecips: cfg.MaxRecipients, maxIdleSeconds: cfg.MaxIdleSeconds, maxMessageBytes: cfg.MaxMessageBytes, storeMessages: cfg.StoreMessages, domainNoStore: strings.ToLower(cfg.DomainNoStore), waitgroup: new(sync.WaitGroup)} } -// Main listener loop +// Start the listener and handle incoming connections func (s *Server) Start() { - cfg := config.GetSmtpConfig() + cfg := config.GetSMTPConfig() addr, err := net.ResolveTCPAddr("tcp4", fmt.Sprintf("%v:%v", - cfg.Ip4address, cfg.Ip4port)) + cfg.IP4address, cfg.IP4port)) if err != nil { - log.LogError("Failed to build tcp4 address: %v", err) + log.Errorf("Failed to build tcp4 address: %v", err) // TODO More graceful early-shutdown procedure panic(err) } - log.LogInfo("SMTP listening on TCP4 %v", addr) + log.Infof("SMTP listening on TCP4 %v", addr) s.listener, err = net.ListenTCP("tcp4", addr) if err != nil { - log.LogError("SMTP failed to start tcp4 listener: %v", err) + log.Errorf("SMTP failed to start tcp4 listener: %v", err) // TODO More graceful early-shutdown procedure panic(err) } if !s.storeMessages { - log.LogInfo("Load test mode active, messages will not be stored") + log.Infof("Load test mode active, messages will not be stored") } else if s.domainNoStore != "" { - log.LogInfo("Messages sent to domain '%v' will be discarded", s.domainNoStore) + log.Infof("Messages sent to domain '%v' will be discarded", s.domainNoStore) } // Start retention scanner @@ -96,12 +96,12 @@ func (s *Server) Start() { if max := 1 * time.Second; tempDelay > max { tempDelay = max } - log.LogError("SMTP accept error: %v; retrying in %v", err, tempDelay) + log.Errorf("SMTP accept error: %v; retrying in %v", err, tempDelay) time.Sleep(tempDelay) continue } else { if s.shutdown { - log.LogTrace("SMTP listener shutting down on request") + log.Tracef("SMTP listener shutting down on request") return } // TODO Implement a max error counter before shutdown? @@ -119,15 +119,17 @@ func (s *Server) Start() { // Stop requests the SMTP server closes it's listener func (s *Server) Stop() { - log.LogTrace("SMTP shutdown requested, connections will be drained") + log.Tracef("SMTP shutdown requested, connections will be drained") s.shutdown = true - s.listener.Close() + if err := s.listener.Close(); err != nil { + log.Errorf("Failed to close SMTP listener: %v", err) + } } // Drain causes the caller to block until all active SMTP sessions have finished func (s *Server) Drain() { s.waitgroup.Wait() - log.LogTrace("SMTP connections drained") + log.Tracef("SMTP connections drained") } // When the provided Ticker ticks, we update our metrics history diff --git a/smtpd/retention.go b/smtpd/retention.go index f17a6af..ac1b3f8 100644 --- a/smtpd/retention.go +++ b/smtpd/retention.go @@ -21,20 +21,22 @@ var expRetainedCurrent = new(expvar.Int) var retentionDeletesHist = list.New() var retainedHist = list.New() -// History rendered as comma delim string +// History rendered as comma delimited string var expRetentionDeletesHist = new(expvar.String) var expRetainedHist = new(expvar.String) +// StartRetentionScanner launches a go-routine that scans for expired +// messages, following the configured interval func StartRetentionScanner(ds DataStore) { cfg := config.GetDataStoreConfig() expRetentionPeriod.Set(int64(cfg.RetentionMinutes * 60)) if cfg.RetentionMinutes > 0 { // Retention scanning enabled - log.LogInfo("Retention configured for %v minutes", cfg.RetentionMinutes) + log.Infof("Retention configured for %v minutes", cfg.RetentionMinutes) go retentionScanner(ds, time.Duration(cfg.RetentionMinutes)*time.Minute, time.Duration(cfg.RetentionSleep)*time.Millisecond) } else { - log.LogInfo("Retention scanner disabled") + log.Infof("Retention scanner disabled") } } @@ -45,21 +47,21 @@ func retentionScanner(ds DataStore, maxAge time.Duration, sleep time.Duration) { since := time.Since(start) if since < time.Minute { dur := time.Minute - since - log.LogTrace("Retention scanner sleeping for %v", dur) + log.Tracef("Retention scanner sleeping for %v", dur) time.Sleep(dur) } start = time.Now() // Kickoff scan if err := doRetentionScan(ds, maxAge, sleep); err != nil { - log.LogError("Error during retention scan: %v", err) + log.Errorf("Error during retention scan: %v", err) } } } // doRetentionScan does a single pass of all mailboxes looking for messages that can be purged func doRetentionScan(ds DataStore, maxAge time.Duration, sleep time.Duration) error { - log.LogTrace("Starting retention scan") + log.Tracef("Starting retention scan") cutoff := time.Now().Add(-1 * maxAge) mboxes, err := ds.AllMailboxes() if err != nil { @@ -74,11 +76,11 @@ func doRetentionScan(ds DataStore, maxAge time.Duration, sleep time.Duration) er } for _, msg := range messages { if msg.Date().Before(cutoff) { - log.LogTrace("Purging expired message %v", msg.Id()) + log.Tracef("Purging expired message %v", msg.ID()) err = msg.Delete() if err != nil { // Log but don't abort - log.LogError("Failed to purge message %v: %v", msg.Id(), err) + log.Errorf("Failed to purge message %v: %v", msg.ID(), err) } else { expRetentionDeletesTotal.Add(1) } diff --git a/smtpd/retention_test.go b/smtpd/retention_test.go index d85e511..f77cd3a 100644 --- a/smtpd/retention_test.go +++ b/smtpd/retention_test.go @@ -36,7 +36,9 @@ func TestDoRetentionScan(t *testing.T) { mb3.On("GetMessages").Return([]Message{new3}, nil) // Test 4 hour retention - doRetentionScan(mds, 4*time.Hour, 0) + if err := doRetentionScan(mds, 4*time.Hour, 0); err != nil { + t.Error(err) + } // Check our assertions mds.AssertExpectations(t) @@ -58,7 +60,7 @@ func TestDoRetentionScan(t *testing.T) { // Make a MockMessage of a specific age func mockMessage(ageHours int) *MockMessage { msg := &MockMessage{} - msg.On("Id").Return(fmt.Sprintf("MSG[age=%vh]", ageHours)) + msg.On("ID").Return(fmt.Sprintf("MSG[age=%vh]", ageHours)) msg.On("Date").Return(time.Now().Add(time.Duration(ageHours*-1) * time.Hour)) msg.On("Delete").Return(nil) return msg @@ -114,7 +116,7 @@ type MockMessage struct { mock.Mock } -func (m *MockMessage) Id() string { +func (m *MockMessage) ID() string { args := m.Called() return args.String(0) } diff --git a/smtpd/utils.go b/smtpd/utils.go index 165e76d..06a38fe 100644 --- a/smtpd/utils.go +++ b/smtpd/utils.go @@ -9,9 +9,10 @@ import ( "strings" ) -// Take "user+ext" and return "user", aka the mailbox we'll store it in -// Return error if it contains invalid characters, we don't accept anything -// that must be quoted according to RFC3696. +// ParseMailboxName takes a localPart string (ex: "user+ext" without "@domain") +// and returns just the mailbox name (ex: "user"). Returns an error if +// localPart contains invalid characters; it won't accept any that must be +// quoted according to RFC3696. func ParseMailboxName(localPart string) (result string, err error) { if localPart == "" { return "", fmt.Errorf("Mailbox name cannot be empty") @@ -41,10 +42,14 @@ func ParseMailboxName(localPart string) (result string, err error) { return result, nil } -// Take a mailbox name and hash it into the directory we'll store it in +// HashMailboxName accepts a mailbox name and hashes it. Inbucket uses this as +// the directory to house the mailbox func HashMailboxName(mailbox string) string { h := sha1.New() - io.WriteString(h, mailbox) + if _, err := io.WriteString(h, mailbox); err != nil { + // This shouldn't ever happen + return "" + } return fmt.Sprintf("%x", h.Sum(nil)) } @@ -138,15 +143,15 @@ LOOP: switch { case ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'): // Letters are OK - buf.WriteByte(c) + _ = buf.WriteByte(c) inCharQuote = false case '0' <= c && c <= '9': // Numbers are OK - buf.WriteByte(c) + _ = buf.WriteByte(c) inCharQuote = false case bytes.IndexByte([]byte("!#$%&'*+-/=?^_`{|}~"), c) >= 0: // These specials can be used unquoted - buf.WriteByte(c) + _ = buf.WriteByte(c) inCharQuote = false case c == '.': // A single period is OK @@ -154,13 +159,13 @@ LOOP: // Sequence of periods is not permitted return "", "", fmt.Errorf("Sequence of periods is not permitted") } - buf.WriteByte(c) + _ = buf.WriteByte(c) inCharQuote = false case c == '\\': inCharQuote = true case c == '"': if inCharQuote { - buf.WriteByte(c) + _ = buf.WriteByte(c) inCharQuote = false } else if inStringQuote { inStringQuote = false @@ -173,7 +178,7 @@ LOOP: } case c == '@': if inCharQuote || inStringQuote { - buf.WriteByte(c) + _ = buf.WriteByte(c) inCharQuote = false } else { // End of local-part @@ -190,7 +195,7 @@ LOOP: return "", "", fmt.Errorf("Characters outside of US-ASCII range not permitted") default: if inCharQuote || inStringQuote { - buf.WriteByte(c) + _ = buf.WriteByte(c) inCharQuote = false } else { return "", "", fmt.Errorf("Character %q must be quoted", c) diff --git a/themes/bootstrap/templates/mailbox/_list.html b/themes/bootstrap/templates/mailbox/_list.html index 9500bcf..a27bc49 100644 --- a/themes/bootstrap/templates/mailbox/_list.html +++ b/themes/bootstrap/templates/mailbox/_list.html @@ -1,6 +1,6 @@ {{$name := .name}} {{range .messages}} - {{if .htmlAvailable}} diff --git a/themes/integral/templates/mailbox/_list.html b/themes/integral/templates/mailbox/_list.html index 49a656e..6404fd9 100644 --- a/themes/integral/templates/mailbox/_list.html +++ b/themes/integral/templates/mailbox/_list.html @@ -1,6 +1,6 @@ {{$name := .name}} {{range .messages}} -
+
{{.Subject}}
{{.From}}
{{friendlyTime .Date}}
diff --git a/themes/integral/templates/mailbox/_show.html b/themes/integral/templates/mailbox/_show.html index 4ca2c55..6dbaf3d 100644 --- a/themes/integral/templates/mailbox/_show.html +++ b/themes/integral/templates/mailbox/_show.html @@ -1,11 +1,11 @@ {{$name := .name}} -{{$id := .message.Id}} +{{$id := .message.ID}}
Link - Delete - Source + Delete + Source {{if .htmlAvailable}} - HTML + HTML {{end}}
diff --git a/web/context.go b/web/context.go index 0eda5dd..b23891b 100644 --- a/web/context.go +++ b/web/context.go @@ -9,13 +9,15 @@ import ( "github.com/jhillyerd/inbucket/smtpd" ) +// Context is passed into every request handler function type Context struct { Vars map[string]string Session *sessions.Session DataStore smtpd.DataStore - IsJson bool + IsJSON bool } +// Close the Context (currently does nothing) func (c *Context) Close() { // Do nothing } @@ -37,6 +39,7 @@ func headerMatch(req *http.Request, name string, value string) bool { return false } +// NewContext returns a Context for the given HTTP Request func NewContext(req *http.Request) (*Context, error) { vars := mux.Vars(req) sess, err := sessionStore.Get(req, "inbucket") @@ -44,7 +47,7 @@ func NewContext(req *http.Request) (*Context, error) { Vars: vars, Session: sess, DataStore: DataStore, - IsJson: headerMatch(req, "Accept", "application/json"), + IsJSON: headerMatch(req, "Accept", "application/json"), } if err != nil { return ctx, err diff --git a/web/helpers.go b/web/helpers.go index 1f06dab..6aa7c99 100644 --- a/web/helpers.go +++ b/web/helpers.go @@ -11,10 +11,11 @@ import ( "github.com/jhillyerd/inbucket/log" ) +// TemplateFuncs declares functions made available to all templates (including partials) var TemplateFuncs = template.FuncMap{ "friendlyTime": friendlyTime, "reverse": reverse, - "textToHtml": textToHtml, + "textToHtml": textToHTML, } // From http://daringfireball.net/2010/07/improved_regex_for_matching_urls @@ -40,7 +41,7 @@ func reverse(name string, things ...interface{}) string { // Grab the route u, err := Router.Get(name).URL(strs...) if err != nil { - log.LogError("Failed to reverse route: %v", err) + log.Errorf("Failed to reverse route: %v", err) return "/ROUTE-ERROR" } return u.Path @@ -48,15 +49,15 @@ func reverse(name string, things ...interface{}) string { // textToHtml takes plain text, escapes it and tries to pretty it up for // HTML display -func textToHtml(text string) template.HTML { +func textToHTML(text string) template.HTML { text = html.EscapeString(text) - text = urlRE.ReplaceAllStringFunc(text, wrapUrl) + text = urlRE.ReplaceAllStringFunc(text, wrapURL) replacer := strings.NewReplacer("\r\n", "
\n", "\r", "
\n", "\n", "
\n") return template.HTML(replacer.Replace(text)) } // wrapUrl wraps a tag around the provided URL -func wrapUrl(url string) string { +func wrapURL(url string) string { unescaped := strings.Replace(url, "&", "&", -1) return fmt.Sprintf("%s", unescaped, url) } diff --git a/web/helpers_test.go b/web/helpers_test.go index 6c6c9ae..ca7837d 100644 --- a/web/helpers_test.go +++ b/web/helpers_test.go @@ -9,22 +9,22 @@ import ( func TestTextToHtml(t *testing.T) { // Identity - assert.Equal(t, textToHtml("html"), template.HTML("html")) + assert.Equal(t, textToHTML("html"), template.HTML("html")) // Check it escapes - assert.Equal(t, textToHtml(""), template.HTML("<html>")) + assert.Equal(t, textToHTML(""), template.HTML("<html>")) // Check for linebreaks - assert.Equal(t, textToHtml("line\nbreak"), template.HTML("line
\nbreak")) - assert.Equal(t, textToHtml("line\r\nbreak"), template.HTML("line
\nbreak")) - assert.Equal(t, textToHtml("line\rbreak"), template.HTML("line
\nbreak")) + assert.Equal(t, textToHTML("line\nbreak"), template.HTML("line
\nbreak")) + assert.Equal(t, textToHTML("line\r\nbreak"), template.HTML("line
\nbreak")) + assert.Equal(t, textToHTML("line\rbreak"), template.HTML("line
\nbreak")) } func TestURLDetection(t *testing.T) { assert.Equal(t, - textToHtml("http://google.com/"), + textToHTML("http://google.com/"), template.HTML("http://google.com/")) assert.Equal(t, - textToHtml("http://a.com/?q=a&n=v"), + textToHTML("http://a.com/?q=a&n=v"), template.HTML("http://a.com/?q=a&n=v")) } diff --git a/web/mailbox_controller.go b/web/mailbox_controller.go index ccbcba7..acee9d3 100644 --- a/web/mailbox_controller.go +++ b/web/mailbox_controller.go @@ -13,24 +13,35 @@ import ( "github.com/jhillyerd/inbucket/smtpd" ) -type JsonMessageHeader struct { - Mailbox, Id, From, Subject string - Date time.Time - Size int64 +// JSONMessageHeader contains the basic header data for a message +type JSONMessageHeader struct { + Mailbox string + ID string `json:"Id"` + From string + Subject string + Date time.Time + Size int64 } -type JsonMessage struct { - Mailbox, Id, From, Subject string - Date time.Time - Size int64 - Body *JsonMessageBody - Header mail.Header +// JSONMessage contains the same data as the header plus a JSONMessageBody +type JSONMessage struct { + Mailbox string + ID string `json:"Id"` + From string + Subject string + Date time.Time + Size int64 + Body *JSONMessageBody + Header mail.Header } -type JsonMessageBody struct { - Text, Html string +// JSONMessageBody contains the Text and HTML versions of the message body +type JSONMessageBody struct { + Text string + HTML string `json:"Html"` } +// MailboxIndex renders the index page for a particular mailbox func MailboxIndex(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { // Form values must be validated manually name := req.FormValue("name") @@ -59,6 +70,7 @@ func MailboxIndex(w http.ResponseWriter, req *http.Request, ctx *Context) (err e }) } +// MailboxLink handles pretty links to a particular message func MailboxLink(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] @@ -74,6 +86,7 @@ func MailboxLink(w http.ResponseWriter, req *http.Request, ctx *Context) (err er return nil } +// MailboxList renders a list of messages in a mailbox func MailboxList(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) @@ -88,21 +101,21 @@ func MailboxList(w http.ResponseWriter, req *http.Request, ctx *Context) (err er if err != nil { return fmt.Errorf("Failed to get messages for %v: %v", name, err) } - log.LogTrace("Got %v messsages", len(messages)) + log.Tracef("Got %v messsages", len(messages)) - if ctx.IsJson { - jmessages := make([]*JsonMessageHeader, len(messages)) + if ctx.IsJSON { + jmessages := make([]*JSONMessageHeader, len(messages)) for i, msg := range messages { - jmessages[i] = &JsonMessageHeader{ + jmessages[i] = &JSONMessageHeader{ Mailbox: name, - Id: msg.Id(), + ID: msg.ID(), From: msg.From(), Subject: msg.Subject(), Date: msg.Date(), Size: msg.Size(), } } - return RenderJson(w, jmessages) + return RenderJSON(w, jmessages) } return RenderPartial("mailbox/_list.html", w, map[string]interface{}{ @@ -112,6 +125,7 @@ func MailboxList(w http.ResponseWriter, req *http.Request, ctx *Context) (err er }) } +// MailboxShow renders a particular message from a mailbox func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] @@ -140,24 +154,24 @@ func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *Context) (err er return fmt.Errorf("ReadBody() failed: %v", err) } - if ctx.IsJson { - return RenderJson(w, - &JsonMessage{ + if ctx.IsJSON { + return RenderJSON(w, + &JSONMessage{ Mailbox: name, - Id: msg.Id(), + ID: msg.ID(), From: msg.From(), Subject: msg.Subject(), Date: msg.Date(), Size: msg.Size(), Header: header.Header, - Body: &JsonMessageBody{ + Body: &JSONMessageBody{ Text: mime.Text, - Html: mime.Html, + HTML: mime.Html, }, }) } - body := template.HTML(textToHtml(mime.Text)) + body := template.HTML(textToHTML(mime.Text)) htmlAvailable := mime.Html != "" return RenderPartial("mailbox/_show.html", w, map[string]interface{}{ @@ -170,6 +184,7 @@ func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *Context) (err er }) } +// MailboxPurge deletes all messages from a mailbox func MailboxPurge(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) @@ -183,18 +198,21 @@ func MailboxPurge(w http.ResponseWriter, req *http.Request, ctx *Context) (err e if err := mb.Purge(); err != nil { return fmt.Errorf("Mailbox(%q) Purge: %v", name, err) } - log.LogTrace("Purged mailbox for %q", name) + log.Tracef("Purged mailbox for %q", name) - if ctx.IsJson { - return RenderJson(w, "OK") + if ctx.IsJSON { + return RenderJSON(w, "OK") } w.Header().Set("Content-Type", "text/plain") - io.WriteString(w, "OK") + if _, err := io.WriteString(w, "OK"); err != nil { + return err + } return nil } -func MailboxHtml(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { +// MailboxHTML displays the HTML content of a message +func MailboxHTML(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) @@ -224,6 +242,7 @@ func MailboxHtml(w http.ResponseWriter, req *http.Request, ctx *Context) (err er }) } +// MailboxSource displays the raw source of a message, including headers func MailboxSource(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] @@ -245,10 +264,14 @@ func MailboxSource(w http.ResponseWriter, req *http.Request, ctx *Context) (err } w.Header().Set("Content-Type", "text/plain") - io.WriteString(w, *raw) + if _, err := io.WriteString(w, *raw); err != nil { + return err + } return nil } +// MailboxDownloadAttach sends the attachment to the client; disposition: +// attachment, type: application/octet-stream func MailboxDownloadAttach(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] @@ -287,10 +310,13 @@ func MailboxDownloadAttach(w http.ResponseWriter, req *http.Request, ctx *Contex w.Header().Set("Content-Type", "application/octet-stream") w.Header().Set("Content-Disposition", "attachment") - w.Write(part.Content()) + if _, err := w.Write(part.Content()); err != nil { + return err + } return nil } +// MailboxViewAttach sends the attachment to the client for online viewing func MailboxViewAttach(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 name, err := smtpd.ParseMailboxName(ctx.Vars["name"]) @@ -328,10 +354,13 @@ func MailboxViewAttach(w http.ResponseWriter, req *http.Request, ctx *Context) ( part := body.Attachments[num] w.Header().Set("Content-Type", part.ContentType()) - w.Write(part.Content()) + if _, err := w.Write(part.Content()); err != nil { + return err + } return nil } +// MailboxDelete removes a particular message from a mailbox func MailboxDelete(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] @@ -352,11 +381,13 @@ func MailboxDelete(w http.ResponseWriter, req *http.Request, ctx *Context) (err return err } - if ctx.IsJson { - return RenderJson(w, "OK") + if ctx.IsJSON { + return RenderJSON(w, "OK") } w.Header().Set("Content-Type", "text/plain") - io.WriteString(w, "OK") + if _, err := io.WriteString(w, "OK"); err != nil { + return err + } return nil } diff --git a/web/rest.go b/web/rest.go index be5238d..aa9d864 100644 --- a/web/rest.go +++ b/web/rest.go @@ -5,7 +5,9 @@ import ( "net/http" ) -func RenderJson(w http.ResponseWriter, data interface{}) error { +// RenderJSON sets the correct HTTP headers for JSON, then writes the specified +// data (typically a struct) encoded in JSON +func RenderJSON(w http.ResponseWriter, data interface{}) error { w.Header().Set("Content-Type", "application/json; charset=utf-8") w.Header().Set("Expires", "-1") enc := json.NewEncoder(w) diff --git a/web/rest_test.go b/web/rest_test.go index b50b1ab..c7254e0 100644 --- a/web/rest_test.go +++ b/web/rest_test.go @@ -19,31 +19,42 @@ import ( "github.com/stretchr/testify/mock" ) -type OutputJsonHeader struct { - Mailbox, Id, From, Subject, Date string - Size int +// OutputJSONHeader holds the received Header +type OutputJSONHeader struct { + Mailbox string + ID string `json:"Id"` + From, Subject, Date string + Size int } -type OutputJsonMessage struct { - Mailbox, Id, From, Subject, Date string - Size int - Header map[string][]string - Body struct { - Text, Html string +// OutputJSONMessage holds the received Message +type OutputJSONMessage struct { + Mailbox string + ID string `json:"Id"` + From, Subject, Date string + Size int + Header map[string][]string + Body struct { + Text string + HTML string `json:"Html"` } } +// InputMessageData holds the message we want to test type InputMessageData struct { - Mailbox, Id, From, Subject string - Date time.Time - Size int - Header mail.Header - Html, Text string + Mailbox string + ID string `json:"Id"` + From, Subject string + Date time.Time + Size int + Header mail.Header + HTML string `json:"Html"` + Text string } func (d *InputMessageData) MockMessage() *MockMessage { msg := &MockMessage{} - msg.On("Id").Return(d.Id) + msg.On("ID").Return(d.ID) msg.On("From").Return(d.From) msg.On("Subject").Return(d.Subject) msg.On("Date").Return(d.Date) @@ -54,20 +65,20 @@ func (d *InputMessageData) MockMessage() *MockMessage { msg.On("ReadHeader").Return(gomsg, nil) body := &enmime.MIMEBody{ Text: d.Text, - Html: d.Html, + Html: d.HTML, } msg.On("ReadBody").Return(body, nil) return msg } -func (d *InputMessageData) CompareToJsonHeader(j *OutputJsonHeader) (errors []string) { +func (d *InputMessageData) CompareToJSONHeader(j *OutputJSONHeader) (errors []string) { if d.Mailbox != j.Mailbox { errors = append(errors, fmt.Sprintf("Expected JSON.Mailbox=%q, got %q", d.Mailbox, j.Mailbox)) } - if d.Id != j.Id { - errors = append(errors, fmt.Sprintf("Expected JSON.Id=%q, got %q", d.Id, - j.Id)) + if d.ID != j.ID { + errors = append(errors, fmt.Sprintf("Expected JSON.Id=%q, got %q", d.ID, + j.ID)) } if d.From != j.From { errors = append(errors, fmt.Sprintf("Expected JSON.From=%q, got %q", d.From, @@ -90,14 +101,14 @@ func (d *InputMessageData) CompareToJsonHeader(j *OutputJsonHeader) (errors []st return errors } -func (d *InputMessageData) CompareToJsonMessage(j *OutputJsonMessage) (errors []string) { +func (d *InputMessageData) CompareToJSONMessage(j *OutputJSONMessage) (errors []string) { if d.Mailbox != j.Mailbox { errors = append(errors, fmt.Sprintf("Expected JSON.Mailbox=%q, got %q", d.Mailbox, j.Mailbox)) } - if d.Id != j.Id { - errors = append(errors, fmt.Sprintf("Expected JSON.Id=%q, got %q", d.Id, - j.Id)) + if d.ID != j.ID { + errors = append(errors, fmt.Sprintf("Expected JSON.Id=%q, got %q", d.ID, + j.ID)) } if d.From != j.From { errors = append(errors, fmt.Sprintf("Expected JSON.From=%q, got %q", d.From, @@ -120,9 +131,9 @@ func (d *InputMessageData) CompareToJsonMessage(j *OutputJsonMessage) (errors [] errors = append(errors, fmt.Sprintf("Expected JSON.Text=%q, got %q", d.Text, j.Body.Text)) } - if d.Html != j.Body.Html { - errors = append(errors, fmt.Sprintf("Expected JSON.Html=%q, got %q", d.Html, - j.Body.Html)) + if d.HTML != j.Body.HTML { + errors = append(errors, fmt.Sprintf("Expected JSON.Html=%q, got %q", d.HTML, + j.Body.HTML)) } for k, vals := range d.Header { jvals, ok := j.Header[k] @@ -191,7 +202,7 @@ func TestRestMailboxList(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } // Test MailboxFor error @@ -211,14 +222,14 @@ func TestRestMailboxList(t *testing.T) { // Test JSON message headers data1 := &InputMessageData{ Mailbox: "good", - Id: "0001", + ID: "0001", From: "from1", Subject: "subject 1", Date: time.Date(2012, 2, 1, 10, 11, 12, 253, time.FixedZone("PST", -800)), } data2 := &InputMessageData{ Mailbox: "good", - Id: "0002", + ID: "0002", From: "from2", Subject: "subject 2", Date: time.Date(2012, 7, 1, 10, 11, 12, 253, time.FixedZone("PDT", -700)), @@ -241,19 +252,19 @@ func TestRestMailboxList(t *testing.T) { // Check JSON dec := json.NewDecoder(w.Body) - var result []OutputJsonHeader + var result []OutputJSONHeader if err := dec.Decode(&result); err != nil { t.Errorf("Failed to decode JSON: %v", err) } if len(result) != 2 { t.Errorf("Expected 2 results, got %v", len(result)) } - if errors := data1.CompareToJsonHeader(&result[0]); len(errors) > 0 { + if errors := data1.CompareToJSONHeader(&result[0]); len(errors) > 0 { for _, e := range errors { t.Error(e) } } - if errors := data2.CompareToJsonHeader(&result[1]); len(errors) > 0 { + if errors := data2.CompareToJSONHeader(&result[1]); len(errors) > 0 { for _, e := range errors { t.Error(e) } @@ -263,7 +274,7 @@ func TestRestMailboxList(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -311,7 +322,7 @@ func TestRestMessage(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } // Test GetMessage error @@ -331,7 +342,7 @@ func TestRestMessage(t *testing.T) { // Test JSON message headers data1 := &InputMessageData{ Mailbox: "good", - Id: "0001", + ID: "0001", From: "from1", Subject: "subject 1", Date: time.Date(2012, 2, 1, 10, 11, 12, 253, time.FixedZone("PST", -800)), @@ -339,7 +350,7 @@ func TestRestMessage(t *testing.T) { "To": []string{"fred@fish.com", "keyword@nsa.gov"}, }, Text: "This is some text", - Html: "This is some HTML", + HTML: "This is some HTML", } goodbox := &MockMailbox{} ds.On("MailboxFor", "good").Return(goodbox, nil) @@ -358,11 +369,11 @@ func TestRestMessage(t *testing.T) { // Check JSON dec := json.NewDecoder(w.Body) - var result OutputJsonMessage + var result OutputJSONMessage if err := dec.Decode(&result); err != nil { t.Errorf("Failed to decode JSON: %v", err) } - if errors := data1.CompareToJsonMessage(&result); len(errors) > 0 { + if errors := data1.CompareToJSONMessage(&result); len(errors) > 0 { for _, e := range errors { t.Error(e) } @@ -372,7 +383,7 @@ func TestRestMessage(t *testing.T) { // Wait for handler to finish logging time.Sleep(2 * time.Second) // Dump buffered log data if there was a failure - io.Copy(os.Stderr, logbuf) + _, _ = io.Copy(os.Stderr, logbuf) } } @@ -404,7 +415,7 @@ func setupWebServer(ds smtpd.DataStore) *bytes.Buffer { return buf } -// Mock DataStore object +// MockDataStore used to mock DataStore interface type MockDataStore struct { mock.Mock } @@ -419,7 +430,7 @@ func (m *MockDataStore) AllMailboxes() ([]smtpd.Mailbox, error) { return args.Get(0).([]smtpd.Mailbox), args.Error(1) } -// Mock Mailbox object +// MockMailbox used to mock Mailbox interface type MockMailbox struct { mock.Mock } @@ -454,7 +465,7 @@ type MockMessage struct { mock.Mock } -func (m *MockMessage) Id() string { +func (m *MockMessage) ID() string { args := m.Called() return args.String(0) } diff --git a/web/root_controller.go b/web/root_controller.go index b7d2c1c..adcc874 100644 --- a/web/root_controller.go +++ b/web/root_controller.go @@ -9,6 +9,7 @@ import ( "github.com/jhillyerd/inbucket/config" ) +// RootIndex serves the Inbucket landing page func RootIndex(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { greeting, err := ioutil.ReadFile(config.GetWebConfig().GreetingFile) if err != nil { @@ -21,18 +22,19 @@ func RootIndex(w http.ResponseWriter, req *http.Request, ctx *Context) (err erro }) } +// RootStatus serves the Inbucket status page func RootStatus(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) { retentionMinutes := config.GetDataStoreConfig().RetentionMinutes - smtpListener := fmt.Sprintf("%s:%d", config.GetSmtpConfig().Ip4address.String(), - config.GetSmtpConfig().Ip4port) - pop3Listener := fmt.Sprintf("%s:%d", config.GetPop3Config().Ip4address.String(), - config.GetPop3Config().Ip4port) - webListener := fmt.Sprintf("%s:%d", config.GetWebConfig().Ip4address.String(), - config.GetWebConfig().Ip4port) + smtpListener := fmt.Sprintf("%s:%d", config.GetSMTPConfig().IP4address.String(), + config.GetSMTPConfig().IP4port) + pop3Listener := fmt.Sprintf("%s:%d", config.GetPOP3Config().IP4address.String(), + config.GetPOP3Config().IP4port) + webListener := fmt.Sprintf("%s:%d", config.GetWebConfig().IP4address.String(), + config.GetWebConfig().IP4port) return RenderTemplate("root/status.html", w, map[string]interface{}{ "ctx": ctx, - "version": config.VERSION, - "buildDate": config.BUILD_DATE, + "version": config.Version, + "buildDate": config.BuildDate, "retentionMinutes": retentionMinutes, "smtpListener": smtpListener, "pop3Listener": pop3Listener, diff --git a/web/server.go b/web/server.go index 461ed62..b6994d9 100644 --- a/web/server.go +++ b/web/server.go @@ -1,6 +1,4 @@ -/* - The web package contains all the code to provide Inbucket's web GUI -*/ +// Package web provides Inbucket's web GUI and RESTful API package web import ( @@ -19,12 +17,18 @@ import ( type handler func(http.ResponseWriter, *http.Request, *Context) error -var webConfig config.WebConfig -var DataStore smtpd.DataStore -var Router *mux.Router -var listener net.Listener -var sessionStore sessions.Store -var shutdown bool +var ( + // DataStore is where all the mailboxes and messages live + DataStore smtpd.DataStore + + // Router sends incoming requests to the correct handler function + Router *mux.Router + + webConfig config.WebConfig + listener net.Listener + sessionStore sessions.Store + shutdown bool +) // Initialize sets up things for unit tests or the Start() method func Initialize(cfg config.WebConfig, ds smtpd.DataStore) { @@ -39,8 +43,8 @@ func Initialize(cfg config.WebConfig, ds smtpd.DataStore) { } func setupRoutes(cfg config.WebConfig) { - log.LogInfo("Theme templates mapped to '%v'", cfg.TemplateDir) - log.LogInfo("Theme static content mapped to '%v'", cfg.PublicDir) + log.Infof("Theme templates mapped to '%v'", cfg.TemplateDir) + log.Infof("Theme static content mapped to '%v'", cfg.PublicDir) r := mux.NewRouter() // Static content @@ -55,7 +59,7 @@ func setupRoutes(cfg config.WebConfig) { r.Path("/mailbox/{name}").Handler(handler(MailboxList)).Name("MailboxList").Methods("GET") r.Path("/mailbox/{name}").Handler(handler(MailboxPurge)).Name("MailboxPurge").Methods("DELETE") r.Path("/mailbox/{name}/{id}").Handler(handler(MailboxShow)).Name("MailboxShow").Methods("GET") - r.Path("/mailbox/{name}/{id}/html").Handler(handler(MailboxHtml)).Name("MailboxHtml").Methods("GET") + r.Path("/mailbox/{name}/{id}/html").Handler(handler(MailboxHTML)).Name("MailboxHtml").Methods("GET") r.Path("/mailbox/{name}/{id}/source").Handler(handler(MailboxSource)).Name("MailboxSource").Methods("GET") r.Path("/mailbox/{name}/{id}").Handler(handler(MailboxDelete)).Name("MailboxDelete").Methods("DELETE") r.Path("/mailbox/dattach/{name}/{id}/{num}/{file}").Handler(handler(MailboxDownloadAttach)).Name("MailboxDownloadAttach").Methods("GET") @@ -66,9 +70,9 @@ func setupRoutes(cfg config.WebConfig) { http.Handle("/", Router) } -// Start() the web server +// Start begins listening for HTTP requests func Start() { - addr := fmt.Sprintf("%v:%v", webConfig.Ip4address, webConfig.Ip4port) + addr := fmt.Sprintf("%v:%v", webConfig.IP4address, webConfig.IP4port) server := &http.Server{ Addr: addr, Handler: nil, @@ -77,30 +81,33 @@ func Start() { } // We don't use ListenAndServe because it lacks a way to close the listener - log.LogInfo("HTTP listening on TCP4 %v", addr) + log.Infof("HTTP listening on TCP4 %v", addr) var err error listener, err = net.Listen("tcp", addr) if err != nil { - log.LogError("HTTP failed to start TCP4 listener: %v", err) + log.Errorf("HTTP failed to start TCP4 listener: %v", err) // TODO More graceful early-shutdown procedure panic(err) } err = server.Serve(listener) if shutdown { - log.LogTrace("HTTP server shutting down on request") + log.Tracef("HTTP server shutting down on request") } else if err != nil { - log.LogError("HTTP server failed: %v", err) + log.Errorf("HTTP server failed: %v", err) } } +// Stop shuts down the HTTP server func Stop() { - log.LogTrace("HTTP shutdown requested") + log.Tracef("HTTP shutdown requested") shutdown = true if listener != nil { - listener.Close() + if err := listener.Close(); err != nil { + log.Errorf("Error closing HTTP listener: %v", err) + } } else { - log.LogError("HTTP listener was nil during shutdown") + log.Errorf("HTTP listener was nil during shutdown") } } @@ -109,7 +116,7 @@ func (h handler) ServeHTTP(w http.ResponseWriter, req *http.Request) { // Create the context ctx, err := NewContext(req) if err != nil { - log.LogError("Failed to create context: %v", err) + log.Errorf("Failed to create context: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -117,21 +124,23 @@ func (h handler) ServeHTTP(w http.ResponseWriter, req *http.Request) { // Run the handler, grab the error, and report it buf := new(httpbuf.Buffer) - log.LogTrace("Web: %v %v %v %v", req.RemoteAddr, req.Proto, req.Method, req.RequestURI) + log.Tracef("Web: %v %v %v %v", req.RemoteAddr, req.Proto, req.Method, req.RequestURI) err = h(buf, req, ctx) if err != nil { - log.LogError("Error handling %v: %v", req.RequestURI, err) + log.Errorf("Error handling %v: %v", req.RequestURI, err) http.Error(w, err.Error(), http.StatusInternalServerError) return } // Save the session if err = ctx.Session.Save(req, buf); err != nil { - log.LogError("Failed to save session: %v", err) + log.Errorf("Failed to save session: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } // Apply the buffered response to the writer - buf.Apply(w) + if _, err = buf.Apply(w); err != nil { + log.Errorf("Failed to write response: %v", err) + } } diff --git a/web/template.go b/web/template.go index b0aa105..61c9b84 100644 --- a/web/template.go +++ b/web/template.go @@ -20,7 +20,7 @@ var cachedPartials = map[string]*template.Template{} func RenderTemplate(name string, w http.ResponseWriter, data interface{}) error { t, err := ParseTemplate(name, false) if err != nil { - log.LogError("Error in template '%v': %v", name, err) + log.Errorf("Error in template '%v': %v", name, err) return err } w.Header().Set("Expires", "-1") @@ -32,7 +32,7 @@ func RenderTemplate(name string, w http.ResponseWriter, data interface{}) error func RenderPartial(name string, w http.ResponseWriter, data interface{}) error { t, err := ParseTemplate(name, true) if err != nil { - log.LogError("Error in template '%v': %v", name, err) + log.Errorf("Error in template '%v': %v", name, err) return err } w.Header().Set("Expires", "-1") @@ -51,7 +51,7 @@ func ParseTemplate(name string, partial bool) (*template.Template, error) { tempPath := strings.Replace(name, "/", string(filepath.Separator), -1) tempFile := filepath.Join(webConfig.TemplateDir, tempPath) - log.LogTrace("Parsing template %v", tempFile) + log.Tracef("Parsing template %v", tempFile) var err error var t *template.Template @@ -71,10 +71,10 @@ func ParseTemplate(name string, partial bool) (*template.Template, error) { // Allows us to disable caching for theme development if webConfig.TemplateCache { if partial { - log.LogTrace("Caching partial %v", name) + log.Tracef("Caching partial %v", name) cachedTemplates[name] = t } else { - log.LogTrace("Caching template %v", name) + log.Tracef("Caching template %v", name) cachedTemplates[name] = t } }