Don't use decode_packet/3 for parsing the request-line

First step in making all methods and header names binaries to
get rid of many inconsistencies caused by decode_packet/3.

Methods are all binary now. Note that since they are case
sensitive, the usual methods become <<"GET">>, <<"POST">> and so on.

5 files changed, 131 insertions, 91 deletions

diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl
index f3457dc..83a67fe 100644
--- a/src/cowboy_http.erl
+++ b/src/cowboy_http.erl
@@ -17,6 +17,7 @@
 -module(cowboy_http).
 
 %% Parsing.
+-export([request_line/1]).
 -export([list/2]).
 -export([nonempty_list/2]).
 -export([content_type/1]).
@@ -50,8 +51,6 @@
 -export([urlencode/2]).
 -export([x_www_form_urlencoded/2]).
 
--type method() :: 'OPTIONS' | 'GET' | 'HEAD'
-	| 'POST' | 'PUT' | 'DELETE' | 'TRACE' | binary().
 -type uri() :: '*' | {absoluteURI, http | https, Host::binary(),
 	Port::integer() | undefined, Path::binary()}
 	| {scheme, Scheme::binary(), binary()}
@@ -73,7 +72,6 @@
 -type headers() :: [{header(), iodata()}].
 -type status() :: non_neg_integer() | binary().
 
--export_type([method/0]).
 -export_type([uri/0]).
 -export_type([version/0]).
 -export_type([header/0]).
@@ -86,6 +84,46 @@
 
 %% Parsing.
 
+%% @doc Parse a request-line.
+-spec request_line(binary())
+	-> {binary(), binary(), version()} | {error, badarg}.
+request_line(Data) ->
+	token(Data,
+		fun (Rest, Method) ->
+			whitespace(Rest,
+				fun (Rest2) ->
+					uri_to_abspath(Rest2,
+						fun (Rest3, AbsPath) ->
+							whitespace(Rest3,
+								fun (<< "HTTP/", Maj, ".", Min, _/binary >>)
+											when Maj >= $0, Maj =< $9,
+												Min >= $0, Min =< $9 ->
+										{Method, AbsPath, {Maj - $0, Min - $0}};
+									(_) ->
+										{error, badarg}
+								end)
+						end)
+				end)
+		end).
+
+%% We just want to extract the path/qs and skip everything else.
+%% We do not really parse the URI, nor do we need to.
+uri_to_abspath(Data, Fun) ->
+	case binary:split(Data, <<" ">>) of
+		[_] -> %% We require the HTTP version.
+			{error, badarg};
+		[URI, Rest] ->
+			case binary:split(URI, <<"://">>) of
+				[_] -> %% Already is a path or "*".
+					Fun(Rest, URI);
+				[_, NoScheme] ->
+					case binary:split(NoScheme, <<"/">>) of
+						[_] -> <<"/">>;
+						[_, NoHost] -> Fun(Rest, << "/", NoHost/binary >>)
+					end
+			end
+	end.
+
 %% @doc Parse a non-empty list of the given type.
 -spec nonempty_list(binary(), fun()) -> [any(), ...] | {error, badarg}.
 nonempty_list(Data, Fun) ->
diff --git a/src/cowboy_protocol.erl b/src/cowboy_protocol.erl
index 2e734c5..4caa00b 100644
--- a/src/cowboy_protocol.erl
+++ b/src/cowboy_protocol.erl
@@ -113,19 +113,6 @@ init(ListenerPid, Socket, Transport, Opts) ->
 		timeout=Timeout, onrequest=OnRequest, onresponse=OnResponse,
 		urldecode=URLDec}).
 
