Merge pull request #209 from SkynetLabs/ivo/fix_email_panic

Fix a panic when the email configuration is invalid.

Author: MSevey

email/sender.go

@@ -23,6 +23,9 @@ const (
 )
 
 var (
+	// ErrInvalidEmailConfiguration is returned when  the email URI given in the
+	// environment (ACCOUNTS_EMAIL_URI) is either empty or otherwise invalid.
+	ErrInvalidEmailConfiguration = errors.New("missing or invalid email configuration field ACCOUNTS_EMAIL_URI")
 	// From is the address we send emails from. It defaults to the user
 	// from DefaultConnectionURI but can be overridden by the ACCOUNTS_EMAIL_FROM
 	// environment variable.
@@ -202,6 +205,9 @@ func (s Sender) sendMultiple(m ...*mail.Message) error {
 // values from it.
 func config(connURI string) (emailConfig, error) {
 	match := matchPattern.FindStringSubmatch(connURI)
+	if len(match) == 0 {
+		return emailConfig{}, ErrInvalidEmailConfiguration
+	}
 	result := make(map[string]string)
 	for i, name := range matchPattern.SubexpNames() {
 		if i != 0 && name != "" {
@@ -216,14 +222,14 @@ func config(connURI string) (emailConfig, error) {
 	// These fields are obligatory, so we return an error if any of them are
 	// missing.
 	if !(e1 && e2 && e3 && e4) {
-		return emailConfig{}, errors.New("missing obligatory email configuration field. One of server, port, user, or password is missing")
+		return emailConfig{}, ErrInvalidEmailConfiguration
 	}
 	user, err1 := url.QueryUnescape(user)
 	password, err2 := url.QueryUnescape(password)
 	port, err3 := strconv.Atoi(portStr)
 	err := errors.Compose(err1, err2, err3)
 	if err != nil {
-		return emailConfig{}, errors.AddContext(err, "invalid value for username, password, or port in email connection string")
+		return emailConfig{}, errors.Compose(err, ErrInvalidEmailConfiguration)
 	}
 	skip := result["skip_ssl_verify"]
 	return emailConfig{

email/sender_test.go

@@ -2,10 +2,13 @@ package email
 
 import (
 	"testing"
+
+	"gitlab.com/NebulousLabs/errors"
 )
 
 // TestConfig ensures that config properly parses email connection URIs.
 func TestConfig(t *testing.T) {
+	// Valid URI with skip_ssl_verify.
 	s := "smtps://test:test1@mailslurper:1025/?skip_ssl_verify=true"
 	c, err := config(s)
 	if err != nil {
@@ -14,7 +17,7 @@ func TestConfig(t *testing.T) {
 	if c.Server != "mailslurper" || c.Port != 1025 || c.User != "test" || c.Pass != "test1" || !c.InsecureSkipVerify {
 		t.Fatal("Unexpected result.")
 	}
-
+	// Valid URI without skip_ssl_verify.
 	s = "smtps://asdf:[email protected]:999"
 	c, err = config(s)
 	if err != nil {
@@ -23,6 +26,12 @@ func TestConfig(t *testing.T) {
 	if c.Server != "mail.siasky.net" || c.Port != 999 || c.User != "asdf" || c.Pass != "fdsa" || c.InsecureSkipVerify {
 		t.Fatal("Unexpected result.")
 	}
+	// Invalid URI (missing port).
+	s = "smtps://asdf:[email protected]"
+	c, err = config(s)
+	if err == nil || !errors.Contains(err, ErrInvalidEmailConfiguration) {
+		t.Fatalf("Expected error '%s', got '%s'", ErrInvalidEmailConfiguration, err)
+	}
 }
 
 // TestServerLockID make sure that ServerLockID is set in testing mode. If it's

main.go

@@ -171,12 +171,12 @@ func parseConfiguration(logger *logrus.Logger) (ServiceConfig, error) {
 	config.EmailURI = os.Getenv(envEmailURI)
 	{
 		if config.EmailURI == "" {
-			return ServiceConfig{}, errors.New(envEmailURI + " is empty")
+			return ServiceConfig{}, email.ErrInvalidEmailConfiguration
 		}
 		// Validate the given URI.
 		uri, err := url.Parse(config.EmailURI)
 		if err != nil || uri.Host == "" || uri.User == nil {
-			return ServiceConfig{}, errors.New("invalid email URI given in " + envEmailURI)
+			return ServiceConfig{}, email.ErrInvalidEmailConfiguration
 		}
 		// Set the FROM address to outgoing emails. This can be overridden by
 		// the ACCOUNTS_EMAIL_FROM optional environment variable.

main_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/SkynetLabs/skynet-accounts/database"
+	"github.com/SkynetLabs/skynet-accounts/email"
 	"github.com/sirupsen/logrus"
 	"gitlab.com/NebulousLabs/errors"
 )
@@ -143,7 +144,7 @@ func TestParseConfiguration(t *testing.T) {
 		t.Fatal(err)
 	}
 	_, err = parseConfiguration(logger)
-	if err == nil || !strings.Contains(err.Error(), envEmailURI+" is empty") {
+	if err == nil || !errors.Contains(err, email.ErrInvalidEmailConfiguration) {
 		t.Fatal("Failed to error out on empty", envEmailURI)
 	}
 	// Invalid ACCOUNTS_EMAIL_URI
@@ -152,7 +153,7 @@ func TestParseConfiguration(t *testing.T) {
 		t.Fatal(err)
 	}
 	_, err = parseConfiguration(logger)
-	if err == nil || !strings.Contains(err.Error(), "invalid email URI") {
+	if err == nil || !errors.Contains(err, email.ErrInvalidEmailConfiguration) {
 		t.Log(err)
 		t.Fatal("Failed to error out on invalid", envEmailURI)
 	}