Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Host() with a port fails to match when the host is supplied in the HTTP Host: header #2

Closed
kisielk opened this issue Oct 5, 2012 · 4 comments

Comments

@kisielk
Copy link
Contributor

kisielk commented Oct 5, 2012

Old issue: http://code.google.com/p/gorilla/issues/detail?id=51

According to section 14.23 of RFC 2616 the Host header can include the port number if the default value of 80 is not used.

See https://github.com/gorilla/mux/blob/master/mux_test.go#L119 for a commented-out test case that fails.

Potential fix is editing getHost() https://github.com/gorilla/mux/blob/master/regexp.go#L237 so it has a flag as to whether to strip off the port information. A new field would need to be probably need to be added somewhere such as routeRegexp.

The main reason I was using Host() match with a port in the first place is for trying to build reverse routes that include the absolute path as opposed to a relative one.

@soul9
Copy link

soul9 commented Dec 16, 2012

An other problem with the Host() function is that it can't handle ipv6 addresses well.
For the "http://[::1]:8080/foo" url, and Host("{host:.+}") regexp, mux.Vars returns map[string]string{"file":"foo", "host":"["}

@dominikh
Copy link

What's the status of this issue? Experimentation makes me believe that currently, one has to call Host() without a port, and that will work even if the Host header includes a port. Is that correct?

Also, in response to @soul9, getHost() should probably use net.SplitHostPort instead of just splitting on a colon, but that's a different issue.

@kisielk
Copy link
Contributor Author

kisielk commented Aug 18, 2014

Honestly I haven't looked in to it since Oct 2012 most likely :)

@stale
Copy link

stale bot commented Dec 9, 2018

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants