git » chasquid » commit 8bbb611

diff --git a/internal/aliases/aliases.go b/internal/aliases/aliases.go
index 8ed9e63..20b556d 100644
--- a/internal/aliases/aliases.go
+++ b/internal/aliases/aliases.go
@@ -151,28 +151,25 @@ func (v *Resolver) Resolve(tr *trace.Trace, addr string) ([]Recipient, error) {
 	return v.resolve(0, addr, tr)
 }
 
-// Exists check that the address exists in the database.
-// It returns the cleaned address, and a boolean indicating the result.
-// The clean address can be used to look it up in other databases, even if it
-// doesn't exist. It must only be called for local addresses.
-func (v *Resolver) Exists(tr *trace.Trace, addr string) (string, bool) {
+// Exists check that the address exists in the database.  It must only be
+// called for local addresses.
+func (v *Resolver) Exists(tr *trace.Trace, addr string) bool {
 	tr = tr.NewChild("Alias.Exists", addr)
 	defer tr.Finish()
 
-	addr = v.cleanIfLocal(addr)
-
+	addr = v.RemoveDropsAndSuffix(addr)
 	rcpts, _ := v.lookup(addr, tr)
 	if len(rcpts) > 0 {
-		return addr, true
+		return true
 	}
 
 	domain := envelope.DomainOf(addr)
 	catchAll, _ := v.lookup("*@"+domain, tr)
 	if len(catchAll) > 0 {
-		return addr, true
+		return true
 	}
 
-	return addr, false
+	return false
 }
 
 func (v *Resolver) lookup(addr string, tr *trace.Trace) ([]Recipient, error) {
@@ -209,7 +206,7 @@ func (v *Resolver) resolve(rcount int, addr string, tr *trace.Trace) ([]Recipien
 	// Drop suffixes and chars to get the "clean" address before resolving.
 	// This also means that we will return the clean version if there's no
 	// match, which our callers can rely upon.
-	addr = v.cleanIfLocal(addr)
+	addr = v.RemoveDropsAndSuffix(addr)
 
 	// Lookup in the aliases database.
 	rcpts, err := v.lookup(addr, tr)
@@ -278,17 +275,8 @@ func (v *Resolver) resolve(rcount int, addr string, tr *trace.Trace) ([]Recipien
 	return ret, nil
 }
 
-func (v *Resolver) cleanIfLocal(addr string) string {
+func (v *Resolver) RemoveDropsAndSuffix(addr string) string {
 	user, domain := envelope.Split(addr)
-
-	v.mu.Lock()
-	isLocal := v.domains[domain]
-	v.mu.Unlock()
-
-	if !isLocal {
-		return addr
-	}
-
 	user = removeAllAfter(user, v.SuffixSep)
 	user = removeChars(user, v.DropChars)
 	user, _ = normalize.User(user)
diff --git a/internal/aliases/aliases_test.go b/internal/aliases/aliases_test.go
index ccc63e9..a6b44be 100644
--- a/internal/aliases/aliases_test.go
+++ b/internal/aliases/aliases_test.go
@@ -41,7 +41,7 @@ func mustExist(t *testing.T, r *Resolver, addrs ...string) {
 	tr := trace.New("test", "mustExist")
 	defer tr.Finish()
 	for _, addr := range addrs {
-		if _, ok := r.Exists(tr, addr); !ok {
+		if ok := r.Exists(tr, addr); !ok {
 			t.Errorf("address %q does not exist, it should", addr)
 		}
 	}
@@ -52,7 +52,7 @@ func mustNotExist(t *testing.T, r *Resolver, addrs ...string) {
 	tr := trace.New("test", "mustNotExist")
 	defer tr.Finish()
 	for _, addr := range addrs {
-		if _, ok := r.Exists(tr, addr); ok {
+		if ok := r.Exists(tr, addr); ok {
 			t.Errorf("address %q exists, it should not", addr)
 		}
 	}
@@ -86,9 +86,9 @@ func TestBasic(t *testing.T) {
 	resolver.AddDomain("localA")
 	resolver.AddDomain("localB")
 	resolver.aliases = map[string][]Recipient{
-		"a@localA": {{"c@d", EMAIL}, {"e@localB", EMAIL}},
-		"e@localB": {{"cmd", PIPE}},
-		"cmd":      {{"x@y", EMAIL}}, // it's a trap!
+		"a@localA":   {{"c@d", EMAIL}, {"e@localB", EMAIL}},
+		"e@localB":   {{"cmd", PIPE}},
+		"cmd@localA": {{"x@y", EMAIL}},
 	}
 
 	cases := Cases{
@@ -98,7 +98,7 @@ func TestBasic(t *testing.T) {
 	}
 	cases.check(t, resolver)
 
-	mustExist(t, resolver, "a@localA", "e@localB", "cmd")
+	mustExist(t, resolver, "a@localA", "e@localB", "cmd@localA")
 	mustNotExist(t, resolver, "x@y")
 }
 
@@ -195,7 +195,7 @@ func TestAddrRewrite(t *testing.T) {
 	cases.check(t, resolver)
 }
 
-func TestExistsRewrite(t *testing.T) {
+func TestExists(t *testing.T) {
 	resolver := NewResolver(allUsersExist)
 	resolver.AddDomain("def")
 	resolver.AddDomain("p-q.com")
@@ -207,35 +207,53 @@ func TestExistsRewrite(t *testing.T) {
 	resolver.DropChars = ".~"
 	resolver.SuffixSep = "-+"
 
-	mustExist(t, resolver, "abc@def", "a.bc+blah@def", "ño.ño@def")
-	mustNotExist(t, resolver, "abc@d.ef", "nothere@def")
+	mustExist(t, resolver,
+		"abc@def",
+		"abc+blah@def",
+		"a.bc+blah@def",
+		"a.b~c@def",
+		"ñoño@def",
+		"ño.ño@def",
+		"recu@def",
+		"re.cu@def")
+	mustNotExist(t, resolver,
+		"abc@d.ef",
+		"nothere@def",
+		"ex@def",
+		"a.bc@unknown",
+		"x.yz@def",
+		"x.yz@d.ef",
+		"abc@d.ef")
+}
 
-	tr := trace.New("test", "TestExistsRewrite")
-	defer tr.Finish()
+func TestRemoveDropsAndSuffix(t *testing.T) {
+	resolver := NewResolver(allUsersExist)
+	resolver.AddDomain("def")
+	resolver.AddDomain("p-q.com")
+	resolver.aliases = map[string][]Recipient{
+		"abc@def":  {{"x@y", EMAIL}},
+		"ñoño@def": {{"x@y", EMAIL}},
+		"recu@def": {{"ab+cd@p-q.com", EMAIL}},
+	}
+	resolver.DropChars = ".~"
+	resolver.SuffixSep = "-+"
 
 	cases := []struct {
-		addr         string
-		expectAddr   string
-		expectExists bool
+		addr string
+		want string
 	}{
-		{"abc@def", "abc@def", true},
-		{"abc+blah@def", "abc@def", true},
-		{"a.b~c@def", "abc@def", true},
-		{"a.bc+blah@def", "abc@def", true},
-
-		{"a.bc@unknown", "a.bc@unknown", false},
-		{"x.yz@def", "xyz@def", false},
-		{"x.yz@d.ef", "x.yz@d.ef", false},
+		{"abc@def", "abc@def"},
+		{"abc+blah@def", "abc@def"},
+		{"a.b~c@def", "abc@def"},
+		{"a.bc+blah@def", "abc@def"},
+		{"x.yz@def", "xyz@def"},
+		{"x.yz@d.ef", "xyz@d.ef"},
 	}
 	for _, c := range cases {
-		addr, exists := resolver.Exists(tr, c.addr)
-		if addr != c.expectAddr {
-			t.Errorf("%q: expected addr %q, got %q",
-				c.addr, c.expectAddr, addr)
-		}
-		if exists != c.expectExists {
-			t.Errorf("%q: expected exists %v, got %v",
-				c.addr, c.expectExists, exists)
+		addr := resolver.RemoveDropsAndSuffix(c.addr)
+		if addr != c.want {
+			t.Errorf("RemoveDropsAndSuffix(%q): want %q, got %q",
+				c.addr, c.want, addr)
 		}
 	}
 }
diff --git a/internal/smtpsrv/conn.go b/internal/smtpsrv/conn.go
index f58bc3f..a69f025 100644
--- a/internal/smtpsrv/conn.go
+++ b/internal/smtpsrv/conn.go
@@ -591,7 +591,7 @@ func (c *Conn) RCPT(params string) (code int, msg string) {
 			return 550, "5.1.3 Destination address is invalid"
 		}
 
-		ok, err := c.userExists(addr)
+		ok, err := c.localUserExists(addr)
 		if err != nil {
 			c.tr.Errorf("error checking if user %q exists: %v", addr, err)
 			maillog.Rejected(c.remoteAddr, c.mailFrom, []string{addr},
@@ -1096,17 +1096,14 @@ func (c *Conn) resetEnvelope() {
 	c.spfError = nil
 }
 
-func (c *Conn) userExists(addr string) (bool, error) {
-	var ok bool
-	addr, ok = c.aliasesR.Exists(c.tr, addr)
-	if ok {
+func (c *Conn) localUserExists(addr string) (bool, error) {
+	if c.aliasesR.Exists(c.tr, addr) {
 		return true, nil
 	}
 
-	// Note we used the address returned by the aliases resolver, which has
-	// cleaned it up. This means that a check for "us.er@domain" will have us
-	// look up "user" in our databases if the domain is local, which is what
-	// we want.
+	// Remove the drop chars and suffixes, if any, so the database lookup is
+	// on a "clean" address.
+	addr = c.aliasesR.RemoveDropsAndSuffix(addr)
 	user, domain := envelope.Split(addr)
 	return c.authr.Exists(c.tr, user, domain)
 }