-%% @private
--spec parse_request(#state{}) -> ok.
-%% We limit the length of the Request-line to MaxLength to avoid endlessly
-%% reading from the socket and eventually crashing.
-parse_request(State=#state{buffer=Buffer, max_line_length=MaxLength}) ->
-	case erlang:decode_packet(http_bin, Buffer, []) of
-		{ok, Request, Rest} -> request(Request, State#state{buffer=Rest});
-		{more, _Length} when byte_size(Buffer) > MaxLength ->
-			error_terminate(413, State);
-		{more, _Length} -> wait_request(State);
-		{error, _Reason} -> error_terminate(400, State)
-	end.
-
 -spec wait_request(#state{}) -> ok.
 wait_request(State=#state{socket=Socket, transport=Transport,
 		timeout=T, buffer=Buffer}) ->
@@ -135,48 +122,56 @@ wait_request(State=#state{socket=Socket, transport=Transport,
 		{error, _Reason} -> terminate(State)
 	end.
 
--spec request({http_request, cowboy_http:method(), cowboy_http:uri(),
-	cowboy_http:version()}, #state{}) -> ok.
-request({http_request, _Method, _URI, Version}, State)
+%% @private
+-spec parse_request(#state{}) -> ok.
+%% We limit the length of the Request-line to MaxLength to avoid endlessly
+%% reading from the socket and eventually crashing.
+parse_request(State=#state{buffer=Buffer, max_line_length=MaxLength,
+		req_empty_lines=ReqEmpty, max_empty_lines=MaxEmpty}) ->
+	case binary:split(Buffer, <<"\r\n">>) of
+		[_] when byte_size(Buffer) > MaxLength ->
+			error_terminate(413, State);
+		[<< "\n", _/binary >>] ->
+			error_terminate(400, State);
+		[_] ->
+			wait_request(State);
+		[<<>>, _] when ReqEmpty =:= MaxEmpty ->
+			error_terminate(400, State);
+		[<<>>, Rest] ->
+			parse_request(State#state{
+				buffer=Rest, req_empty_lines=ReqEmpty + 1});
+		[RequestLine, Rest] ->
+			case cowboy_http:request_line(RequestLine) of
+				{Method, AbsPath, Version} ->
+					request(State#state{buffer=Rest}, Method, AbsPath, Version);
+				{error, _} ->
+					error_terminate(400, State)
+			end
+	end.
+
+-spec request(#state{}, binary(), binary(), cowboy_http:version()) -> ok.
+request(State, _, _, Version)
 		when Version =/= {1, 0}, Version =/= {1, 1} ->
 	error_terminate(505, State);
-%% We still receive the original Host header.
-request({http_request, Method, {absoluteURI, _Scheme, _Host, _Port, Path},
-		Version}, State) ->
-	request({http_request, Method, {abs_path, Path}, Version}, State);
-request({http_request, Method, {abs_path, AbsPath}, Version},
-		State=#state{socket=Socket, transport=Transport,
-		req_keepalive=Keepalive, max_keepalive=MaxKeepalive,
-		onresponse=OnResponse, urldecode={URLDecFun, URLDecArg}=URLDec}) ->
-	URLDecode = fun(Bin) -> URLDecFun(Bin, URLDecArg) end,
-	{PathTokens, RawPath, Qs}
-		= cowboy_dispatcher:split_path(AbsPath, URLDecode),
-	ConnAtom = if Keepalive < MaxKeepalive -> version_to_connection(Version);
-		true -> close
-	end,
-	parse_header(cowboy_req:new(Socket, Transport, ConnAtom, Method, Version,
-		RawPath, Qs, OnResponse, URLDec), State#state{path_tokens=PathTokens});
-request({http_request, Method, '*', Version},
-		State=#state{socket=Socket, transport=Transport,
-		req_keepalive=Keepalive, max_keepalive=MaxKeepalive,
-		onresponse=OnResponse, urldecode=URLDec}) ->
-	ConnAtom = if Keepalive < MaxKeepalive -> version_to_connection(Version);
-		true -> close
-	end,
-	parse_header(cowboy_req:new(Socket, Transport, ConnAtom, Method, Version,
-		<<"*">>, <<>>, OnResponse, URLDec), State#state{path_tokens='*'});
-request({http_request, _Method, _URI, _Version}, State) ->
-	error_terminate(501, State);
-request({http_error, <<"\r\n">>},
-		State=#state{req_empty_lines=N, max_empty_lines=N}) ->
-	error_terminate(400, State);
-request({http_error, <<"\r\n">>}, State=#state{req_empty_lines=N}) ->
-	parse_request(State#state{req_empty_lines=N + 1});
-request(_Any, State) ->
-	error_terminate(400, State).
-
--spec parse_header(cowboy_req:req(), #state{}) -> ok.
-parse_header(Req, State=#state{buffer=Buffer, max_line_length=MaxLength}) ->
+request(State=#state{socket=Socket, transport=Transport,
+		onresponse=OnResponse, urldecode=URLDec},
+		Method, <<"*">>, Version) ->
+	Connection = version_to_connection(State, Version),
+	parse_header(State#state{path_tokens= '*'},
+		cowboy_req:new(Socket, Transport, Connection, Method, Version,
+			<<"*">>, <<>>, OnResponse, URLDec));
+request(State=#state{socket=Socket, transport=Transport,
+		onresponse=OnResponse, urldecode=URLDec={URLDecFun, URLDecArg}},
+		Method, AbsPath, Version) ->
+	Connection = version_to_connection(State, Version),
+	{PathTokens, Path, Qs} = cowboy_dispatcher:split_path(AbsPath,
+		fun(Bin) -> URLDecFun(Bin, URLDecArg) end),
+	parse_header(State#state{path_tokens=PathTokens},
+		cowboy_req:new(Socket, Transport, Connection, Method, Version,
+			Path, Qs, OnResponse, URLDec)).
+
+-spec parse_header(#state{}, cowboy_req:req()) -> ok.
+parse_header(State=#state{buffer=Buffer, max_line_length=MaxLength}, Req) ->
 	case erlang:decode_packet(httph_bin, Buffer, []) of
 		{ok, Header, Rest} -> header(Header, Req, State#state{buffer=Rest});
 		{more, _Length} when byte_size(Buffer) > MaxLength ->
@@ -189,8 +184,8 @@ parse_header(Req, State=#state{buffer=Buffer, max_line_length=MaxLength}) ->
 wait_header(Req, State=#state{socket=Socket,
 		transport=Transport, timeout=T, buffer=Buffer}) ->
 	case Transport:recv(Socket, 0, T) of
-		{ok, Data} -> parse_header(Req, State#state{
-			buffer= << Buffer/binary, Data/binary >>});
+		{ok, Data} -> parse_header(State#state{
+			buffer= << Buffer/binary, Data/binary >>}, Req);
 		{error, timeout} -> error_terminate(408, State);
 		{error, closed} -> terminate(State)
 	end.
@@ -203,24 +198,24 @@ header({http_header, _I, 'Host', _R, RawHost}, Req,
 	case catch cowboy_dispatcher:split_host(RawHost2) of
 		{HostTokens, Host, undefined} ->
 			Port = default_port(Transport:name()),
-			parse_header(cowboy_req:set_host(Host, Port, RawHost, Req),
-				State#state{host_tokens=HostTokens});
+			parse_header(State#state{host_tokens=HostTokens},
+				cowboy_req:set_host(Host, Port, RawHost, Req));
 		{HostTokens, Host, Port} ->
-			parse_header(cowboy_req:set_host(Host, Port, RawHost, Req),
-				State#state{host_tokens=HostTokens});
+			parse_header(State#state{host_tokens=HostTokens},
+				cowboy_req:set_host(Host, Port, RawHost, Req));
 		{'EXIT', _Reason} ->
 			error_terminate(400, State)
 	end;
 %% Ignore Host headers if we already have it.
 header({http_header, _I, 'Host', _R, _V}, Req, State) ->
-	parse_header(Req, State);
+	parse_header(State, Req);
 header({http_header, _I, 'Connection', _R, Connection}, Req,
 		State=#state{req_keepalive=Keepalive, max_keepalive=MaxKeepalive})
 		when Keepalive < MaxKeepalive ->
-	parse_header(cowboy_req:set_connection(Connection, Req), State);
+	parse_header(State, cowboy_req:set_connection(Connection, Req));
 header({http_header, _I, Field, _R, Value}, Req, State) ->
 	Field2 = format_header(Field),
-	parse_header(cowboy_req:add_header(Field2, Value, Req), State);
+	parse_header(State, cowboy_req:add_header(Field2, Value, Req));
 %% The Host header is required in HTTP/1.1 and optional in HTTP/1.0.
 header(http_eoh, Req, State=#state{host_tokens=undefined,
 		buffer=Buffer, transport=Transport}) ->
@@ -431,7 +426,7 @@ error_terminate(Code, State=#state{socket=Socket, transport=Transport,
 		{cowboy_req, resp_sent} -> ok
 	after 0 ->
 		_ = cowboy_req:reply(Code, cowboy_req:new(Socket, Transport,
-			close, 'GET', {1, 1}, <<>>, <<>>, OnResponse, undefined)),
+			close, <<"GET">>, {1, 1}, <<>>, <<>>, OnResponse, undefined)),
 		ok
 	end,
 	terminate(State).
@@ -443,9 +438,15 @@ terminate(#state{socket=Socket, transport=Transport}) ->
 
 %% Internal.
 
--spec version_to_connection(cowboy_http:version()) -> keepalive | close.
-version_to_connection({1, 1}) -> keepalive;
-version_to_connection(_Any) -> close.
+-spec version_to_connection(#state{}, cowboy_http:version())
+	-> keepalive | close.
+version_to_connection(#state{req_keepalive=Keepalive,
+		max_keepalive=MaxKeepalive}, _) when Keepalive >= MaxKeepalive ->
+	close;
+version_to_connection(_, {1, 1}) ->
+	keepalive;
+version_to_connection(_, _) ->
+	close.
 
 -spec default_port(atom()) -> 80 | 443.
 default_port(ssl) -> 443;
diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl
index f8e0b6a..8d2cd98 100644
--- a/src/cowboy_req.erl
+++ b/src/cowboy_req.erl
@@ -130,7 +130,7 @@
 
 	%% Request.
 	pid = undefined :: pid(),
-	method = 'GET' :: cowboy_http:method(),
+	method = <<"GET">> :: binary(),
 	version = {1, 1} :: cowboy_http:version(),
 	peer = undefined :: undefined | {inet:ip_address(), inet:port_number()},
 	host = undefined :: undefined | binary(),
@@ -172,7 +172,7 @@
 %% This function takes care of setting the owner's pid to self().
 %% @private
 -spec new(inet:socket(), module(), keepalive | close,
-	cowboy_http:method(), cowboy_http:version(), binary(), binary(),
+	binary(), cowboy_http:version(), binary(), binary(),
 	undefined | fun(), undefined | {fun(), atom()})
 	-> req().
 new(Socket, Transport, Connection, Method, Version, Path, Qs,
@@ -182,7 +182,7 @@ new(Socket, Transport, Connection, Method, Version, Path, Qs,
 		onresponse=OnResponse, urldecode=URLDecode}.
 
 %% @doc Return the HTTP method of the request.
--spec method(Req) -> {cowboy_http:method(), Req} when Req::req().
+-spec method(Req) -> {binary(), Req} when Req::req().
 method(Req) ->
 	{Req#http_req.method, Req}.
 
@@ -878,7 +878,7 @@ reply(Status, Headers, Body, Req=#http_req{socket=Socket, transport=Transport,
 		{<<"Date">>, cowboy_clock:rfc1123()},
 		{<<"Server">>, <<"Cowboy">>}
 	|HTTP11Headers], Req),
-	if	Method =:= 'HEAD' -> ok;
+	if	Method =:= <<"HEAD">> -> ok;
 		ReplyType =:= hook -> ok; %% Hook replied for us, stop there.
 		true ->
 			case Body of
@@ -919,7 +919,7 @@ chunked_reply(Status, Headers, Req=#http_req{
 %%
 %% A chunked reply must have been initiated before calling this function.
 -spec chunk(iodata(), req()) -> ok | {error, atom()}.
-chunk(_Data, #http_req{socket=_Socket, transport=_Transport, method='HEAD'}) ->
+chunk(_Data, #http_req{method= <<"HEAD">>}) ->
 	ok;
 chunk(Data, #http_req{socket=Socket, transport=Transport, version={1, 0}}) ->
 	Transport:send(Socket, Data);
@@ -950,7 +950,7 @@ ensure_response(Req=#http_req{resp_state=waiting}, Status) ->
 	_ = reply(Status, [], [], Req),
 	ok;
 %% Terminate the chunked body for HTTP/1.1 only.
-ensure_response(#http_req{method='HEAD', resp_state=chunks}, _) ->
+ensure_response(#http_req{method= <<"HEAD">>, resp_state=chunks}, _) ->
 	ok;
 ensure_response(#http_req{version={1, 0}, resp_state=chunks}, _) ->
 	ok;
diff --git a/src/cowboy_rest.erl b/src/cowboy_rest.erl
index da52ffe..f084a6c 100644
--- a/src/cowboy_rest.erl
+++ b/src/cowboy_rest.erl
@@ -23,7 +23,7 @@
 -export([upgrade/4]).
 
 -record(state, {
-	method = undefined :: cowboy_http:method(),
+	method = undefined :: binary(),
 
 	%% Handler.
 	handler :: atom(),
@@ -87,9 +87,10 @@ service_available(Req, State) ->
 %% known_methods/2 should return a list of atoms or binary methods.
 known_methods(Req, State=#state{method=Method}) ->
 	case call(Req, State, known_methods) of
-		no_call when Method =:= 'HEAD'; Method =:= 'GET'; Method =:= 'POST';
-					Method =:= 'PUT'; Method =:= 'DELETE'; Method =:= 'TRACE';
-					Method =:= 'CONNECT'; Method =:= 'OPTIONS' ->
+		no_call when Method =:= <<"HEAD">>; Method =:= <<"GET">>;
+				Method =:= <<"POST">>; Method =:= <<"PUT">>;
+				Method =:= <<"DELETE">>; Method =:= <<"TRACE">>;
+				Method =:= <<"CONNECT">>; Method =:= <<"OPTIONS">> ->
 			next(Req, State, fun uri_too_long/2);
 		no_call ->
 			next(Req, State, 501);
@@ -109,10 +110,10 @@ uri_too_long(Req, State) ->
 %% allowed_methods/2 should return a list of atoms or binary methods.
 allowed_methods(Req, State=#state{method=Method}) ->
 	case call(Req, State, allowed_methods) of
-		no_call when Method =:= 'HEAD'; Method =:= 'GET' ->
+		no_call when Method =:= <<"HEAD">>; Method =:= <<"GET">> ->
 			next(Req, State, fun malformed_request/2);
 		no_call ->
-			method_not_allowed(Req, State, ['GET', 'HEAD']);
+			method_not_allowed(Req, State, [<<"GET">>, <<"HEAD">>]);
 		{halt, Req2, HandlerState} ->
 			terminate(Req2, State#state{handler_state=HandlerState});
 		{List, Req2, HandlerState} ->
@@ -172,7 +173,7 @@ valid_entity_length(Req, State) ->
 
 %% If you need to add additional headers to the response at this point,
 %% you should do it directly in the options/2 call using set_resp_headers.
-options(Req, State=#state{method='OPTIONS'}) ->
+options(Req, State=#state{method= <<"OPTIONS">>}) ->
 	case call(Req, State, options) of
 		{halt, Req2, HandlerState} ->
 			terminate(Req2, State#state{handler_state=HandlerState});
@@ -542,7 +543,7 @@ if_none_match(Req, State, EtagsList) ->
 	end.
 
 precondition_is_head_get(Req, State=#state{method=Method})
-		when Method =:= 'HEAD'; Method =:= 'GET' ->
+		when Method =:= <<"HEAD">>; Method =:= <<"GET">> ->
 	not_modified(Req, State);
 precondition_is_head_get(Req, State) ->
 	precondition_failed(Req, State).
@@ -584,7 +585,7 @@ not_modified(Req, State) ->
 precondition_failed(Req, State) ->
 	respond(Req, State, 412).
 
-is_put_to_missing_resource(Req, State=#state{method='PUT'}) ->
+is_put_to_missing_resource(Req, State=#state{method= <<"PUT">>}) ->
 	moved_permanently(Req, State, fun is_conflict/2);
 is_put_to_missing_resource(Req, State) ->
 	previously_existed(Req, State).
@@ -626,7 +627,7 @@ moved_temporarily(Req, State) ->
 			is_post_to_missing_resource(Req, State, 410)
 	end.
 
-is_post_to_missing_resource(Req, State=#state{method='POST'}, OnFalse) ->
+is_post_to_missing_resource(Req, State=#state{method= <<"POST">>}, OnFalse) ->
 	allow_missing_post(Req, State, OnFalse);
 is_post_to_missing_resource(Req, State, OnFalse) ->
 	respond(Req, State, OnFalse).
@@ -634,14 +635,14 @@ is_post_to_missing_resource(Req, State, OnFalse) ->
 allow_missing_post(Req, State, OnFalse) ->
 	expect(Req, State, allow_missing_post, true, fun post_is_create/2, OnFalse).
 
-method(Req, State=#state{method='DELETE'}) ->
+method(Req, State=#state{method= <<"DELETE">>}) ->
 	delete_resource(Req, State);
-method(Req, State=#state{method='POST'}) ->
+method(Req, State=#state{method= <<"POST">>}) ->
 	post_is_create(Req, State);
-method(Req, State=#state{method='PUT'}) ->
+method(Req, State=#state{method= <<"PUT">>}) ->
 	is_conflict(Req, State);
 method(Req, State=#state{method=Method})
-		when Method =:= 'GET'; Method =:= 'HEAD' ->
+		when Method =:= <<"GET">>; Method =:= <<"HEAD">> ->
 	set_resp_body(Req, State);
 method(Req, State) ->
 	multiple_choices(Req, State).
diff --git a/src/cowboy_static.erl b/src/cowboy_static.erl
index aaf798c..5450115 100644
--- a/src/cowboy_static.erl
+++ b/src/cowboy_static.erl
@@ -247,9 +247,9 @@ rest_init(Req, Opts) ->
 
 %% @private Only allow GET and HEAD requests on files.
 -spec allowed_methods(Req, #state{})
-	-> {[atom()], Req, #state{}} when Req::cowboy_req:req().
+	-> {[binary()], Req, #state{}} when Req::cowboy_req:req().
 allowed_methods(Req, State) ->
-	{['GET', 'HEAD'], Req, State}.
+	{[<<"GET">>, <<"HEAD">>], Req, State}.
 
 %% @private
 -spec malformed_request(Req, #state{})