Parse connect requests #462

Open
wants to merge 1 commit into
from

Projects

None yet

3 participants

@pib
pib commented Mar 6, 2013

A host header is not required when a processing a request with the CONNECT method, and in fact, Wget (and probably other clients) does not send a host header when sending a CONNECT request.

This pull request adds an extra function to pull out the host and act as if there had been a host header, even if there wasn't one.

This allows cowboy to handle all CONNECT requests, even ones without a host header.

@essen
Nine Nines member

I need to read more about CONNECT. In the meantime can you add a test? And if possible a second test with wget? Thanks!

@pib
pib commented Mar 6, 2013

Starting in section 5.2 of this is a good overview of how CONNECT is supposed to work: http://www.faqs.org/rfcs/rfc2817.html

This draft also shows some usage without a host header: http://tools.ietf.org/html/draft-luotonen-web-proxy-tunneling-01

I'll try to put a couple of tests together in the near future.

@essen
Nine Nines member

Any news? :)

@benoitc

In connect requests, the host in the absolute url in the status, headers are only used generally to authenticate the request.

http://tools.ietf.org/html/draft-luotonen-web-proxy-tunneling-01#section-3.1

@pib

Yes, that is true, the host header is generally not given in connect requests. However, the request parsing code in Cowboy relies on there being a host header, so the easiest way for me to implement this was to pretend like there was a host header when finding the host in the first line of the request.

@pib

I rebased the changes on the latest master and added a couple of tests to the http suite.

@pib

Of course, this isn't how you would use a request with a CONNECT header. An actual usage of it can be seen here: https://github.com/pib/qdecp/blob/master/src/qdecp_cowboy_handler.erl#L9

@essen
Nine Nines member

Looks alright, can you please however squash this into one commit, fix indentation and do alphabetical order for tests? Thanks!

@essen
Nine Nines member

Hi.

@pib

@essen Ok, done! Sorry for the extremely long delay, but I've been busy with other things.

@essen
Nine Nines member

No problem, I'm not the fastest one around either.

@essen essen commented on an outdated diff Aug 29, 2013
src/cowboy_protocol.erl
@@ -219,6 +221,13 @@ parse_uri(<< "https://", Rest/bits >>, State, Method) ->
parse_uri(Buffer, State, Method) ->
parse_uri_path(Buffer, State, Method, <<>>).
+parse_connect_host(<< C, Rest/bits >>, State, Method, SoFar) ->
+ case C of
+ $\r -> error_terminate(400, State);
+ $\s -> parse_version(Rest, State, Method, <<>>, <<>>, [{<<"host">>, SoFar}]);
@essen
essen Aug 29, 2013

Pretty sure you blew the 80 columns limit here.

@essen essen commented on an outdated diff Aug 29, 2013
src/cowboy_protocol.erl
@@ -250,11 +259,14 @@ skip_uri_fragment(<< C, Rest/bits >>, S, M, P, Q) ->
_ -> skip_uri_fragment(Rest, S, M, P, Q)
end.
-parse_version(<< "HTTP/1.1\r\n", Rest/bits >>, S, M, P, Q) ->
- parse_header(Rest, S, M, P, Q, 'HTTP/1.1', []);
-parse_version(<< "HTTP/1.0\r\n", Rest/bits >>, S, M, P, Q) ->
- parse_header(Rest, S, M, P, Q, 'HTTP/1.0', []);
-parse_version(_, State, _, _, _) ->
+parse_version(Buffer, S, M, P, Q) ->
+ parse_version(Buffer, S, M, P, Q, []).
@essen
essen Aug 29, 2013

Please remove that and call the parse_version/6 directly from all other places.

@essen
Nine Nines member

Small things to correct.

@pib pib Properly parse and get host in case of CONNECT request
Add tests for connect requests with or without host header

parse_version Fragment arg was removed
f4883c7
@pib

Alright, those bits are fixed as well.

@essen
Nine Nines member

The host needs to be lowercased because it's case insensitive. It also seems you didn't extract the port from the host. But don't change this yet, there's another PR adding support for ipv6 hosts that's coming in first.

@essen
Nine Nines member

The ipv6 hosts patch is in, so please update your PR, make sure you extract the port and lowercase hostnames properly.

@essen essen modified the milestone: 2.0.0 Aug 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment