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

Should include port in host header #28

Closed
woahdae opened this issue Aug 27, 2014 · 4 comments
Closed

Should include port in host header #28

woahdae opened this issue Aug 27, 2014 · 4 comments

Comments

@woahdae
Copy link

woahdae commented Aug 27, 2014

Per section 14.23 of the HTTP spec, gun should include the port along with the hostname in the host header.

the request function in gun_http.erl should have line 196 and line 210 read something like:

false ->
    HostWithPort =
        case Port of
            undefined -> Host;
            80 -> Host;
            _ -> lists:concat(Host, ":", Port)
        end,
    [{<<"host">>, HostWithPort}|Headers2]`

Then you'd have to pass in the port from the main gun loop and change the other protocol request callback methods to match.

I didn't do this because the workaround is just to specify the host header yourself, there isn't a test suite, and I wasn't sure if someone had other opinions. I'm willing to submit some actual code if that's desired, though.

FYI, this is causing issues when we use gun to test a cowboy server, and cowboy uses the host header (apparently) to determine the port, thus passing an incorrect port number (80 instead of 8080).

@essen
Copy link
Member

essen commented Aug 27, 2014

Sounds like an oversight. Yes send a patch! Thanks.

Loïc Hoguin
http://ninenines.eu

-------- Original Message --------
From:Woody Peterson notifications@github.com
Sent:Wed, 27 Aug 2014 05:41:28 +0300
To:extend/gun gun@noreply.github.com
Subject:[gun] Should include port in host header (#28)

Per section 14.23 of the HTTP spec, gun should include the port along with the hostname in the host header.

the request function in gun_http.erl should have line 196 and line 210 read something like:

false -> HostWithPort = case Port of undefined -> Host; 80 -> Host; _ -> lists:concat(Host, ":", Bar) end, [{<<"host">>, HostWithPort}|Headers2]`

Then you'd have to pass in the port from the main gun loop and change the other protocol request callback methods to match.

I didn't do this because the workaround is just to specify the host header yourself, there isn't a test suite, and I wasn't sure if someone had other opinions. I'm willing to submit some actual code if that's desired, though.


Reply to this email directly or view it on GitHub.

@woahdae
Copy link
Author

woahdae commented Aug 28, 2014

Wish I could/knew how to attach a pull request to an existing issue.

Anyways, see #29 for the code.

@essen
Copy link
Member

essen commented Aug 28, 2014

Well I prefer them to be separate. :-) I'll look soon, thanks.

@essen
Copy link
Member

essen commented Mar 29, 2015

Done.

@essen essen closed this as completed Mar 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants