From d132efd6fadc73a38ae947330cb6acde8b988a72 Mon Sep 17 00:00:00 2001 From: James Hillyerd Date: Sat, 17 Mar 2018 09:18:28 -0700 Subject: [PATCH] policy: Create new policy package for #84 --- pkg/policy/address.go | 204 +++++++++++++++++++++++++++++ pkg/policy/address_test.go | 138 ++++++++++++++++++++ pkg/rest/apiv1_controller.go | 11 +- pkg/rest/socketv1_controller.go | 4 +- pkg/server/smtp/handler.go | 9 +- pkg/storage/file/fstore.go | 5 +- pkg/stringutil/utils.go | 208 ------------------------------ pkg/stringutil/utils_test.go | 220 +++----------------------------- pkg/webui/mailbox_controller.go | 18 +-- pkg/webui/root_controller.go | 4 +- 10 files changed, 388 insertions(+), 433 deletions(-) create mode 100644 pkg/policy/address.go create mode 100644 pkg/policy/address_test.go diff --git a/pkg/policy/address.go b/pkg/policy/address.go new file mode 100644 index 0000000..7cc85f0 --- /dev/null +++ b/pkg/policy/address.go @@ -0,0 +1,204 @@ +package policy + +import ( + "bytes" + "fmt" + "strings" +) + +// 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 +} + +// ValidateDomainPart returns true if the domain part complies to RFC3696, RFC1035 +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 or domain parts fail validation following the guidelines +// in RFC3696. +func ParseEmailAddress(address string) (local string, domain string, err error) { + if address == "" { + return "", "", fmt.Errorf("Empty address") + } + if len(address) > 320 { + return "", "", fmt.Errorf("Address exceeds 320 characters") + } + if address[0] == '@' { + return "", "", fmt.Errorf("Address cannot start with @ symbol") + } + if address[0] == '.' { + return "", "", fmt.Errorf("Address cannot start with a period") + } + // Loop over address parsing out local part. + buf := new(bytes.Buffer) + prev := byte('.') + inCharQuote := false + inStringQuote := false +LOOP: + for i := 0; i < len(address); i++ { + c := address[i] + switch { + case ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'): + // Letters are OK. + err = buf.WriteByte(c) + if err != nil { + return + } + inCharQuote = false + case '0' <= c && c <= '9': + // Numbers are OK. + err = buf.WriteByte(c) + if err != nil { + return + } + inCharQuote = false + case bytes.IndexByte([]byte("!#$%&'*+-/=?^_`{|}~"), c) >= 0: + // These specials can be used unquoted. + err = buf.WriteByte(c) + if err != nil { + return + } + inCharQuote = false + case c == '.': + // A single period is OK. + if prev == '.' { + // Sequence of periods is not permitted. + return "", "", fmt.Errorf("Sequence of periods is not permitted") + } + err = buf.WriteByte(c) + if err != nil { + return + } + inCharQuote = false + case c == '\\': + inCharQuote = true + case c == '"': + if inCharQuote { + err = buf.WriteByte(c) + if err != nil { + return + } + inCharQuote = false + } else if inStringQuote { + inStringQuote = false + } else { + if i == 0 { + inStringQuote = true + } else { + return "", "", fmt.Errorf("Quoted string can only begin at start of address") + } + } + case c == '@': + if inCharQuote || inStringQuote { + err = buf.WriteByte(c) + if err != nil { + return + } + inCharQuote = false + } else { + // End of local-part. + if i > 128 { + return "", "", fmt.Errorf("Local part must not exceed 128 characters") + } + if prev == '.' { + return "", "", fmt.Errorf("Local part cannot end with a period") + } + domain = address[i+1:] + break LOOP + } + case c > 127: + return "", "", fmt.Errorf("Characters outside of US-ASCII range not permitted") + default: + if inCharQuote || inStringQuote { + err = buf.WriteByte(c) + if err != nil { + return + } + inCharQuote = false + } else { + return "", "", fmt.Errorf("Character %q must be quoted", c) + } + } + prev = c + } + if inCharQuote { + return "", "", fmt.Errorf("Cannot end address with unterminated quoted-pair") + } + 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 +} diff --git a/pkg/policy/address_test.go b/pkg/policy/address_test.go new file mode 100644 index 0000000..149e9b3 --- /dev/null +++ b/pkg/policy/address_test.go @@ -0,0 +1,138 @@ +package policy_test + +import ( + "strings" + "testing" + + "github.com/jhillyerd/inbucket/pkg/policy" +) + +func TestParseMailboxName(t *testing.T) { + var validTable = []struct { + input string + expect string + }{ + {"mailbox", "mailbox"}, + {"user123", "user123"}, + {"MailBOX", "mailbox"}, + {"First.Last", "first.last"}, + {"user+label", "user"}, + {"chars!#$%", "chars!#$%"}, + {"chars&'*-", "chars&'*-"}, + {"chars=/?^", "chars=/?^"}, + {"chars_`.{", "chars_`.{"}, + {"chars|}~", "chars|}~"}, + } + for _, tt := range validTable { + if result, err := policy.ParseMailboxName(tt.input); err != nil { + t.Errorf("Error while parsing %q: %v", tt.input, err) + } else { + if result != tt.expect { + t.Errorf("Parsing %q, expected %q, got %q", tt.input, tt.expect, result) + } + } + } + var invalidTable = []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) + } + } +} + +func TestValidateDomain(t *testing.T) { + var testTable = []struct { + input string + expect bool + msg string + }{ + {"", false, "Empty domain is not valid"}, + {"hostname", true, "Just a hostname is valid"}, + {"github.com", true, "Two labels should be just fine"}, + {"my-domain.com", true, "Hyphen is allowed mid-label"}, + {"_domainkey.foo.com", true, "Underscores are allowed"}, + {"bar.com.", true, "Must be able to end with a dot"}, + {"ABC.6DBS.com", true, "Mixed case is OK"}, + {"mail.123.com", true, "Number only label valid"}, + {"123.com", true, "Number only label valid"}, + {"google..com", false, "Double dot not valid"}, + {".foo.com", false, "Cannot start with a dot"}, + {"google\r.com", false, "Special chars not allowed"}, + {"foo.-bar.com", false, "Label cannot start with hyphen"}, + {"foo-.bar.com", false, "Label cannot end with hyphen"}, + {strings.Repeat("a", 256), false, "Max domain length is 255"}, + {strings.Repeat("a", 63) + ".com", true, "Should allow 63 char domain label"}, + {strings.Repeat("a", 64) + ".com", false, "Max domain label length is 63"}, + } + for _, tt := range testTable { + if policy.ValidateDomainPart(tt.input) != tt.expect { + t.Errorf("Expected %v for %q: %s", tt.expect, tt.input, tt.msg) + } + } +} + +func TestValidateLocal(t *testing.T) { + var testTable = []struct { + input string + expect bool + msg string + }{ + {"", false, "Empty local is not valid"}, + {"a", true, "Single letter should be fine"}, + {strings.Repeat("a", 128), true, "Valid up to 128 characters"}, + {strings.Repeat("a", 129), false, "Only valid up to 128 characters"}, + {"FirstLast", true, "Mixed case permitted"}, + {"user123", true, "Numbers permitted"}, + {"a!#$%&'*+-/=?^_`{|}~", true, "Any of !#$%&'*+-/=?^_`{|}~ are permitted"}, + {"first.last", true, "Embedded period is permitted"}, + {"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"}, + {"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\\@mail", true, "Quoted @ permitted"}, + {"quoted\\ space", true, "Quoted space permitted"}, + {"no\\,commas", true, "Quoted comma is OK"}, + {"t\\[es\\]t", true, "Quoted brackets are OK"}, + {"user\\name", true, "Should be able to quote a-z"}, + {"USER\\NAME", true, "Should be able to quote A-Z"}, + {"user\\1", true, "Should be able to quote a digit"}, + {"one\\$\\|", true, "Should be able to quote plain specials"}, + {"return\\\r", true, "Should be able to quote ASCII control chars"}, + {"high\\\x80", false, "Should not accept > 7-bit quoted chars"}, + {"quote\\\"", true, "Quoted double quote is permitted"}, + {"\"james\"", true, "Quoted a-z is permitted"}, + {"\"first last\"", true, "Quoted space is permitted"}, + {"\"quoted@sign\"", true, "Quoted @ is allowed"}, + {"\"qp\\\"quote\"", true, "Quoted quote within quoted string is OK"}, + {"\"unterminated", false, "Quoted string must be terminated"}, + {"\"unterminated\\\"", false, "Quoted string must be terminated"}, + {"embed\"quote\"string", false, "Embedded quoted string is illegal"}, + {"user+mailbox", true, "RFC3696 test case should be valid"}, + {"customer/department=shipping", true, "RFC3696 test case should be valid"}, + {"$A12345", true, "RFC3696 test case should be valid"}, + {"!def!xyz%abc", true, "RFC3696 test case should be valid"}, + {"_somename", true, "RFC3696 test case should be valid"}, + } + for _, tt := range testTable { + _, _, err := policy.ParseEmailAddress(tt.input + "@domain.com") + if (err != nil) == tt.expect { + if err != nil { + t.Logf("Got error: %s", err) + } + t.Errorf("Expected %v for %q: %s", tt.expect, tt.input, tt.msg) + } + } +} diff --git a/pkg/rest/apiv1_controller.go b/pkg/rest/apiv1_controller.go index 7b40201..2b10eb0 100644 --- a/pkg/rest/apiv1_controller.go +++ b/pkg/rest/apiv1_controller.go @@ -11,6 +11,7 @@ import ( "strconv" "github.com/jhillyerd/inbucket/pkg/log" + "github.com/jhillyerd/inbucket/pkg/policy" "github.com/jhillyerd/inbucket/pkg/rest/model" "github.com/jhillyerd/inbucket/pkg/server/web" "github.com/jhillyerd/inbucket/pkg/storage" @@ -20,7 +21,7 @@ import ( // MailboxListV1 renders a list of messages in a mailbox func MailboxListV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -50,7 +51,7 @@ func MailboxListV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) ( func MailboxShowV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -100,7 +101,7 @@ func MailboxShowV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) ( // MailboxPurgeV1 deletes all messages from a mailbox func MailboxPurgeV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -118,7 +119,7 @@ func MailboxPurgeV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) func MailboxSourceV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -142,7 +143,7 @@ func MailboxSourceV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) func MailboxDeleteV1(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } diff --git a/pkg/rest/socketv1_controller.go b/pkg/rest/socketv1_controller.go index 5ad2dbf..7614ac1 100644 --- a/pkg/rest/socketv1_controller.go +++ b/pkg/rest/socketv1_controller.go @@ -7,9 +7,9 @@ import ( "github.com/gorilla/websocket" "github.com/jhillyerd/inbucket/pkg/log" "github.com/jhillyerd/inbucket/pkg/msghub" + "github.com/jhillyerd/inbucket/pkg/policy" "github.com/jhillyerd/inbucket/pkg/rest/model" "github.com/jhillyerd/inbucket/pkg/server/web" - "github.com/jhillyerd/inbucket/pkg/stringutil" ) const ( @@ -173,7 +173,7 @@ func MonitorAllMessagesV1( // notifies the client of messages received by a particular mailbox. func MonitorMailboxMessagesV1( w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } diff --git a/pkg/server/smtp/handler.go b/pkg/server/smtp/handler.go index d9dad39..e123e9f 100644 --- a/pkg/server/smtp/handler.go +++ b/pkg/server/smtp/handler.go @@ -16,6 +16,7 @@ import ( "github.com/jhillyerd/inbucket/pkg/log" "github.com/jhillyerd/inbucket/pkg/message" "github.com/jhillyerd/inbucket/pkg/msghub" + "github.com/jhillyerd/inbucket/pkg/policy" "github.com/jhillyerd/inbucket/pkg/stringutil" ) @@ -267,7 +268,7 @@ func (ss *Session) readyHandler(cmd string, arg string) { return } from := m[1] - if _, _, err := stringutil.ParseEmailAddress(from); err != nil { + if _, _, err := policy.ParseEmailAddress(from); err != nil { ss.send("501 Bad sender address syntax") ss.logWarn("Bad address as MAIL arg: %q, %s", from, err) return @@ -316,7 +317,7 @@ func (ss *Session) mailHandler(cmd string, arg string) { } // This trim is probably too forgiving recip := strings.Trim(arg[3:], "<> ") - if _, _, err := stringutil.ParseEmailAddress(recip); err != nil { + if _, _, err := policy.ParseEmailAddress(recip); err != nil { ss.send("501 Bad recipient address syntax") ss.logWarn("Bad address as RCPT arg: %q, %s", recip, err) return @@ -355,7 +356,7 @@ func (ss *Session) dataHandler() { if ss.server.storeMessages { for e := ss.recipients.Front(); e != nil; e = e.Next() { recip := e.Value.(string) - local, domain, err := stringutil.ParseEmailAddress(recip) + local, domain, err := policy.ParseEmailAddress(recip) if err != nil { ss.logError("Failed to parse address for %q", recip) ss.send(fmt.Sprintf("451 Failed to open mailbox for %v", recip)) @@ -436,7 +437,7 @@ func (ss *Session) dataHandler() { // deliverMessage creates and populates a new Message for the specified recipient func (ss *Session) deliverMessage(r recipientDetails, content []byte) (ok bool) { - name, err := stringutil.ParseMailboxName(r.localPart) + name, err := policy.ParseMailboxName(r.localPart) if err != nil { // This parse already succeeded when MailboxFor was called, shouldn't fail here. return false diff --git a/pkg/storage/file/fstore.go b/pkg/storage/file/fstore.go index b41b5d3..dc20b5d 100644 --- a/pkg/storage/file/fstore.go +++ b/pkg/storage/file/fstore.go @@ -13,6 +13,7 @@ import ( "github.com/jhillyerd/inbucket/pkg/config" "github.com/jhillyerd/inbucket/pkg/log" + "github.com/jhillyerd/inbucket/pkg/policy" "github.com/jhillyerd/inbucket/pkg/storage" "github.com/jhillyerd/inbucket/pkg/stringutil" ) @@ -218,7 +219,7 @@ func (fs *Store) VisitMailboxes(f func([]storage.StoreMessage) (cont bool)) erro // LockFor returns the RWMutex for this mailbox, or an error. func (fs *Store) LockFor(emailAddress string) (*sync.RWMutex, error) { - name, err := stringutil.ParseMailboxName(emailAddress) + name, err := policy.ParseMailboxName(emailAddress) if err != nil { return nil, err } @@ -237,7 +238,7 @@ func (fs *Store) NewMessage(mailbox string) (storage.StoreMessage, error) { // mbox returns the named mailbox. func (fs *Store) mbox(mailbox string) (*mbox, error) { - name, err := stringutil.ParseMailboxName(mailbox) + name, err := policy.ParseMailboxName(mailbox) if err != nil { return nil, err } diff --git a/pkg/stringutil/utils.go b/pkg/stringutil/utils.go index 193e552..699eff8 100644 --- a/pkg/stringutil/utils.go +++ b/pkg/stringutil/utils.go @@ -1,47 +1,12 @@ package stringutil import ( - "bytes" "crypto/sha1" "fmt" "io" "net/mail" - "strings" ) -// 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 -} - // HashMailboxName accepts a mailbox name and hashes it. filestore uses this as // the directory to house the mailbox func HashMailboxName(mailbox string) string { @@ -53,179 +18,6 @@ func HashMailboxName(mailbox string) string { return fmt.Sprintf("%x", h.Sum(nil)) } -// ValidateDomainPart returns true if the domain part complies to RFC3696, RFC1035 -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 or domain parts fail validation following the guidelines -// in RFC3696. -func ParseEmailAddress(address string) (local string, domain string, err error) { - if address == "" { - return "", "", fmt.Errorf("Empty address") - } - if len(address) > 320 { - return "", "", fmt.Errorf("Address exceeds 320 characters") - } - if address[0] == '@' { - return "", "", fmt.Errorf("Address cannot start with @ symbol") - } - if address[0] == '.' { - return "", "", fmt.Errorf("Address cannot start with a period") - } - - // Loop over address parsing out local part - buf := new(bytes.Buffer) - prev := byte('.') - inCharQuote := false - inStringQuote := false -LOOP: - for i := 0; i < len(address); i++ { - c := address[i] - switch { - case ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'): - // Letters are OK - err = buf.WriteByte(c) - if err != nil { - return - } - inCharQuote = false - case '0' <= c && c <= '9': - // Numbers are OK - err = buf.WriteByte(c) - if err != nil { - return - } - inCharQuote = false - case bytes.IndexByte([]byte("!#$%&'*+-/=?^_`{|}~"), c) >= 0: - // These specials can be used unquoted - err = buf.WriteByte(c) - if err != nil { - return - } - inCharQuote = false - case c == '.': - // A single period is OK - if prev == '.' { - // Sequence of periods is not permitted - return "", "", fmt.Errorf("Sequence of periods is not permitted") - } - err = buf.WriteByte(c) - if err != nil { - return - } - inCharQuote = false - case c == '\\': - inCharQuote = true - case c == '"': - if inCharQuote { - err = buf.WriteByte(c) - if err != nil { - return - } - inCharQuote = false - } else if inStringQuote { - inStringQuote = false - } else { - if i == 0 { - inStringQuote = true - } else { - return "", "", fmt.Errorf("Quoted string can only begin at start of address") - } - } - case c == '@': - if inCharQuote || inStringQuote { - err = buf.WriteByte(c) - if err != nil { - return - } - inCharQuote = false - } else { - // End of local-part - if i > 128 { - return "", "", fmt.Errorf("Local part must not exceed 128 characters") - } - if prev == '.' { - return "", "", fmt.Errorf("Local part cannot end with a period") - } - domain = address[i+1:] - break LOOP - } - case c > 127: - return "", "", fmt.Errorf("Characters outside of US-ASCII range not permitted") - default: - if inCharQuote || inStringQuote { - err = buf.WriteByte(c) - if err != nil { - return - } - inCharQuote = false - } else { - return "", "", fmt.Errorf("Character %q must be quoted", c) - } - } - prev = c - } - if inCharQuote { - return "", "", fmt.Errorf("Cannot end address with unterminated quoted-pair") - } - 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 -} - // StringAddressList converts a list of addresses to a list of strings func StringAddressList(addrs []*mail.Address) []string { s := make([]string, len(addrs)) diff --git a/pkg/stringutil/utils_test.go b/pkg/stringutil/utils_test.go index 330bfbd..8c0b7bd 100644 --- a/pkg/stringutil/utils_test.go +++ b/pkg/stringutil/utils_test.go @@ -1,215 +1,33 @@ -package stringutil +package stringutil_test import ( - "strings" + "net/mail" "testing" - "github.com/stretchr/testify/assert" + "github.com/jhillyerd/inbucket/pkg/stringutil" ) -func TestParseMailboxName(t *testing.T) { - var validTable = []struct { - input string - expect string - }{ - {"mailbox", "mailbox"}, - {"user123", "user123"}, - {"MailBOX", "mailbox"}, - {"First.Last", "first.last"}, - {"user+label", "user"}, - {"chars!#$%", "chars!#$%"}, - {"chars&'*-", "chars&'*-"}, - {"chars=/?^", "chars=/?^"}, - {"chars_`.{", "chars_`.{"}, - {"chars|}~", "chars|}~"}, - } - - for _, tt := range validTable { - if result, err := ParseMailboxName(tt.input); err != nil { - t.Errorf("Error while parsing %q: %v", tt.input, err) - } else { - if result != tt.expect { - t.Errorf("Parsing %q, expected %q, got %q", tt.input, tt.expect, result) - } - } - } - - var invalidTable = []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 := ParseMailboxName(tt.input); err == nil { - t.Errorf("Didn't get an error while parsing %q: %v", tt.input, tt.msg) - } - } -} - func TestHashMailboxName(t *testing.T) { - assert.Equal(t, HashMailboxName("mail"), "1d6e1cf70ec6f9ab28d3ea4b27a49a77654d370e") -} - -func TestValidateDomain(t *testing.T) { - assert.False(t, ValidateDomainPart(strings.Repeat("a", 256)), - "Max domain length is 255") - assert.False(t, ValidateDomainPart(strings.Repeat("a", 64)+".com"), - "Max label length is 63") - assert.True(t, ValidateDomainPart(strings.Repeat("a", 63)+".com"), - "Should allow 63 char label") - - var testTable = []struct { - input string - expect bool - msg string - }{ - {"", false, "Empty domain is not valid"}, - {"hostname", true, "Just a hostname is valid"}, - {"github.com", true, "Two labels should be just fine"}, - {"my-domain.com", true, "Hyphen is allowed mid-label"}, - {"_domainkey.foo.com", true, "Underscores are allowed"}, - {"bar.com.", true, "Must be able to end with a dot"}, - {"ABC.6DBS.com", true, "Mixed case is OK"}, - {"mail.123.com", true, "Number only label valid"}, - {"123.com", true, "Number only label valid"}, - {"google..com", false, "Double dot not valid"}, - {".foo.com", false, "Cannot start with a dot"}, - {"google\r.com", false, "Special chars not allowed"}, - {"foo.-bar.com", false, "Label cannot start with hyphen"}, - {"foo-.bar.com", false, "Label cannot end with hyphen"}, - } - - for _, tt := range testTable { - if ValidateDomainPart(tt.input) != tt.expect { - t.Errorf("Expected %v for %q: %s", tt.expect, tt.input, tt.msg) - } + want := "1d6e1cf70ec6f9ab28d3ea4b27a49a77654d370e" + got := stringutil.HashMailboxName("mail") + if got != want { + t.Errorf("Got %q, want %q", got, want) } } -func TestValidateLocal(t *testing.T) { - var testTable = []struct { - input string - expect bool - msg string - }{ - {"", false, "Empty local is not valid"}, - {"a", true, "Single letter should be fine"}, - {strings.Repeat("a", 128), true, "Valid up to 128 characters"}, - {strings.Repeat("a", 129), false, "Only valid up to 128 characters"}, - {"FirstLast", true, "Mixed case permitted"}, - {"user123", true, "Numbers permitted"}, - {"a!#$%&'*+-/=?^_`{|}~", true, "Any of !#$%&'*+-/=?^_`{|}~ are permitted"}, - {"first.last", true, "Embedded period is permitted"}, - {"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"}, - {"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\\@mail", true, "Quoted @ permitted"}, - {"quoted\\ space", true, "Quoted space permitted"}, - {"no\\,commas", true, "Quoted comma is OK"}, - {"t\\[es\\]t", true, "Quoted brackets are OK"}, - {"user\\name", true, "Should be able to quote a-z"}, - {"USER\\NAME", true, "Should be able to quote A-Z"}, - {"user\\1", true, "Should be able to quote a digit"}, - {"one\\$\\|", true, "Should be able to quote plain specials"}, - {"return\\\r", true, "Should be able to quote ASCII control chars"}, - {"high\\\x80", false, "Should not accept > 7-bit quoted chars"}, - {"quote\\\"", true, "Quoted double quote is permitted"}, - {"\"james\"", true, "Quoted a-z is permitted"}, - {"\"first last\"", true, "Quoted space is permitted"}, - {"\"quoted@sign\"", true, "Quoted @ is allowed"}, - {"\"qp\\\"quote\"", true, "Quoted quote within quoted string is OK"}, - {"\"unterminated", false, "Quoted string must be terminated"}, - {"\"unterminated\\\"", false, "Quoted string must be terminated"}, - {"embed\"quote\"string", false, "Embedded quoted string is illegal"}, - - {"user+mailbox", true, "RFC3696 test case should be valid"}, - {"customer/department=shipping", true, "RFC3696 test case should be valid"}, - {"$A12345", true, "RFC3696 test case should be valid"}, - {"!def!xyz%abc", true, "RFC3696 test case should be valid"}, - {"_somename", true, "RFC3696 test case should be valid"}, +func TestStringAddressList(t *testing.T) { + input := []*mail.Address{ + {Name: "Fred B. Fish", Address: "fred@fish.org"}, + {Name: "User", Address: "user@domain.org"}, } - - for _, tt := range testTable { - _, _, err := ParseEmailAddress(tt.input + "@domain.com") - if (err != nil) == tt.expect { - if err != nil { - t.Logf("Got error: %s", err) - } - t.Errorf("Expected %v for %q: %s", tt.expect, tt.input, tt.msg) - } - } -} - -func TestParseEmailAddress(t *testing.T) { - // Test some good email addresses - var testTable = []struct { - input, local, domain string - }{ - {"root@localhost", "root", "localhost"}, - {"FirstLast@domain.local", "FirstLast", "domain.local"}, - {"route66@prodigy.net", "route66", "prodigy.net"}, - {"lorbit!user@uucp", "lorbit!user", "uucp"}, - {"user+spam@gmail.com", "user+spam", "gmail.com"}, - {"first.last@domain.local", "first.last", "domain.local"}, - {"first\\ last@_key.domain.com", "first last", "_key.domain.com"}, - {"first\\\"last@a.b.c", "first\"last", "a.b.c"}, - {"user\\@internal@myhost.ca", "user@internal", "myhost.ca"}, - {"\"first last@evil\"@top-secret.gov", "first last@evil", "top-secret.gov"}, - {"\"line\nfeed\"@linenoise.co.uk", "line\nfeed", "linenoise.co.uk"}, - {"user+mailbox@host", "user+mailbox", "host"}, - {"customer/department=shipping@host", "customer/department=shipping", "host"}, - {"$A12345@host", "$A12345", "host"}, - {"!def!xyz%abc@host", "!def!xyz%abc", "host"}, - {"_somename@host", "_somename", "host"}, - } - - for _, tt := range testTable { - local, domain, err := ParseEmailAddress(tt.input) - if err != nil { - t.Errorf("Error when parsing %q: %s", tt.input, err) - } else { - if tt.local != local { - t.Errorf("When parsing %q, expected local %q, got %q instead", - tt.input, tt.local, local) - } - if tt.domain != domain { - t.Errorf("When parsing %q, expected domain %q, got %q instead", - tt.input, tt.domain, domain) - } - } - } - - // Check that validations fail correctly - var badTable = []struct { - input, msg string - }{ - {"", "Empty address not permitted"}, - {"user", "Missing domain part"}, - {"@host", "Missing local part"}, - {"user\\@host", "Missing domain part"}, - {"\"user@host\"", "Missing domain part"}, - {"\"user@host", "Unterminated quoted string"}, - {"first last@host", "Unquoted space"}, - {"user@bad!domain", "Invalid domain"}, - {".user@host", "Can't lead with a ."}, - {"user.@host", "Can't end local with a dot"}, - {"user@bad domain", "No spaces in domain permitted"}, - } - - for _, tt := range badTable { - if _, _, err := ParseEmailAddress(tt.input); err == nil { - t.Errorf("Did not get expected error when parsing %q: %s", tt.input, tt.msg) + want := []string{`"Fred B. Fish" `, `"User" `} + output := stringutil.StringAddressList(input) + if len(output) != len(want) { + t.Fatalf("Got %v strings, want: %v", len(output), len(want)) + } + for i, got := range output { + if got != want[i] { + t.Errorf("Got %q, want: %q", got, want[i]) } } } diff --git a/pkg/webui/mailbox_controller.go b/pkg/webui/mailbox_controller.go index 16aa305..2af9ef1 100644 --- a/pkg/webui/mailbox_controller.go +++ b/pkg/webui/mailbox_controller.go @@ -8,9 +8,9 @@ import ( "strconv" "github.com/jhillyerd/inbucket/pkg/log" + "github.com/jhillyerd/inbucket/pkg/policy" "github.com/jhillyerd/inbucket/pkg/server/web" "github.com/jhillyerd/inbucket/pkg/storage" - "github.com/jhillyerd/inbucket/pkg/stringutil" "github.com/jhillyerd/inbucket/pkg/webui/sanitize" ) @@ -25,7 +25,7 @@ func MailboxIndex(w http.ResponseWriter, req *http.Request, ctx *web.Context) (e http.Redirect(w, req, web.Reverse("RootIndex"), http.StatusSeeOther) return nil } - name, err = stringutil.ParseMailboxName(name) + name, err = policy.ParseMailboxName(name) if err != nil { ctx.Session.AddFlash(err.Error(), "errors") _ = ctx.Session.Save(req, w) @@ -52,7 +52,7 @@ func MailboxIndex(w http.ResponseWriter, req *http.Request, ctx *web.Context) (e func MailboxLink(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { ctx.Session.AddFlash(err.Error(), "errors") _ = ctx.Session.Save(req, w) @@ -68,7 +68,7 @@ func MailboxLink(w http.ResponseWriter, req *http.Request, ctx *web.Context) (er // MailboxList renders a list of messages in a mailbox. Renders a partial func MailboxList(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -90,7 +90,7 @@ func MailboxList(w http.ResponseWriter, req *http.Request, ctx *web.Context) (er func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -131,7 +131,7 @@ func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *web.Context) (er func MailboxHTML(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -159,7 +159,7 @@ func MailboxHTML(w http.ResponseWriter, req *http.Request, ctx *web.Context) (er func MailboxSource(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { return err } @@ -183,7 +183,7 @@ func MailboxSource(w http.ResponseWriter, req *http.Request, ctx *web.Context) ( func MailboxDownloadAttach(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 id := ctx.Vars["id"] - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { ctx.Session.AddFlash(err.Error(), "errors") _ = ctx.Session.Save(req, w) @@ -225,7 +225,7 @@ func MailboxDownloadAttach(w http.ResponseWriter, req *http.Request, ctx *web.Co // MailboxViewAttach sends the attachment to the client for online viewing func MailboxViewAttach(w http.ResponseWriter, req *http.Request, ctx *web.Context) (err error) { // Don't have to validate these aren't empty, Gorilla returns 404 - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { ctx.Session.AddFlash(err.Error(), "errors") _ = ctx.Session.Save(req, w) diff --git a/pkg/webui/root_controller.go b/pkg/webui/root_controller.go index 85d1662..21d4dd3 100644 --- a/pkg/webui/root_controller.go +++ b/pkg/webui/root_controller.go @@ -7,8 +7,8 @@ import ( "net/http" "github.com/jhillyerd/inbucket/pkg/config" + "github.com/jhillyerd/inbucket/pkg/policy" "github.com/jhillyerd/inbucket/pkg/server/web" - "github.com/jhillyerd/inbucket/pkg/stringutil" ) // RootIndex serves the Inbucket landing page @@ -58,7 +58,7 @@ func RootMonitorMailbox(w http.ResponseWriter, req *http.Request, ctx *web.Conte http.Redirect(w, req, web.Reverse("RootIndex"), http.StatusSeeOther) return nil } - name, err := stringutil.ParseMailboxName(ctx.Vars["name"]) + name, err := policy.ParseMailboxName(ctx.Vars["name"]) if err != nil { ctx.Session.AddFlash(err.Error(), "errors") _ = ctx.Session.Save(req, w)