Issue 647 NormalizeHost

[murphymj25]

#647

[murphymj25]

For this PR, i've created a new NormalizeHostNoLower which doesn't have the string.ToLower. And changed the if statement in matchingHostNoGlob to use string.EqualFold. This allows the comparison to happen without needing to lower the case.

I don't think we would need to have the separate function, but this approach allowed me to quickly test the change for the matchigHostNoGlob function, which is what we are using.

As I stated in #647 I think there is some opportunity to further reduce the CPU utilization by changes port suffix are trimmed off or perhaps accounting for this right in the matching host functions.

Any thoughts on how best to approach this would be appreciated.

[pschultz]

fabio/route/table.go

Lines 158 to 160 in 660ad76

	func (t Table) addRoute(d *RouteDef) error {
	host, path := hostpath(d.Src)
	host = strings.ToLower(host) // maintain compatibility with parseURLPrefixTag

fabio/route/table.go

Line 184 in 660ad76

t[host] = Routes{r}

The table keys are already lowercase, so it seems we don't even need to use EqualFold:

func (t Table) matchingHostNoGlob(req *http.Request) (hosts []string) {
    host := normalizeHost(req.Host, req.TLS != nil) // note: *with* ToLower here

    for pattern := range t { // pattern is already lower case
        normpat := normalizeHostNoLower(pattern, req.TLS != nil)
        if normpat == host {
            hosts = append(hosts, pattern)
            return
        }   
    }   
    return
}

The Route.Glob field is used to cache compiled path globs. It seems simple enough to add a field for the normalized host, so we don't have to call normalizeHostNoLower on lots of never-changing values for each request. If you want to improve performance with globs enabled, you could also add another field for pre-compiled host globs.

[pschultz]

You should change normalizeHost to use this new function:

func normalizeHost(host string, tls bool) string {
    return strings.ToLower(normalizeHostNoLower(host, tls))
}

[murphymj25]

@pschultz Thanks for the feedback. It makes sense to remove the equalfold and just do a tolower on the host. I'll get this updated.

[aaronhurt]

@pschultz @murphymj25 We good to merge this one? It looks like the comments have been resolved.

[murphymj25]

From my perspective this should be ready to merge. We have been running it in our environments since May without any issues encountered.

[aaronhurt]

@murphymj25 That works for me. Thank you again.