Extend ParseMailboxName()

- Checks for invalid characters, returns useful error if it finds them
- Extended unit tests for ParseMailboxName
- Closes #6

smtpd/filestore.go

@@ -76,7 +76,10 @@ func DefaultFileDataStore() DataStore {
 // 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 := ParseMailboxName(emailAddress)
+	name, err := ParseMailboxName(emailAddress)
+	if err != nil {
+		return nil, err
+	}
 	dir := HashMailboxName(name)
 	s1 := dir[0:3]
 	s2 := dir[0:6]

smtpd/utils.go

@@ -9,16 +9,36 @@ import (
 	"strings"
 )
 
-// Take "user+ext@host.com" and return "user", aka the mailbox we'll store it in
-func ParseMailboxName(emailAddress string) (result string) {
-	result = strings.ToLower(emailAddress)
-	if idx := strings.Index(result, "@"); idx > -1 {
-		result = result[0:idx]
+// 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.
+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
+	return result, nil
 }
 
 // Take a mailbox name and hash it into the directory we'll store it in

smtpd/utils_test.go

@@ -7,9 +7,47 @@ import (
 )
 
 func TestParseMailboxName(t *testing.T) {
-	assert.Equal(t, ParseMailboxName("MailBOX"), "mailbox")
-	assert.Equal(t, ParseMailboxName("MailBox@Host.Com"), "mailbox")
-	assert.Equal(t, ParseMailboxName("Mail+extra@Host.Com"), "mail")
+	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) {

web/mailbox_controller.go

@@ -41,19 +41,29 @@ func MailboxIndex(w http.ResponseWriter, req *http.Request, ctx *Context) (err e
 		return nil
 	}
 
-	name = smtpd.ParseMailboxName(name)
+	name, err = smtpd.ParseMailboxName(name)
+	if err != nil {
+		ctx.Session.AddFlash(err.Error(), "errors")
+		http.Redirect(w, req, reverse("RootIndex"), http.StatusSeeOther)
+		return nil
+	}
 
 	return RenderTemplate("mailbox/index.html", w, map[string]interface{}{
-		"ctx":  ctx,
-		"name": name,
+		"ctx":      ctx,
+		"name":     name,
 		"selected": selected,
 	})
 }
 
 func MailboxLink(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) {
 	// Don't have to validate these aren't empty, Gorilla returns 404
-	name := smtpd.ParseMailboxName(ctx.Vars["name"])
 	id := ctx.Vars["id"]
+	name, err := smtpd.ParseMailboxName(ctx.Vars["name"])
+	if err != nil {
+		ctx.Session.AddFlash(err.Error(), "errors")
+		http.Redirect(w, req, reverse("RootIndex"), http.StatusSeeOther)
+		return nil
+	}
 
 	uri := fmt.Sprintf("%s?name=%s&id=%s", reverse("MailboxIndex"), name, id)
 	http.Redirect(w, req, uri, http.StatusSeeOther)
@@ -62,8 +72,10 @@ func MailboxLink(w http.ResponseWriter, req *http.Request, ctx *Context) (err er
 
 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 := smtpd.ParseMailboxName(ctx.Vars["name"])
-
+	name, err := smtpd.ParseMailboxName(ctx.Vars["name"])
+	if err != nil {
+		return err
+	}
 	mb, err := ctx.DataStore.MailboxFor(name)
 	if err != nil {
 		return fmt.Errorf("Failed to get mailbox for %v: %v", name, err)
@@ -98,9 +110,11 @@ func MailboxList(w http.ResponseWriter, req *http.Request, ctx *Context) (err er
 
 func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) {
 	// Don't have to validate these aren't empty, Gorilla returns 404
-	name := smtpd.ParseMailboxName(ctx.Vars["name"])
 	id := ctx.Vars["id"]
-
+	name, err := smtpd.ParseMailboxName(ctx.Vars["name"])
+	if err != nil {
+		return err
+	}
 	mb, err := ctx.DataStore.MailboxFor(name)
 	if err != nil {
 		return fmt.Errorf("MailboxFor('%v'): %v", name, err)
@@ -150,8 +164,10 @@ func MailboxShow(w http.ResponseWriter, req *http.Request, ctx *Context) (err er
 
 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 := smtpd.ParseMailboxName(ctx.Vars["name"])
-
+	name, err := smtpd.ParseMailboxName(ctx.Vars["name"])
+	if err != nil {
+		return err
+	}
 	mb, err := ctx.DataStore.MailboxFor(name)
 	if err != nil {
 		return fmt.Errorf("MailboxFor('%v'): %v", name, err)
@@ -172,9 +188,11 @@ func MailboxPurge(w http.ResponseWriter, req *http.Request, ctx *Context) (err e
 
 func MailboxHtml(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) {
 	// Don't have to validate these aren't empty, Gorilla returns 404
-	name := smtpd.ParseMailboxName(ctx.Vars["name"])
 	id := ctx.Vars["id"]
-
+	name, err := smtpd.ParseMailboxName(ctx.Vars["name"])
+	if err != nil {
+		return err
+	}
 	mb, err := ctx.DataStore.MailboxFor(name)
 	if err != nil {
 		return err
@@ -199,9 +217,11 @@ func MailboxHtml(w http.ResponseWriter, req *http.Request, ctx *Context) (err er
 
 func MailboxSource(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) {
 	// Don't have to validate these aren't empty, Gorilla returns 404
-	name := smtpd.ParseMailboxName(ctx.Vars["name"])
 	id := ctx.Vars["id"]
-
+	name, err := smtpd.ParseMailboxName(ctx.Vars["name"])
+	if err != nil {
+		return err
+	}
 	mb, err := ctx.DataStore.MailboxFor(name)
 	if err != nil {
 		return err
@@ -222,8 +242,13 @@ func MailboxSource(w http.ResponseWriter, req *http.Request, ctx *Context) (err
 
 func MailboxDownloadAttach(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) {
 	// Don't have to validate these aren't empty, Gorilla returns 404
-	name := smtpd.ParseMailboxName(ctx.Vars["name"])
 	id := ctx.Vars["id"]
+	name, err := smtpd.ParseMailboxName(ctx.Vars["name"])
+	if err != nil {
+		ctx.Session.AddFlash(err.Error(), "errors")
+		http.Redirect(w, req, reverse("RootIndex"), http.StatusSeeOther)
+		return nil
+	}
 	numStr := ctx.Vars["num"]
 	num, err := strconv.ParseUint(numStr, 10, 32)
 	if err != nil {
@@ -259,7 +284,12 @@ func MailboxDownloadAttach(w http.ResponseWriter, req *http.Request, ctx *Contex
 
 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 := smtpd.ParseMailboxName(ctx.Vars["name"])
+	name, err := smtpd.ParseMailboxName(ctx.Vars["name"])
+	if err != nil {
+		ctx.Session.AddFlash(err.Error(), "errors")
+		http.Redirect(w, req, reverse("RootIndex"), http.StatusSeeOther)
+		return nil
+	}
 	id := ctx.Vars["id"]
 	numStr := ctx.Vars["num"]
 	num, err := strconv.ParseUint(numStr, 10, 32)
@@ -295,9 +325,11 @@ func MailboxViewAttach(w http.ResponseWriter, req *http.Request, ctx *Context) (
 
 func MailboxDelete(w http.ResponseWriter, req *http.Request, ctx *Context) (err error) {
 	// Don't have to validate these aren't empty, Gorilla returns 404
-	name := smtpd.ParseMailboxName(ctx.Vars["name"])
 	id := ctx.Vars["id"]
-
+	name, err := smtpd.ParseMailboxName(ctx.Vars["name"])
+	if err != nil {
+		return err
+	}
 	mb, err := ctx.DataStore.MailboxFor(name)
 	if err != nil {
 		return err