git » chasquid » commit 74e7c96

diff --git a/docs/aliases.md b/docs/aliases.md
index 91b5090..efef317 100644
--- a/docs/aliases.md
+++ b/docs/aliases.md
@@ -76,6 +76,57 @@ pepe: jose
 *: pepe, rose@backgarden
 ```
 
+### Overrides
+
+If the same left-side address appears more than once, the last one will take
+precedence.
+
+For example, in this case, the result is that `pepe` is aliased to `jose`, the
+first line is effectively ignored.
+
+```
+pepe: juan
+pepe: jose
+```
+
+### Drop characters and suffix separators
+
+When parsing aliases files, drop characters will be ignored. Suffix separators
+are kept as-is.
+
+When doing lookups, drop characters will also be ignored. If the address has a
+suffix, the lookup will include it; if there is no match, it will try again
+without the suffix.
+
+In practice, this means that if the aliases file contains:
+
+```
+juana.perez: juana
+juana.perez+fruta: fruta
+```
+
+Then (assuming the default drop characters and suffix separators), these are
+the results:
+
+```
+juana.perez -> juana
+juanaperez -> juana
+ju.ana.pe.rez -> juana
+
+juana.perez+abc -> juana
+juanaperez+abc -> juana
+
+juana.perez+fruta -> fruta
+juanaperez+fruta -> fruta
+```
+
+This allows addresses with suffixes to have specific aliases, without having
+to worry about drop characters, which is the most common use case.
+
+If different semantics are needed, they can be implemented using the
+[hook](#hooks).
+
+
 ## Processing
 
 Aliases files are read upon start-up and refreshed every 30 seconds, so
diff --git a/internal/aliases/aliases.go b/internal/aliases/aliases.go
index e2be600..37211be 100644
--- a/internal/aliases/aliases.go
+++ b/internal/aliases/aliases.go
@@ -50,6 +50,9 @@
 // and "us.er@domain" into "user@domain".
 //
 // Both are optional, and the characters configurable globally.
+//
+// There are more complex semantics around handling of drop characters and
+// suffixes, see the documentation for more details.
 package aliases
 
 import (
@@ -157,12 +160,22 @@ func (v *Resolver) Exists(tr *trace.Trace, addr string) bool {
 	tr = tr.NewChild("Alias.Exists", addr)
 	defer tr.Finish()
 
-	addr = v.RemoveDropsAndSuffix(addr)
+	// First, see if there's an exact match in the database.
+	// This allows us to have aliases that include suffixes in them, and have
+	// them take precedence.
 	rcpts, _ := v.lookup(addr, tr)
 	if len(rcpts) > 0 {
 		return true
 	}
 
+	// "Clean" the address, removing drop characters and suffixes, and try
+	// again.
+	addr = v.RemoveDropsAndSuffix(addr)
+	rcpts, _ = v.lookup(addr, tr)
+	if len(rcpts) > 0 {
+		return true
+	}
+
 	domain := envelope.DomainOf(addr)
 	catchAll, _ := v.lookup("*@"+domain, tr)
 	if len(catchAll) > 0 {
@@ -173,11 +186,17 @@ func (v *Resolver) Exists(tr *trace.Trace, addr string) bool {
 }
 
 func (v *Resolver) lookup(addr string, tr *trace.Trace) ([]Recipient, error) {
+	// Do a lookup in the aliases map. Note we remove drop characters first,
+	// which matches what we did at parsing time. Suffixes, if any, are left
+	// as-is; that is handled by the callers.
+	clean := v.RemoveDropCharacters(addr)
 	v.mu.Lock()
-	rcpts := v.aliases[addr]
+	rcpts := v.aliases[clean]
 	v.mu.Unlock()
 
 	// Augment with the hook results.
+	// Note we use the original address, to give maximum flexibility to the
+	// hooks.
 	hr, err := v.runResolveHook(tr, addr)
 	if err != nil {
 		tr.Debugf("lookup(%q) hook error: %v", addr, err)
@@ -203,18 +222,27 @@ func (v *Resolver) resolve(rcount int, addr string, tr *trace.Trace) ([]Recipien
 		return []Recipient{{addr, EMAIL}}, nil
 	}
 
-	// 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.RemoveDropsAndSuffix(addr)
-
-	// Lookup in the aliases database.
+	// First, see if there's an exact match in the database.
+	// This allows us to have aliases that include suffixes in them, and have
+	// them take precedence.
 	rcpts, err := v.lookup(addr, tr)
 	if err != nil {
 		tr.Debugf("%d| error in lookup: %v", rcount, err)
 		return nil, err
 	}
 
+	if len(rcpts) == 0 {
+		// Retry after removing drop characters and suffixes.
+		// This also means that we will return the clean version if there's no
+		// match, which our callers can rely upon.
+		addr = v.RemoveDropsAndSuffix(addr)
+		rcpts, err = v.lookup(addr, tr)
+		if err != nil {
+			tr.Debugf("%d| error in lookup: %v", rcount, err)
+			return nil, err
+		}
+	}
+
 	// No alias for this local address.
 	if len(rcpts) == 0 {
 		tr.Debugf("%d| no alias found", rcount)
@@ -275,6 +303,32 @@ func (v *Resolver) resolve(rcount int, addr string, tr *trace.Trace) ([]Recipien
 	return ret, nil
 }
 
+// Remove drop characters, but only up to the first suffix separator.
+func (v *Resolver) RemoveDropCharacters(addr string) string {
+	user, domain := envelope.Split(addr)
+
+	// Remove drop characters up to the first suffix separator.
+	firstSuffixSep := strings.IndexAny(user, v.SuffixSep)
+	if firstSuffixSep == -1 {
+		firstSuffixSep = len(user)
+	}
+
+	nu := ""
+	for _, c := range user[:firstSuffixSep] {
+		if !strings.ContainsRune(v.DropChars, c) {
+			nu += string(c)
+		}
+	}
+
+	// Copy any remaining suffix as-is.
+	if firstSuffixSep < len(user) {
+		nu += user[firstSuffixSep:]
+	}
+
+	nu, _ = normalize.User(nu)
+	return nu + "@" + domain
+}
+
 func (v *Resolver) RemoveDropsAndSuffix(addr string) string {
 	user, domain := envelope.Split(addr)
 	user = removeAllAfter(user, v.SuffixSep)
@@ -394,7 +448,11 @@ func (v *Resolver) parseReader(domain string, r io.Reader) (map[string][]Recipie
 			continue
 		}
 
+		// We remove DropChars from the address, but leave the suffixes (if
+		// any). This matches the behaviour expected by Exists and Resolve,
+		// see the documentation for more details.
 		addr = addr + "@" + domain
+		addr = v.RemoveDropCharacters(addr)
 		addr, _ = normalize.Addr(addr)
 
 		rs := parseRHS(rawalias, domain)
diff --git a/internal/aliases/aliases_test.go b/internal/aliases/aliases_test.go
index 15569ab..8dafc8e 100644
--- a/internal/aliases/aliases_test.go
+++ b/internal/aliases/aliases_test.go
@@ -162,6 +162,11 @@ func TestAddrRewrite(t *testing.T) {
 		"ñoño@def": {{"x@y", EMAIL}},
 		"recu@def": {{"ab+cd@p-q.com", EMAIL}},
 		"remo@def": {{"x-@y-z.com", EMAIL}},
+
+		// Aliases with a suffix, to make sure we handle them correctly.
+		// Note we don't allow aliases with drop characters, they get
+		// normalized at parsing time.
+		"recu-zzz@def": {{"z@z", EMAIL}},
 	}
 	resolver.DropChars = ".~"
 	resolver.SuffixSep = "-+"
@@ -185,6 +190,18 @@ func TestAddrRewrite(t *testing.T) {
 		// Clean the right hand side too (if it's a local domain).
 		{"recu+blah@def", []Recipient{{"ab@p-q.com", EMAIL}}, nil},
 
+		// Requests for "recu" and variants, because it has an alias with a
+		// suffix.
+		{"re-cu@def", []Recipient{{"re@def", EMAIL}}, nil},
+		{"re.cu@def", []Recipient{{"ab@p-q.com", EMAIL}}, nil},
+		{"re.cu-zzz@def", []Recipient{{"z@z", EMAIL}}, nil},
+
+		// Check that because we have an alias with a suffix, we do not
+		// accidentally use it for their "clean" versions.
+		{"re@def", []Recipient{{"re@def", EMAIL}}, nil},
+		{"r.e.c.u@def", []Recipient{{"ab@p-q.com", EMAIL}}, nil},
+		{"re.cu-yyy@def", []Recipient{{"ab@p-q.com", EMAIL}}, nil},
+
 		// We should not mess with emails for domains we don't know.
 		{"xy@z.com", []Recipient{{"xy@z.com", EMAIL}}, nil},
 		{"x.y@z.com", []Recipient{{"x.y@z.com", EMAIL}}, nil},
@@ -203,6 +220,11 @@ func TestExists(t *testing.T) {
 		"abc@def":  {{"x@y", EMAIL}},
 		"ñoño@def": {{"x@y", EMAIL}},
 		"recu@def": {{"ab+cd@p-q.com", EMAIL}},
+
+		// Aliases with a suffix, to make sure we handle them correctly.
+		// Note we don't allow aliases with drop characters, they get
+		// normalized at parsing time.
+		"ex-act@def": {{"x@y", EMAIL}},
 	}
 	resolver.DropChars = ".~"
 	resolver.SuffixSep = "-+"
@@ -215,7 +237,9 @@ func TestExists(t *testing.T) {
 		"ñoño@def",
 		"ño.ño@def",
 		"recu@def",
-		"re.cu@def")
+		"re.cu@def",
+		"ex-act@def",
+	)
 	mustNotExist(t, resolver,
 		"abc@d.ef",
 		"nothere@def",
@@ -223,7 +247,11 @@ func TestExists(t *testing.T) {
 		"a.bc@unknown",
 		"x.yz@def",
 		"x.yz@d.ef",
-		"abc@d.ef")
+		"abc@d.ef",
+		"exact@def",
+		"exa.ct@def",
+		"ex@def",
+	)
 }
 
 func TestRemoveDropsAndSuffix(t *testing.T) {
@@ -258,6 +286,51 @@ func TestRemoveDropsAndSuffix(t *testing.T) {
 	}
 }
 
+func TestRemoveDropCharacters(t *testing.T) {
+	resolver := NewResolver(allUsersExist)
+	resolver.AddDomain("def")
+	resolver.DropChars = "._"
+	resolver.SuffixSep = "-+"
+
+	cases := []struct {
+		addr string
+		want string
+	}{
+		{"abc@def", "abc@def"},
+		{"abc+blah@def", "abc+blah@def"},
+		{"a.b@def", "ab@def"},
+		{"a.b+c@def", "ab+c@def"},
+		{"a.b+c.d@def", "ab+c.d@def"},
+		{"a@def", "a@def"},
+		{"a+b@def", "a+b@def"},
+
+		// Cases with UTF-8, to make sure we handle indexing correctly.
+		{"ñoño@def", "ñoño@def"},
+		{"ñoño+blah@def", "ñoño+blah@def"},
+		{"ño.ño@def", "ñoño@def"},
+		{"ño.ño+blah@def", "ñoño+blah@def"},
+		{"ño.ño+ñaca@def", "ñoño+ñaca@def"},
+		{"ño.ño+ña.ca@def", "ñoño+ña.ca@def"},
+		{"ño.ño+ñaña@def", "ñoño+ñaña@def"},
+		{"ño.ño+ña.ña@def", "ñoño+ña.ña@def"},
+
+		// Check "the other" drop char/suffix separator to make sure we
+		// don't skip any of them.
+		{"a_b@def", "ab@def"},
+		{"a_b-c@def", "ab-c@def"},
+		{"a_b-c.d@def", "ab-c.d@def"},
+		{"ño_ño-ña.ña@def", "ñoño-ña.ña@def"},
+	}
+
+	for _, c := range cases {
+		addr := resolver.RemoveDropCharacters(c.addr)
+		if addr != c.want {
+			t.Errorf("RemoveDropCharacters(%q): want %q, got %q",
+				c.addr, c.want, addr)
+		}
+	}
+}
+
 func TestTooMuchRecursion(t *testing.T) {
 	resolver := NewResolver(allUsersExist)
 	resolver.AddDomain("b")
@@ -399,6 +472,15 @@ o1: b
 # Check that we normalize the right hand side.
 aA: bB@dom-B
 
+# Test that exact aliases take precedence.
+pq: pa
+p.q: pb
+p.q+r: pc
+pq+r: pd
+ppp1: p.q+r
+ppp2: p.q
+ppp3: ppp2
+
 # Finally one to make the file NOT end in \n:
 y: z`
 
