diff --git a/inbucket.go b/inbucket.go index 81b51d0..d2a016d 100644 --- a/inbucket.go +++ b/inbucket.go @@ -179,7 +179,6 @@ func signalProcessor(c <-chan os.Signal) { log.Infof("Recieved SIGHUP, cycling logfile") closeLogFile() // There is nothing we can do if the log open fails - // TODO We could panic, but that would be lame? _ = openLogFile() } else { log.Infof("Ignoring SIGHUP, logfile not configured") diff --git a/pop3d/listener.go b/pop3d/listener.go index 94c63f0..3f0d698 100644 --- a/pop3d/listener.go +++ b/pop3d/listener.go @@ -23,7 +23,10 @@ type Server struct { // New creates a new Server struct func New() *Server { - // TODO is two filestores better/worse than sharing w/ smtpd? + // Get a new instance of the the FileDataStore - the locking and counting + // mechanisms are both global variables in the smtpd package. If that + // changes in the future, this should be modified to use the same DataStore + // instance. ds := smtpd.DefaultFileDataStore() cfg := config.GetPOP3Config() return &Server{domain: cfg.Domain, dataStore: ds, maxIdleSeconds: cfg.MaxIdleSeconds, diff --git a/smtpd/datastore.go b/smtpd/datastore.go index d8dcd9b..ea1256d 100644 --- a/smtpd/datastore.go +++ b/smtpd/datastore.go @@ -9,8 +9,13 @@ import ( "github.com/jhillyerd/go.enmime" ) -// ErrNotExist indicates the requested message does not exist -var ErrNotExist = errors.New("Message does not exist") +var ( + // ErrNotExist indicates the requested message does not exist + ErrNotExist = errors.New("Message does not exist") + + // ErrNotWritable indicates the message is closed; no longer writable + ErrNotWritable = errors.New("Message not writable") +) // DataStore is an interface to get Mailboxes stored in Inbucket type DataStore interface { diff --git a/smtpd/filestore.go b/smtpd/filestore.go index 2fb9c5d..6b6862b 100644 --- a/smtpd/filestore.go +++ b/smtpd/filestore.go @@ -3,7 +3,6 @@ package smtpd import ( "bufio" "encoding/gob" - "errors" "fmt" "io" "io/ioutil" @@ -28,9 +27,6 @@ var ( // million index files indexLock = new(sync.RWMutex) - // TODO Consider moving this to the Message interface - errNotWritable = errors.New("Message not writable") - // 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 @@ -222,17 +218,15 @@ func (mb *FileMailbox) readIndex() error { // Decode gob data dec := gob.NewDecoder(bufio.NewReader(file)) for { - // TODO Detect EOF msg := new(FileMessage) if err = dec.Decode(msg); err != nil { if err == io.EOF { // It's OK to get an EOF here break } - return fmt.Errorf("While decoding message: %v", err) + return fmt.Errorf("Corrupt mailbox %q: %v", mb.indexPath, err) } msg.mailbox = mb - log.Tracef("Found: %v", msg) mb.messages = append(mb.messages, msg) } @@ -339,8 +333,7 @@ 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? +// Date returns the date/time this Message was received by Inbucket func (m *FileMessage) Date() time.Time { return m.Fdate } @@ -443,7 +436,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 { diff --git a/smtpd/handler.go b/smtpd/handler.go index 7ade826..7ffd58a 100644 --- a/smtpd/handler.go +++ b/smtpd/handler.go @@ -413,8 +413,11 @@ func (ss *Session) dataHandler() { for _, m := range messages { if m != nil { if err := m.Close(); err != nil { + // This logic should be updated to report failures + // writing the initial message file to the client + // after we implement a single-store system (issue + // #23) ss.logError("Error: %v while writing message", err) - // TODO Report to client? } expReceivedTotal.Add(1) } @@ -437,7 +440,7 @@ func (ss *Session) dataHandler() { ss.send("552 Maximum message size exceeded") ss.logWarn("Max message size exceeded while in DATA") ss.reset() - // TODO: Should really cleanup the crap on filesystem... + // Should really cleanup the crap on filesystem (after issue #23) return } // Append to message objects @@ -448,7 +451,7 @@ func (ss *Session) dataHandler() { ss.logError("Failed to append to mailbox %v: %v", mailboxes[i], err) ss.send("554 Something went wrong") ss.reset() - // TODO: Should really cleanup the crap on filesystem... + // Should really cleanup the crap on filesystem (after issue #23) return } } diff --git a/webui/mailbox_controller.go b/webui/mailbox_controller.go index bc62d75..717281e 100644 --- a/webui/mailbox_controller.go +++ b/webui/mailbox_controller.go @@ -251,7 +251,7 @@ func MailboxHTML(w http.ResponseWriter, req *http.Request, ctx *httpd.Context) ( "ctx": ctx, "name": name, "message": message, - // TODO: It is not really safe to render, need to sanitize. + // TODO It is not really safe to render, need to sanitize, issue #5 "body": template.HTML(mime.HTML), }) }