Skip to content

Commit

Permalink
Issue #506: sort domain names in reverse dns notation
Browse files Browse the repository at this point in the history
This patch fixes a behaviour 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
  • Loading branch information
magiconair committed Jun 10, 2018
1 parent 4ea9d1b commit a7bdd07
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 4 deletions.
35 changes: 34 additions & 1 deletion route/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
9 changes: 6 additions & 3 deletions route/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
`

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit a7bdd07

Please sign in to comment.