@@ -407,6 +489,8 @@ func TestRichFile(t *testing.T) {
 	defer os.Remove(fname)
 
 	resolver := NewResolver(allUsersExist)
+	resolver.DropChars = "."
+	resolver.SuffixSep = "+"
 	err := resolver.AddAliasesFile("dom", fname)
 	if err != nil {
 		t.Fatalf("failed to add file: %v", err)
@@ -416,9 +500,23 @@ func TestRichFile(t *testing.T) {
 		{"a@dom", []Recipient{{"b@dom", EMAIL}}, nil},
 		{"c@dom", []Recipient{{"d@e", EMAIL}, {"f@dom", EMAIL}}, nil},
 		{"x@dom", []Recipient{{"command", PIPE}}, nil},
+
 		{"o1@dom", []Recipient{{"b@dom", EMAIL}}, nil},
+
 		{"aA@dom", []Recipient{{"bb@dom-b", EMAIL}}, nil},
 		{"aa@dom", []Recipient{{"bb@dom-b", EMAIL}}, nil},
+
+		{"pq@dom", []Recipient{{"pb@dom", EMAIL}}, nil},
+		{"p.q@dom", []Recipient{{"pb@dom", EMAIL}}, nil},
+		{"p.q+r@dom", []Recipient{{"pd@dom", EMAIL}}, nil},
+		{"pq+r@dom", []Recipient{{"pd@dom", EMAIL}}, nil},
+		{"pq+z@dom", []Recipient{{"pb@dom", EMAIL}}, nil},
+		{"p..q@dom", []Recipient{{"pb@dom", EMAIL}}, nil},
+		{"p..q+r@dom", []Recipient{{"pd@dom", EMAIL}}, nil},
+		{"ppp1@dom", []Recipient{{"pd@dom", EMAIL}}, nil},
+		{"ppp2@dom", []Recipient{{"pb@dom", EMAIL}}, nil},
+		{"ppp3@dom", []Recipient{{"pb@dom", EMAIL}}, nil},
+
 		{"y@dom", []Recipient{{"z@dom", EMAIL}}, nil},
 	}
 	cases.check(t, resolver)
diff --git a/test/t-04-aliases/alias-resolve-hook b/test/t-04-aliases/alias-resolve-hook
index c73df5a..68ca4b7 100755
--- a/test/t-04-aliases/alias-resolve-hook
+++ b/test/t-04-aliases/alias-resolve-hook
@@ -5,6 +5,9 @@ case "$1" in
 	# Test one naked, one full. These exist in the static aliases file.
 	echo pepe, joan@testserver
 	;;
+"vic.uña+abc@testserver")
+	echo uña
+	;;
 "ñandú@testserver")
 	echo "| writemailto ../.data/pipe_alias_worked"
 	;;
diff --git a/test/t-04-aliases/run.sh b/test/t-04-aliases/run.sh
index 1eb9573..f2a8873 100755
--- a/test/t-04-aliases/run.sh
+++ b/test/t-04-aliases/run.sh
@@ -50,8 +50,11 @@ mail_diff content .data/pipe_alias_worked
 mkdir -p config/hooks/
 cp alias-resolve-hook config/hooks/alias-resolve
 
-# Test email aliases.
+# Test email aliases via the hook.
 send_and_check vicuña juan jose
+send_and_check vi.cu.ña juan jose
+send_and_check vi.cu.ña+abc juan jose
+send_and_check vic.uña+abc uña
 
 # Test the pipe alias separately.
 rm -f .data/pipe_alias_worked