Parse 'Connection' headers as a list of tokens

Replaces the 'Connection' interpretation in cowboy_http_protocol
from raw value to the parsed value, looking for a single token
matching close/keep-alive instead of the whole raw value (which
could contain more than one token, for example with Firefox 6+
using websocket).

Introduce the functions cowboy_http_req:parse_header/2 and /3
to semantically parse the header values and return a proper
Erlang term.

5 files changed, 184 insertions, 20 deletions

diff --git a/include/http.hrl b/include/http.hrl
index 2364daa..3178381 100644
--- a/include/http.hrl
+++ b/include/http.hrl
@@ -58,6 +58,7 @@
 	raw_qs     = undefined :: undefined | binary(),
 	bindings   = undefined :: undefined | cowboy_dispatcher:bindings(),
 	headers    = []        :: http_headers(),
+	p_headers  = []        :: [any()], %% @todo Improve those specs.
 	cookies    = undefined :: undefined | http_cookies(),
 
 	%% Request body.
diff --git a/src/cowboy_http.erl b/src/cowboy_http.erl
new file mode 100644
index 0000000..8d60f82
--- /dev/null
+++ b/src/cowboy_http.erl
@@ -0,0 +1,124 @@
+%% Copyright (c) 2011, Loïc Hoguin <[email protected]>
+%%
+%% Permission to use, copy, modify, and/or distribute this software for any
+%% purpose with or without fee is hereby granted, provided that the above
+%% copyright notice and this permission notice appear in all copies.
+%%
+%% THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+%% WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+%% MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+%% ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+%% WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+%% ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+%% OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+-module(cowboy_http).
+
+%% Parsing.
+-export([parse_tokens_list/1]).
+
+%% Interpretation.
+-export([connection_to_atom/1]).
+
+-include("include/http.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+%% Parsing.
+
+%% @doc Parse a list of tokens, as is often found in HTTP headers.
+%%
+%% From the RFC:
+%% <blockquote>Wherever this construct is used, null elements are allowed,
+%% but do not contribute to the count of elements present.
+%% That is, "(element), , (element) " is permitted, but counts
+%% as only two elements. Therefore, where at least one element is required,
+%% at least one non-null element MUST be present.</blockquote>
+-spec parse_tokens_list(binary()) -> [binary()] | {error, badarg}.
+parse_tokens_list(Value) ->
+	case parse_tokens_list(Value, ws_or_sep, <<>>, []) of
+		{error, badarg} ->
+			{error, badarg};
+		L when length(L) =:= 0 ->
+			{error, badarg};
+		L ->
+			lists:reverse(L)
+	end.
+
+-spec parse_tokens_list(binary(), token | ws | ws_or_sep, binary(),
+	[binary()]) -> [binary()] | {error, badarg}.
+parse_tokens_list(<<>>, token, Token, Acc) ->
+	[Token|Acc];
+parse_tokens_list(<< C, Rest/bits >>, token, Token, Acc)
+		when C =:= $\s; C =:= $\t ->
+	parse_tokens_list(Rest, ws, <<>>, [Token|Acc]);
+parse_tokens_list(<< $,, Rest/bits >>, token, Token, Acc) ->
+	parse_tokens_list(Rest, ws_or_sep, <<>>, [Token|Acc]);
+parse_tokens_list(<< C, Rest/bits >>, token, Token, Acc) ->
+	parse_tokens_list(Rest, token, << Token/binary, C >>, Acc);
+parse_tokens_list(<< C, Rest/bits >>, ws, <<>>, Acc)
+		when C =:= $\s; C =:= $\t ->
+	parse_tokens_list(Rest, ws, <<>>, Acc);
+parse_tokens_list(<< $,, Rest/bits >>, ws, <<>>, Acc) ->
+	parse_tokens_list(Rest, ws_or_sep, <<>>, Acc);
+parse_tokens_list(<<>>, ws_or_sep, <<>>, Acc) ->
+	Acc;
+parse_tokens_list(<< C, Rest/bits >>, ws_or_sep, <<>>, Acc)
+		when C =:= $\s; C =:= $\t ->
+	parse_tokens_list(Rest, ws_or_sep, <<>>, Acc);
+parse_tokens_list(<< $,, Rest/bits >>, ws_or_sep, <<>>, Acc) ->
+	parse_tokens_list(Rest, ws_or_sep, <<>>, Acc);
+parse_tokens_list(<< C, Rest/bits >>, ws_or_sep, <<>>, Acc) ->
+	parse_tokens_list(Rest, token, << C >>, Acc);
+parse_tokens_list(_Value, _State, _Token, _Acc) ->
+	{error, badarg}.
+
+%% Interpretation.
+
+%% @doc Walk through a tokens list and return whether
+%% the connection is keepalive or closed.
+-spec connection_to_atom([binary()]) -> keepalive | close.
+connection_to_atom([]) ->
+	keepalive;
+connection_to_atom([<<"keep-alive">>|_Tail]) ->
+	keepalive;
+connection_to_atom([<<"close">>|_Tail]) ->
+	close;
+connection_to_atom([Connection|Tail]) ->
+	case cowboy_bstr:to_lower(Connection) of
+		<<"close">> -> close;
+		<<"keep-alive">> -> keepalive;
+		_Any -> connection_to_atom(Tail)
+	end.
+
+%% Tests.
+
+-ifdef(TEST).
+
+parse_tokens_list_test_() ->
+	%% {Value, Result}
+	Tests = [
+		{<<>>, {error, badarg}},
+		{<<" ">>, {error, badarg}},
+		{<<" , ">>, {error, badarg}},
+		{<<",,,">>, {error, badarg}},
+		{<<"a b">>, {error, badarg}},
+		{<<"a , , , ">>, [<<"a">>]},
+		{<<" , , , a">>, [<<"a">>]},
+		{<<"a, , b">>, [<<"a">>, <<"b">>]},
+		{<<"close">>, [<<"close">>]},
+		{<<"keep-alive, upgrade">>, [<<"keep-alive">>, <<"upgrade">>]}
+	],
+	[{V, fun() -> R = parse_tokens_list(V) end} || {V, R} <- Tests].
+
+connection_to_atom_test_() ->
+	%% {Tokens, Result}
+	Tests = [
+		{[<<"close">>], close},
+		{[<<"ClOsE">>], close},
+		{[<<"Keep-Alive">>], keepalive},
+		{[<<"Keep-Alive">>, <<"Upgrade">>], keepalive}
+	],
+	[{lists:flatten(io_lib:format("~p", [T])),
+		fun() -> R = connection_to_atom(T) end} || {T, R} <- Tests].
+
+-endif.
diff --git a/src/cowboy_http_protocol.erl b/src/cowboy_http_protocol.erl
index 8f6ab35..dc226a6 100644
--- a/src/cowboy_http_protocol.erl
+++ b/src/cowboy_http_protocol.erl
@@ -158,10 +158,13 @@ header({http_header, _I, 'Host', _R, RawHost}, Req=#http_req{
 %% Ignore Host headers if we already have it.
 header({http_header, _I, 'Host', _R, _V}, Req, State) ->
 	parse_header(Req, State);
-header({http_header, _I, 'Connection', _R, Connection}, Req, State) ->
-	ConnAtom = connection_to_atom(Connection),
-	parse_header(Req#http_req{connection=ConnAtom,
-		headers=[{'Connection', Connection}|Req#http_req.headers]}, State);
+header({http_header, _I, 'Connection', _R, Connection},
+		Req=#http_req{headers=Headers}, State) ->
+	Req2 = Req#http_req{headers=[{'Connection', Connection}|Headers]},
+	{tokens, ConnTokens, Req3}
+		= cowboy_http_req:parse_header('Connection', Req2),
+	ConnAtom = cowboy_http:connection_to_atom(ConnTokens),
+	parse_header(Req3#http_req{connection=ConnAtom}, State);
 header({http_header, _I, Field, _R, Value}, Req, State) ->
 	Field2 = format_header(Field),
 	parse_header(Req#http_req{headers=[{Field2, Value}|Req#http_req.headers]},
@@ -304,18 +307,6 @@ terminate(#state{socket=Socket, transport=Transport}) ->
 version_to_connection({1, 1}) -> keepalive;
 version_to_connection(_Any) -> close.
 
-%% @todo Connection can take more than one value.
--spec connection_to_atom(binary()) -> keepalive | close.
-connection_to_atom(<<"keep-alive">>) ->
-	keepalive;
-connection_to_atom(<<"close">>) ->
-	close;
-connection_to_atom(Connection) ->
-	case cowboy_bstr:to_lower(Connection) of
-		<<"close">> -> close;
-		_Any -> keepalive
-	end.
-
 -spec default_port(atom()) -> 80 | 443.
 default_port(ssl) -> 443;
 default_port(_) -> 80.
diff --git a/src/cowboy_http_req.erl b/src/cowboy_http_req.erl
index 272c8a8..808a108 100644
--- a/src/cowboy_http_req.erl
+++ b/src/cowboy_http_req.erl
@@ -28,6 +28,7 @@
 	qs_val/2, qs_val/3, qs_vals/1, raw_qs/1,
 	binding/2, binding/3, bindings/1,
 	header/2, header/3, headers/1,
+	parse_header/2, parse_header/3,
 	cookie/2, cookie/3, cookies/1
 ]). %% Request API.
 
@@ -182,6 +183,54 @@ header(Name, Req, Default) when is_atom(Name) orelse is_binary(Name) ->
 headers(Req) ->
 	{Req#http_req.headers, Req}.
 
+%% @doc Semantically parse headers.
+%%
+%% When the value isn't found, a proper default value for the type
+%% returned is used as a return value.
+%% @see parse_header/3
+-spec parse_header(http_header(), #http_req{})
+	-> {tokens, [binary()], #http_req{}}
+	 | {undefined, binary(), #http_req{}}
+	 | {error, badarg}.
+parse_header('Connection', Req) ->
+	parse_header('Connection', Req, []);
+parse_header(Name, Req) ->
+	parse_header(Name, Req, undefined).
+
+%% @doc Semantically parse headers.
+%%
+%% When the header is known, a named tuple is returned containing
+%% {Type, P, Req} with Type being the type of value found in P.
+%% For example, the header 'Connection' is a list of tokens, therefore
+%% the value returned will be a list of binary values and Type will be
+%% 'tokens'.
+%%
+%% When the header is known but not found, the tuple {Type, Default, Req}
+%% is returned instead.
+%%
+%% When the header is unknown, the value is returned directly as an
+%% 'undefined' tagged tuple.
+-spec parse_header(http_header(), #http_req{}, any())
+	-> {tokens, [binary()], #http_req{}}
+	 | {undefined, binary(), #http_req{}}
+	 | {error, badarg}.
+parse_header(Name, Req=#http_req{p_headers=PHeaders}, Default)
+		when Name =:= 'Connection' ->
+	case header(Name, Req) of
+		{undefined, Req2} -> {tokens, Default, Req2};
+		{Value, Req2} ->
+			case cowboy_http:parse_tokens_list(Value) of
+				{error, badarg} ->
+					{error, badarg};
+				P ->
+					{tokens, P, Req2#http_req{
+						p_headers=[{Name, P}|PHeaders]}}
+			end
+	end;
+parse_header(Name, Req, Default) ->
+	{Value, Req2} = header(Name, Req, Default),
+	{undefined, Value, Req2}.
+
 %% @equiv cookie(Name, Req, undefined)
 -spec cookie(binary(), #http_req{})
 	-> {binary() | true | undefined, #http_req{}}.
diff --git a/src/cowboy_http_websocket.erl b/src/cowboy_http_websocket.erl
index 2a1f3e4..8b2fd34 100644
--- a/src/cowboy_http_websocket.erl
+++ b/src/cowboy_http_websocket.erl
@@ -76,10 +76,9 @@ upgrade(ListenerPid, Handler, Opts, Req) ->
 %%       instead of having ugly code like this case here.
 -spec websocket_upgrade(#state{}, #http_req{}) -> {ok, #state{}, #http_req{}}.
 websocket_upgrade(State, Req) ->
-	case cowboy_http_req:header('Connection', Req) of
-		{<<"Upgrade">>, Req2} -> ok;
-		{<<"keep-alive, Upgrade">>, Req2} -> ok %% @todo Temp. For Firefox 6.
-	end,
+	{tokens, ConnTokens, Req2}
+		= cowboy_http_req:parse_header('Connection', Req),
+	true = lists:member(<<"Upgrade">>, ConnTokens),
 	{Version, Req3} = cowboy_http_req:header(<<"Sec-Websocket-Version">>, Req2),
 	websocket_upgrade(Version, State, Req3).