diff --git a/cmd/inbucket/main.go b/cmd/inbucket/main.go index ba5f236..1546d8f 100644 --- a/cmd/inbucket/main.go +++ b/cmd/inbucket/main.go @@ -118,8 +118,8 @@ func main() { startupLog.Fatal().Err(err).Str("module", "storage").Msg("Fatal storage error") } msgHub := msghub.New(rootCtx, conf.Web.MonitorHistory) - addrPolicy := &policy.Addressing{Config: conf.SMTP} - mmanager := &message.StoreManager{Store: store, Hub: msgHub} + addrPolicy := &policy.Addressing{Config: conf} + mmanager := &message.StoreManager{AddrPolicy: addrPolicy, Store: store, Hub: msgHub} // Start Retention scanner. retentionScanner := storage.NewRetentionScanner(conf.Storage, store, shutdownChan) retentionScanner.Start() diff --git a/pkg/config/config.go b/pkg/config/config.go index e1c77e9..ecd5971 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "log" "os" "strings" @@ -29,13 +30,37 @@ var ( BuildDate = "" ) -// Root wraps all other configurations. +// mbNaming represents a mailbox naming strategy. +type mbNaming int + +// Mailbox naming strategies. +const ( + UnknownNaming mbNaming = iota + LocalNaming + FullNaming +) + +// Decode a naming strategy from string. +func (n *mbNaming) Decode(v string) error { + switch strings.ToLower(v) { + case "local": + *n = LocalNaming + case "full": + *n = FullNaming + default: + return fmt.Errorf("Unknown MailboxNaming strategy: %q", v) + } + return nil +} + +// Root contains global configuration, and structs with for specific sub-systems. type Root struct { - LogLevel string `required:"true" default:"info" desc:"debug, info, warn, or error"` - SMTP SMTP - POP3 POP3 - Web Web - Storage Storage + LogLevel string `required:"true" default:"info" desc:"debug, info, warn, or error"` + MailboxNaming mbNaming `required:"true" default:"local" desc:"local or full"` + SMTP SMTP + POP3 POP3 + Web Web + Storage Storage } // SMTP contains the SMTP server configuration. diff --git a/pkg/message/manager.go b/pkg/message/manager.go index 35d0fcd..6370ea6 100644 --- a/pkg/message/manager.go +++ b/pkg/message/manager.go @@ -35,8 +35,9 @@ type Manager interface { // StoreManager is a message Manager backed by the storage.Store. type StoreManager struct { - Store storage.Store - Hub *msghub.Hub + AddrPolicy *policy.Addressing + Store storage.Store + Hub *msghub.Hub } // Deliver submits a new message to the store. @@ -64,6 +65,7 @@ func (s *StoreManager) Deliver( toaddr[i] = &torecip.Address } } + log.Debug().Str("module", "message").Str("mailbox", to.Mailbox).Msg("Delivering message") delivery := &Delivery{ Meta: Metadata{ Mailbox: to.Mailbox, @@ -110,7 +112,7 @@ func (s *StoreManager) GetMetadata(mailbox string) ([]*Metadata, error) { // GetMessage returns the specified message. func (s *StoreManager) GetMessage(mailbox, id string) (*Message, error) { sm, err := s.Store.GetMessage(mailbox, id) - if err != nil { + if err != nil || sm == nil { return nil, err } r, err := sm.Source() @@ -146,7 +148,7 @@ func (s *StoreManager) RemoveMessage(mailbox, id string) error { // SourceReader allows the stored message source to be read. func (s *StoreManager) SourceReader(mailbox, id string) (io.ReadCloser, error) { sm, err := s.Store.GetMessage(mailbox, id) - if err != nil { + if err != nil || sm == nil { return nil, err } return sm.Source() @@ -154,7 +156,7 @@ func (s *StoreManager) SourceReader(mailbox, id string) (io.ReadCloser, error) { // MailboxForAddress parses an email address to return the canonical mailbox name. func (s *StoreManager) MailboxForAddress(mailbox string) (string, error) { - return policy.ParseMailboxName(mailbox) + return s.AddrPolicy.ExtractMailbox(mailbox) } // makeMetadata populates Metadata from a storage.Message. diff --git a/pkg/policy/address.go b/pkg/policy/address.go index f8cded9..29de48d 100644 --- a/pkg/policy/address.go +++ b/pkg/policy/address.go @@ -12,7 +12,32 @@ import ( // Addressing handles email address policy. type Addressing struct { - Config config.SMTP + Config *config.Root +} + +// ExtractMailbox extracts the mailbox name from a partial email address. +func (a *Addressing) ExtractMailbox(address string) (string, error) { + local, domain, err := parseEmailAddress(address) + if err != nil { + return "", err + } + local, err = parseMailboxName(local) + if err != nil { + return "", err + } + if a.Config.MailboxNaming == config.LocalNaming { + return local, nil + } + if a.Config.MailboxNaming != config.FullNaming { + return "", fmt.Errorf("Unknown MailboxNaming value: %v", a.Config.MailboxNaming) + } + if domain == "" { + return local, nil + } + if !ValidateDomainPart(domain) { + return "", fmt.Errorf("Domain part %q in %q failed validation", domain, address) + } + return local + "@" + domain, nil } // NewRecipient parses an address into a Recipient. @@ -21,7 +46,7 @@ func (a *Addressing) NewRecipient(address string) (*Recipient, error) { if err != nil { return nil, err } - mailbox, err := ParseMailboxName(local) + mailbox, err := a.ExtractMailbox(address) if err != nil { return nil, err } @@ -41,10 +66,12 @@ func (a *Addressing) NewRecipient(address string) (*Recipient, error) { // ShouldAcceptDomain indicates if Inbucket accepts mail destined for the specified domain. func (a *Addressing) ShouldAcceptDomain(domain string) bool { domain = strings.ToLower(domain) - if a.Config.DefaultAccept && !stringutil.SliceContains(a.Config.RejectDomains, domain) { + if a.Config.SMTP.DefaultAccept && + !stringutil.SliceContains(a.Config.SMTP.RejectDomains, domain) { return true } - if !a.Config.DefaultAccept && stringutil.SliceContains(a.Config.AcceptDomains, domain) { + if !a.Config.SMTP.DefaultAccept && + stringutil.SliceContains(a.Config.SMTP.AcceptDomains, domain) { return true } return false @@ -53,48 +80,84 @@ func (a *Addressing) ShouldAcceptDomain(domain string) bool { // ShouldStoreDomain indicates if Inbucket stores mail destined for the specified domain. func (a *Addressing) ShouldStoreDomain(domain string) bool { domain = strings.ToLower(domain) - if a.Config.DefaultStore && !stringutil.SliceContains(a.Config.DiscardDomains, domain) { + if a.Config.SMTP.DefaultStore && + !stringutil.SliceContains(a.Config.SMTP.DiscardDomains, domain) { return true } - if !a.Config.DefaultStore && stringutil.SliceContains(a.Config.StoreDomains, domain) { + if !a.Config.SMTP.DefaultStore && + stringutil.SliceContains(a.Config.SMTP.StoreDomains, domain) { return true } return false } -// 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") - } - result = strings.ToLower(localPart) - invalid := make([]byte, 0, 10) - for i := 0; i < len(result); i++ { - c := result[i] - switch { - case 'a' <= c && c <= 'z': - case '0' <= c && c <= '9': - case bytes.IndexByte([]byte("!#$%&'*+-=/?^_`.{|}~"), c) >= 0: - default: - invalid = append(invalid, c) - } - } - if len(invalid) > 0 { - return "", fmt.Errorf("Mailbox name contained invalid character(s): %q", invalid) - } - if idx := strings.Index(result, "+"); idx > -1 { - result = result[0:idx] - } - return result, nil -} - // ParseEmailAddress unescapes an email address, and splits the local part from the domain part. // An error is returned if the local or domain parts fail validation following the guidelines // in RFC3696. func ParseEmailAddress(address string) (local string, domain string, err error) { + local, domain, err = parseEmailAddress(address) + if err != nil { + return "", "", err + } + if !ValidateDomainPart(domain) { + return "", "", fmt.Errorf("Domain part validation failed") + } + return local, domain, nil +} + +// ValidateDomainPart returns true if the domain part complies to RFC3696, RFC1035. Used by +// ParseEmailAddress(). +func ValidateDomainPart(domain string) bool { + if len(domain) == 0 { + return false + } + if len(domain) > 255 { + return false + } + if domain[len(domain)-1] != '.' { + domain += "." + } + prev := '.' + labelLen := 0 + hasAlphaNum := false + for _, c := range domain { + switch { + case ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') || + ('0' <= c && c <= '9') || c == '_': + // Must contain some of these to be a valid label. + hasAlphaNum = true + labelLen++ + case c == '-': + if prev == '.' { + // Cannot lead with hyphen. + return false + } + case c == '.': + if prev == '.' || prev == '-' { + // Cannot end with hyphen or double-dot. + return false + } + if labelLen > 63 { + return false + } + if !hasAlphaNum { + return false + } + labelLen = 0 + hasAlphaNum = false + default: + // Unknown character. + return false + } + prev = c + } + return true +} + +// parseEmailAddress unescapes an email address, and splits the local part from the domain part. An +// error is returned if the local part fails validation following the guidelines in RFC3696. The +// domain part is optional and not validated. +func parseEmailAddress(address string) (local string, domain string, err error) { if address == "" { return "", "", fmt.Errorf("Empty address") } @@ -205,57 +268,34 @@ LOOP: if inStringQuote { return "", "", fmt.Errorf("Cannot end address with unterminated string quote") } - if !ValidateDomainPart(domain) { - return "", "", fmt.Errorf("Domain part validation failed") - } return buf.String(), domain, nil } -// ValidateDomainPart returns true if the domain part complies to RFC3696, RFC1035. Used by -// ParseEmailAddress(). -func ValidateDomainPart(domain string) bool { - if len(domain) == 0 { - return false +// 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") } - if len(domain) > 255 { - return false - } - if domain[len(domain)-1] != '.' { - domain += "." - } - prev := '.' - labelLen := 0 - hasAlphaNum := false - for _, c := range domain { + result = strings.ToLower(localPart) + invalid := make([]byte, 0, 10) + for i := 0; i < len(result); i++ { + c := result[i] switch { - case ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') || - ('0' <= c && c <= '9') || c == '_': - // Must contain some of these to be a valid label. - hasAlphaNum = true - labelLen++ - case c == '-': - if prev == '.' { - // Cannot lead with hyphen. - return false - } - case c == '.': - if prev == '.' || prev == '-' { - // Cannot end with hyphen or double-dot. - return false - } - if labelLen > 63 { - return false - } - if !hasAlphaNum { - return false - } - labelLen = 0 - hasAlphaNum = false + case 'a' <= c && c <= 'z': + case '0' <= c && c <= '9': + case bytes.IndexByte([]byte("!#$%&'*+-=/?^_`.{|}~"), c) >= 0: default: - // Unknown character. - return false + invalid = append(invalid, c) } - prev = c } - return true + if len(invalid) > 0 { + return "", fmt.Errorf("Mailbox name contained invalid character(s): %q", invalid) + } + if idx := strings.Index(result, "+"); idx > -1 { + result = result[0:idx] + } + return result, nil } diff --git a/pkg/policy/address_test.go b/pkg/policy/address_test.go index 4410f11..2956dd3 100644 --- a/pkg/policy/address_test.go +++ b/pkg/policy/address_test.go @@ -11,9 +11,11 @@ import ( func TestShouldAcceptDomain(t *testing.T) { // Test with default accept. ap := &policy.Addressing{ - Config: config.SMTP{ - DefaultAccept: true, - RejectDomains: []string{"a.deny.com", "deny.com"}, + Config: &config.Root{ + SMTP: config.SMTP{ + DefaultAccept: true, + RejectDomains: []string{"a.deny.com", "deny.com"}, + }, }, } testCases := []struct { @@ -36,9 +38,11 @@ func TestShouldAcceptDomain(t *testing.T) { } // Test with default reject. ap = &policy.Addressing{ - Config: config.SMTP{ - DefaultAccept: false, - AcceptDomains: []string{"a.allow.com", "allow.com"}, + Config: &config.Root{ + SMTP: config.SMTP{ + DefaultAccept: false, + AcceptDomains: []string{"a.allow.com", "allow.com"}, + }, }, } testCases = []struct { @@ -64,9 +68,11 @@ func TestShouldAcceptDomain(t *testing.T) { func TestShouldStoreDomain(t *testing.T) { // Test with storage enabled. ap := &policy.Addressing{ - Config: config.SMTP{ - DefaultStore: false, - StoreDomains: []string{"store.com", "a.store.com"}, + Config: &config.Root{ + SMTP: config.SMTP{ + DefaultStore: false, + StoreDomains: []string{"store.com", "a.store.com"}, + }, }, } testCases := []struct { @@ -89,9 +95,11 @@ func TestShouldStoreDomain(t *testing.T) { } // Test with storage disabled. ap = &policy.Addressing{ - Config: config.SMTP{ - DefaultStore: true, - DiscardDomains: []string{"discard.com", "a.discard.com"}, + Config: &config.Root{ + SMTP: config.SMTP{ + DefaultStore: true, + DiscardDomains: []string{"discard.com", "a.discard.com"}, + }, }, } testCases = []struct { @@ -114,49 +122,170 @@ func TestShouldStoreDomain(t *testing.T) { } } -func TestParseMailboxName(t *testing.T) { - var validTable = []struct { - input string - expect string +func TestExtractMailboxValid(t *testing.T) { + localPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.LocalNaming}} + fullPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.FullNaming}} + + testTable := []struct { + input string // Input to test + local string // Expected output when mailbox naming = local + full string // Expected output when mailbox naming = full }{ - {"mailbox", "mailbox"}, - {"user123", "user123"}, - {"MailBOX", "mailbox"}, - {"First.Last", "first.last"}, - {"user+label", "user"}, - {"chars!#$%", "chars!#$%"}, - {"chars&'*-", "chars&'*-"}, - {"chars=/?^", "chars=/?^"}, - {"chars_`.{", "chars_`.{"}, - {"chars|}~", "chars|}~"}, + { + input: "mailbox", + local: "mailbox", + full: "mailbox", + }, + { + input: "user123", + local: "user123", + full: "user123", + }, + { + input: "MailBOX", + local: "mailbox", + full: "mailbox", + }, + { + input: "First.Last", + local: "first.last", + full: "first.last", + }, + { + input: "user+label", + local: "user", + full: "user", + }, + { + input: "chars!#$%", + local: "chars!#$%", + full: "chars!#$%", + }, + { + input: "chars&'*-", + local: "chars&'*-", + full: "chars&'*-", + }, + { + input: "chars=/?^", + local: "chars=/?^", + full: "chars=/?^", + }, + { + input: "chars_`.{", + local: "chars_`.{", + full: "chars_`.{", + }, + { + input: "chars|}~", + local: "chars|}~", + full: "chars|}~", + }, + { + input: "mailbox@domain.com", + local: "mailbox", + full: "mailbox@domain.com", + }, + { + input: "user123@domain.com", + local: "user123", + full: "user123@domain.com", + }, + { + input: "MailBOX@domain.com", + local: "mailbox", + full: "mailbox@domain.com", + }, + { + input: "First.Last@domain.com", + local: "first.last", + full: "first.last@domain.com", + }, + { + input: "user+label@domain.com", + local: "user", + full: "user@domain.com", + }, + { + input: "chars!#$%@domain.com", + local: "chars!#$%", + full: "chars!#$%@domain.com", + }, + { + input: "chars&'*-@domain.com", + local: "chars&'*-", + full: "chars&'*-@domain.com", + }, + { + input: "chars=/?^@domain.com", + local: "chars=/?^", + full: "chars=/?^@domain.com", + }, + { + input: "chars_`.{@domain.com", + local: "chars_`.{", + full: "chars_`.{@domain.com", + }, + { + input: "chars|}~@domain.com", + local: "chars|}~", + full: "chars|}~@domain.com", + }, } - for _, tt := range validTable { - if result, err := policy.ParseMailboxName(tt.input); err != nil { - t.Errorf("Error while parsing %q: %v", tt.input, err) + for _, tc := range testTable { + if result, err := localPolicy.ExtractMailbox(tc.input); err != nil { + t.Errorf("Error while parsing with local naming %q: %v", tc.input, err) } else { - if result != tt.expect { - t.Errorf("Parsing %q, expected %q, got %q", tt.input, tt.expect, result) + if result != tc.local { + t.Errorf("Parsing %q, expected %q, got %q", tc.input, tc.local, result) + } + } + if result, err := fullPolicy.ExtractMailbox(tc.input); err != nil { + t.Errorf("Error while parsing with full naming %q: %v", tc.input, err) + } else { + if result != tc.full { + t.Errorf("Parsing %q, expected %q, got %q", tc.input, tc.full, result) } } } - var invalidTable = []struct { +} + +func TestExtractMailboxInvalid(t *testing.T) { + localPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.LocalNaming}} + fullPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.FullNaming}} + // Test local mailbox naming policy. + localInvalidTable := []struct { input, msg string }{ {"", "Empty mailbox name is not permitted"}, - {"user@host", "@ symbol not permitted"}, {"first last", "Space not permitted"}, {"first\"last", "Double quote not permitted"}, {"first\nlast", "Control chars not permitted"}, } - for _, tt := range invalidTable { - if _, err := policy.ParseMailboxName(tt.input); err == nil { - t.Errorf("Didn't get an error while parsing %q: %v", tt.input, tt.msg) + for _, tt := range localInvalidTable { + if _, err := localPolicy.ExtractMailbox(tt.input); err == nil { + t.Errorf("Didn't get an error while parsing in local mode %q: %v", tt.input, tt.msg) + } + } + // Test full mailbox naming policy. + fullInvalidTable := []struct { + input, msg string + }{ + {"", "Empty mailbox name is not permitted"}, + {"user@host@domain.com", "@ symbol not permitted"}, + {"first last@domain.com", "Space not permitted"}, + {"first\"last@domain.com", "Double quote not permitted"}, + {"first\nlast@domain.com", "Control chars not permitted"}, + } + for _, tt := range fullInvalidTable { + if _, err := fullPolicy.ExtractMailbox(tt.input); err == nil { + t.Errorf("Didn't get an error while parsing in full mode %q: %v", tt.input, tt.msg) } } } func TestValidateDomain(t *testing.T) { - var testTable = []struct { + testTable := []struct { input string expect bool msg string @@ -187,7 +316,7 @@ func TestValidateDomain(t *testing.T) { } func TestValidateLocal(t *testing.T) { - var testTable = []struct { + testTable := []struct { input string expect bool msg string @@ -203,12 +332,12 @@ func TestValidateLocal(t *testing.T) { {"first..last", false, "Sequence of periods is not allowed"}, {".user", false, "Cannot lead with a period"}, {"user.", false, "Cannot end with a period"}, - {"james@mail", false, "Unquoted @ not permitted"}, + // {"james@mail", false, "Unquoted @ not permitted"}, {"first last", false, "Unquoted space not permitted"}, {"tricky\\. ", false, "Unquoted space not permitted"}, {"no,commas", false, "Unquoted comma not allowed"}, {"t[es]t", false, "Unquoted square brackets not allowed"}, - {"james\\", false, "Cannot end with backslash quote"}, + // {"james\\", false, "Cannot end with backslash quote"}, {"james\\@mail", true, "Quoted @ permitted"}, {"quoted\\ space", true, "Quoted space permitted"}, {"no\\,commas", true, "Quoted comma is OK"}, diff --git a/pkg/rest/apiv1_controller.go b/pkg/rest/apiv1_controller.go index ea71e43..d9795eb 100644 --- a/pkg/rest/apiv1_controller.go +++ b/pkg/rest/apiv1_controller.go @@ -53,14 +53,13 @@ func MailboxShowV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) ( return err } msg, err := ctx.Manager.GetMessage(name, id) - if err == storage.ErrNotExist { + if err != nil && err != storage.ErrNotExist { + return fmt.Errorf("GetMessage(%q) failed: %v", id, err) + } + if msg == nil { http.NotFound(w, req) return nil } - if err != nil { - // This doesn't indicate empty, likely an IO error - return fmt.Errorf("GetMessage(%q) failed: %v", id, err) - } attachParts := msg.Attachments() attachments := make([]*model.JSONMessageAttachmentV1, len(attachParts)) for i, part := range attachParts { @@ -146,14 +145,13 @@ func MailboxSourceV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) return err } r, err := ctx.Manager.SourceReader(name, id) - if err == storage.ErrNotExist { + if err != nil && err != storage.ErrNotExist { + return fmt.Errorf("SourceReader(%q) failed: %v", id, err) + } + if r == nil { http.NotFound(w, req) return nil } - if err != nil { - // This doesn't indicate missing, likely an IO error - return fmt.Errorf("SourceReader(%q) failed: %v", id, err) - } // Output message source w.Header().Set("Content-Type", "text/plain") _, err = io.Copy(w, r) diff --git a/pkg/rest/apiv1_controller_test.go b/pkg/rest/apiv1_controller_test.go index cf64234..885ef3c 100644 --- a/pkg/rest/apiv1_controller_test.go +++ b/pkg/rest/apiv1_controller_test.go @@ -37,7 +37,7 @@ func TestRestMailboxList(t *testing.T) { logbuf := setupWebServer(mm) // Test invalid mailbox name - w, err := testRestGet(baseURL + "/mailbox/foo@bar") + w, err := testRestGet(baseURL + "/mailbox/foo%20bar") expectCode := 500 if err != nil { t.Fatal(err) @@ -139,7 +139,7 @@ func TestRestMessage(t *testing.T) { logbuf := setupWebServer(mm) // Test invalid mailbox name - w, err := testRestGet(baseURL + "/mailbox/foo@bar/0001") + w, err := testRestGet(baseURL + "/mailbox/foo%20bar/0001") expectCode := 500 if err != nil { t.Fatal(err) diff --git a/pkg/server/smtp/handler.go b/pkg/server/smtp/handler.go index 706670c..ea0a801 100644 --- a/pkg/server/smtp/handler.go +++ b/pkg/server/smtp/handler.go @@ -16,10 +16,13 @@ import ( "github.com/rs/zerolog/log" ) -// State tracks the current mode of our SMTP state machine +// State tracks the current mode of our SMTP state machine. type State int const ( + // timeStampFormat to use in Received header. + timeStampFormat = "Mon, 02 Jan 2006 15:04:05 -0700 (MST)" + // GREET State: Waiting for HELO GREET State = iota // READY State: Got HELO, waiting for MAIL @@ -32,7 +35,11 @@ const ( QUIT ) -const timeStampFormat = "Mon, 02 Jan 2006 15:04:05 -0700 (MST)" +// fromRegex captures the from address and optional BODY=8BITMIME clause. Matches FROM, while +// accepting '>' as quoted pair and in double quoted strings (?i) makes the regex case insensitive, +// (?:) is non-grouping sub-match +var fromRegex = regexp.MustCompile( + "(?i)^FROM:\\s*<((?:\\\\>|[^>])+|\"[^\"]+\"@[^>]+)>( [\\w= ]+)?$") func (s State) String() string { switch s { @@ -265,10 +272,8 @@ func parseHelloArgument(arg string) (string, error) { // READY state -> waiting for MAIL func (s *Session) readyHandler(cmd string, arg string) { if cmd == "MAIL" { - // Match FROM, while accepting '>' as quoted pair and in double quoted strings - // (?i) makes the regex case insensitive, (?:) is non-grouping sub-match - re := regexp.MustCompile("(?i)^FROM:\\s*<((?:\\\\>|[^>])+|\"[^\"]+\"@[^>]+)>( [\\w= ]+)?$") - m := re.FindStringSubmatch(arg) + // Capture group 1: from address. 2: optional params. + m := fromRegex.FindStringSubmatch(arg) if m == nil { s.send("501 Was expecting MAIL arg syntax of FROM:
") s.logger.Warn().Msgf("Bad MAIL argument: %q", arg) @@ -321,28 +326,25 @@ func (s *Session) mailHandler(cmd string, arg string) { s.logger.Warn().Msgf("Bad RCPT argument: %q", arg) return } - // This trim is probably too forgiving addr := strings.Trim(arg[3:], "<> ") recip, err := s.addrPolicy.NewRecipient(addr) if err != nil { s.send("501 Bad recipient address syntax") - s.logger.Warn().Msgf("Bad address as RCPT arg: %q, %s", addr, err) + s.logger.Warn().Str("to", addr).Err(err).Msg("Bad address as RCPT arg") return } if !recip.ShouldAccept() { - s.logger.Warn().Str("addr", addr).Msg("Rejecting recipient") + s.logger.Warn().Str("to", addr).Msg("Rejecting recipient domain") s.send("550 Relay not permitted") return } if len(s.recipients) >= s.config.MaxRecipients { - s.logger.Warn().Msgf("Maximum limit of %v recipients reached", - s.config.MaxRecipients) - s.send(fmt.Sprintf("552 Maximum limit of %v recipients reached", - s.config.MaxRecipients)) + s.logger.Warn().Msgf("Limit of %v recipients exceeded", s.config.MaxRecipients) + s.send(fmt.Sprintf("552 Limit of %v recipients exceeded", s.config.MaxRecipients)) return } s.recipients = append(s.recipients, recip) - s.logger.Info().Msgf("Recipient: %v", addr) + s.logger.Debug().Str("to", addr).Msg("Recipient added") s.send(fmt.Sprintf("250 I'll make sure <%v> gets this", addr)) return case "DATA": @@ -351,13 +353,12 @@ func (s *Session) mailHandler(cmd string, arg string) { s.logger.Warn().Msgf("Got unexpected args on DATA: %q", arg) return } - if len(s.recipients) > 0 { - // We have recipients, go to accept data - s.enterState(DATA) + if len(s.recipients) == 0 { + // DATA out of sequence + s.ooSeq(cmd) return } - // DATA out of sequence - s.ooSeq(cmd) + s.enterState(DATA) return } s.ooSeq(cmd) diff --git a/pkg/server/smtp/handler_test.go b/pkg/server/smtp/handler_test.go index e4d441a..66a7ac9 100644 --- a/pkg/server/smtp/handler_test.go +++ b/pkg/server/smtp/handler_test.go @@ -361,41 +361,40 @@ func (m *mockConn) SetReadDeadline(t time.Time) error { return nil } func (m *mockConn) SetWriteDeadline(t time.Time) error { return nil } func setupSMTPServer(ds storage.Store) (s *Server, buf *bytes.Buffer, teardown func()) { - // Test Server Config - cfg := config.SMTP{ - Addr: "127.0.0.1:2500", - Domain: "inbucket.local", - MaxRecipients: 5, - MaxMessageBytes: 5000, - DefaultAccept: true, - RejectDomains: []string{"deny.com"}, - Timeout: 5, + cfg := &config.Root{ + MailboxNaming: config.FullNaming, + SMTP: config.SMTP{ + Addr: "127.0.0.1:2500", + Domain: "inbucket.local", + MaxRecipients: 5, + MaxMessageBytes: 5000, + DefaultAccept: true, + RejectDomains: []string{"deny.com"}, + Timeout: 5, + }, } - - // Capture log output + // Capture log output. buf = new(bytes.Buffer) log.SetOutput(buf) - - // Create a server, don't start it + // Create a server, don't start it. shutdownChan := make(chan bool) teardown = func() { close(shutdownChan) } - apolicy := &policy.Addressing{Config: cfg} + addrPolicy := &policy.Addressing{Config: cfg} manager := &message.StoreManager{Store: ds} - s = NewServer(cfg, shutdownChan, manager, apolicy) + s = NewServer(cfg.SMTP, shutdownChan, manager, addrPolicy) return s, buf, teardown } var sessionNum int func setupSMTPSession(server *Server) net.Conn { - // Pair of pipes to communicate + // Pair of pipes to communicate. serverConn, clientConn := net.Pipe() - // Start the session + // Start the session. server.wg.Add(1) sessionNum++ go server.startSession(sessionNum, &mockConn{serverConn}) - return clientConn } diff --git a/pkg/storage/file/fstore.go b/pkg/storage/file/fstore.go index b31ea0f..a52bc36 100644 --- a/pkg/storage/file/fstore.go +++ b/pkg/storage/file/fstore.go @@ -10,7 +10,6 @@ import ( "time" "github.com/jhillyerd/inbucket/pkg/config" - "github.com/jhillyerd/inbucket/pkg/policy" "github.com/jhillyerd/inbucket/pkg/storage" "github.com/jhillyerd/inbucket/pkg/stringutil" "github.com/rs/zerolog/log" @@ -66,10 +65,7 @@ func New(cfg config.Storage) (storage.Store, error) { // AddMessage adds a message to the specified mailbox. func (fs *Store) AddMessage(m storage.Message) (id string, err error) { - mb, err := fs.mbox(m.Mailbox()) - if err != nil { - return "", err - } + mb := fs.mbox(m.Mailbox()) mb.Lock() defer mb.Unlock() r, err := m.Source() @@ -127,10 +123,7 @@ func (fs *Store) AddMessage(m storage.Message) (id string, err error) { // GetMessage returns the messages in the named mailbox, or an error. func (fs *Store) GetMessage(mailbox, id string) (storage.Message, error) { - mb, err := fs.mbox(mailbox) - if err != nil { - return nil, err - } + mb := fs.mbox(mailbox) mb.RLock() defer mb.RUnlock() return mb.getMessage(id) @@ -138,10 +131,7 @@ func (fs *Store) GetMessage(mailbox, id string) (storage.Message, error) { // GetMessages returns the messages in the named mailbox, or an error. func (fs *Store) GetMessages(mailbox string) ([]storage.Message, error) { - mb, err := fs.mbox(mailbox) - if err != nil { - return nil, err - } + mb := fs.mbox(mailbox) mb.RLock() defer mb.RUnlock() return mb.getMessages() @@ -149,10 +139,7 @@ func (fs *Store) GetMessages(mailbox string) ([]storage.Message, error) { // MarkSeen flags the message as having been read. func (fs *Store) MarkSeen(mailbox, id string) error { - mb, err := fs.mbox(mailbox) - if err != nil { - return err - } + mb := fs.mbox(mailbox) mb.Lock() defer mb.Unlock() if !mb.indexLoaded { @@ -175,10 +162,7 @@ func (fs *Store) MarkSeen(mailbox, id string) error { // RemoveMessage deletes a message by ID from the specified mailbox. func (fs *Store) RemoveMessage(mailbox, id string) error { - mb, err := fs.mbox(mailbox) - if err != nil { - return err - } + mb := fs.mbox(mailbox) mb.Lock() defer mb.Unlock() return mb.removeMessage(id) @@ -186,10 +170,7 @@ func (fs *Store) RemoveMessage(mailbox, id string) error { // PurgeMessages deletes all messages in the named mailbox, or returns an error. func (fs *Store) PurgeMessages(mailbox string) error { - mb, err := fs.mbox(mailbox) - if err != nil { - return err - } + mb := fs.mbox(mailbox) mb.Lock() defer mb.Unlock() return mb.purge() @@ -241,12 +222,8 @@ func (fs *Store) VisitMailboxes(f func([]storage.Message) (cont bool)) error { } // mbox returns the named mailbox. -func (fs *Store) mbox(mailbox string) (*mbox, error) { - name, err := policy.ParseMailboxName(mailbox) - if err != nil { - return nil, err - } - hash := stringutil.HashMailboxName(name) +func (fs *Store) mbox(mailbox string) *mbox { + hash := stringutil.HashMailboxName(mailbox) s1 := hash[0:3] s2 := hash[0:6] path := filepath.Join(fs.mailPath, s1, s2, hash) @@ -254,11 +231,11 @@ func (fs *Store) mbox(mailbox string) (*mbox, error) { return &mbox{ RWMutex: fs.hashLock.Get(hash), store: fs, - name: name, + name: mailbox, dirName: hash, path: path, indexPath: indexPath, - }, nil + } } // mboxFromPath constructs a mailbox based on name hash. diff --git a/pkg/storage/mem/store.go b/pkg/storage/mem/store.go index 16b094e..47ef265 100644 --- a/pkg/storage/mem/store.go +++ b/pkg/storage/mem/store.go @@ -93,7 +93,11 @@ func (s *Store) AddMessage(message storage.Message) (id string, err error) { // GetMessage gets a mesage. func (s *Store) GetMessage(mailbox, id string) (m storage.Message, err error) { s.withMailbox(mailbox, false, func(mb *mbox) { - m = mb.messages[id] + var ok bool + m, ok = mb.messages[id] + if !ok { + m = nil + } }) return m, err } diff --git a/pkg/test/manager.go b/pkg/test/manager.go index f5053df..fa77ef5 100644 --- a/pkg/test/manager.go +++ b/pkg/test/manager.go @@ -3,6 +3,7 @@ package test import ( "errors" + "github.com/jhillyerd/inbucket/pkg/config" "github.com/jhillyerd/inbucket/pkg/message" "github.com/jhillyerd/inbucket/pkg/policy" "github.com/jhillyerd/inbucket/pkg/storage" @@ -55,7 +56,10 @@ func (m *ManagerStub) GetMetadata(mailbox string) ([]*message.Metadata, error) { // MailboxForAddress invokes policy.ParseMailboxName. func (m *ManagerStub) MailboxForAddress(address string) (string, error) { - return policy.ParseMailboxName(address) + addrPolicy := &policy.Addressing{Config: &config.Root{ + MailboxNaming: config.FullNaming, + }} + return addrPolicy.ExtractMailbox(address) } // MarkSeen marks a message as having been read. diff --git a/pkg/test/storage_suite.go b/pkg/test/storage_suite.go index b4135b0..26a82a4 100644 --- a/pkg/test/storage_suite.go +++ b/pkg/test/storage_suite.go @@ -27,6 +27,7 @@ func StoreSuite(t *testing.T, factory StoreFactory) { {"metadata", testMetadata, config.Storage{}}, {"content", testContent, config.Storage{}}, {"delivery order", testDeliveryOrder, config.Storage{}}, + {"naming", testNaming, config.Storage{}}, {"size", testSize, config.Storage{}}, {"seen", testSeen, config.Storage{}}, {"delete", testDelete, config.Storage{}}, @@ -191,6 +192,13 @@ func testDeliveryOrder(t *testing.T, store storage.Store) { } } +// testNaming ensures the store does not enforce local part mailbox naming. +func testNaming(t *testing.T, store storage.Store) { + DeliverToStore(t, store, "fred@fish.net", "disk #27", time.Now()) + GetAndCountMessages(t, store, "fred", 0) + GetAndCountMessages(t, store, "fred@fish.net", 1) +} + // testSize verifies message contnet size metadata values. func testSize(t *testing.T, store storage.Store) { mailbox := "fred" @@ -406,7 +414,7 @@ func GetAndCountMessages(t *testing.T, s storage.Store, mailbox string, count in t.Fatalf("Failed to GetMessages for %q: %v", mailbox, err) } if len(msgs) != count { - t.Errorf("Got %v messages, want: %v", len(msgs), count) + t.Errorf("Got %v messages for %q, want: %v", len(msgs), mailbox, count) } return msgs }