From b934042f26b0d3e00aee7e58744c95f77c20d1dd Mon Sep 17 00:00:00 2001
From: Frank Schroeder <frank.schroeder@gmail.com>
Date: Sun, 10 Jun 2018 21:16:19 +0200
Subject: [PATCH] Issue #506: reverse domain names before sorting

This patch fixes a behavior where fabio routes to the wrong backend when
multiple glob matching routes are available since the sorting of the
host names was wrong.

Sorting DNS names requires reversing them before sorting since they have
their most significant part at the front.

Fixes #506
---
 route/table.go      | 35 ++++++++++++++++++++++++++++++++++-
 route/table_test.go |  9 ++++++---
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/route/table.go b/route/table.go
index c417e2c34..bb689dcff 100644
--- a/route/table.go
+++ b/route/table.go
@@ -303,8 +303,38 @@ func (t Table) matchingHosts(req *http.Request) (hosts []string) {
 			hosts = append(hosts, pattern)
 		}
 	}
+
+	if len(hosts) < 2 {
+		return
+	}
+
+	// Issue 506: multiple glob patterns hosts in wrong order
+	//
+	// DNS names have their most specific part at the front. In order to sort
+	// them from most specific to least specific a lexicographic sort will
+	// return the wrong result since it sorts by host name. *.foo.com will come
+	// before *.a.foo.com even though the latter is more specific. To achieve
+	// the correct result we need to reverse the strings, sort them and then
+	// reverse them again.
+	for i, h := range hosts {
+		hosts[i] = Reverse(h)
+	}
 	sort.Sort(sort.Reverse(sort.StringSlice(hosts)))
-	return hosts
+	for i, h := range hosts {
+		hosts[i] = Reverse(h)
+	}
+	return
+}
+
+// Reverse returns its argument string reversed rune-wise left to right.
+//
+// taken from https://github.com/golang/example/blob/master/stringutil/reverse.go
+func Reverse(s string) string {
+	r := []rune(s)
+	for i, j := 0, len(r)-1; i < len(r)/2; i, j = i+1, j-1 {
+		r[i], r[j] = r[j], r[i]
+	}
+	return string(r)
 }
 
 // Lookup finds a target url based on the current matcher and picker
@@ -322,6 +352,9 @@ func (t Table) Lookup(req *http.Request, trace string, pick picker, match matche
 	// find matching hosts for the request
 	// and add "no host" as the fallback option
 	hosts := t.matchingHosts(req)
+	if trace != "" {
+		log.Printf("[TRACE] %s Matching hosts: %v", trace, hosts)
+	}
 	hosts = append(hosts, "")
 	for _, h := range hosts {
 		if target = t.lookup(h, req.URL.Path, trace, pick, match); target != nil {
diff --git a/route/table_test.go b/route/table_test.go
index 33a7869ca..808c9168b 100644
--- a/route/table_test.go
+++ b/route/table_test.go
@@ -569,7 +569,8 @@ func TestTableLookup(t *testing.T) {
 	route add svc z.abc.com/foo/ http://foo.com:3100
 	route add svc *.abc.com/ http://foo.com:4000
 	route add svc *.abc.com/foo/ http://foo.com:5000
-	route add svc *.def.abc.com/ http://foo.com:6000
+	route add svc *.aaa.abc.com/ http://foo.com:6000
+	route add svc *.bbb.abc.com/ http://foo.com:6100
 	route add svc xyz.com:80/ https://xyz.com
 	`
 
@@ -611,8 +612,10 @@ func TestTableLookup(t *testing.T) {
 		{&http.Request{Host: ".abc.com", URL: mustParse("/foo/")}, "http://foo.com:5000"},
 		{&http.Request{Host: "x.y.abc.com", URL: mustParse("/foo/")}, "http://foo.com:5000"},
 		{&http.Request{Host: "y.abc.com:80", URL: mustParse("/foo/")}, "http://foo.com:5000"},
-		{&http.Request{Host: "x.def.abc.com", URL: mustParse("/")}, "http://foo.com:6000"},
-		{&http.Request{Host: "x.def.abc.com", URL: mustParse("/foo")}, "http://foo.com:6000"},
+		{&http.Request{Host: "x.aaa.abc.com", URL: mustParse("/")}, "http://foo.com:6000"},
+		{&http.Request{Host: "x.aaa.abc.com", URL: mustParse("/foo")}, "http://foo.com:6000"},
+		{&http.Request{Host: "x.bbb.abc.com", URL: mustParse("/")}, "http://foo.com:6100"},
+		{&http.Request{Host: "x.bbb.abc.com", URL: mustParse("/foo")}, "http://foo.com:6100"},
 		{&http.Request{Host: "y.abc.com:443", URL: mustParse("/foo/"), TLS: &tls.ConnectionState{}}, "http://foo.com:5000"},
 
 		// exact match has precedence over glob match