Browse Source

Fix cookie_opts() type in code and documentation

It's supposed to be a map, not a proplist.
Loïc Hoguin 5 years ago
parent
commit
02a889d7a6

+ 8 - 8
doc/src/manual/cow_cookie.asciidoc

@@ -20,14 +20,14 @@ and manipulating cookie headers.
 
 
 [source,erlang]
 [source,erlang]
 ----
 ----
-cookie_opts() :: [Option]
-
-Option :: {domain, binary()}
-        | {http_only, boolean()}
-        | {max_age, non_neg_integer()}
-        | {path, binary()}
-        | {same_site, lax | strict}
-        | {secure, boolean()}
+cookie_opts() :: #{
+    domain    => binary(),
+    http_only => boolean(),
+    max_age   => non_neg_integer(),
+    path      => binary(),
+    same_site => lax | strict,
+    secure    => boolean()
+}
 ----
 ----
 
 
 Options for the set-cookie header. They are added to the
 Options for the set-cookie header. They are added to the

+ 1 - 1
doc/src/manual/cow_cookie.setcookie.asciidoc

@@ -45,7 +45,7 @@ An iolist with the generated set-cookie header value.
 ----
 ----
 SetCookie = cow_cookie:setcookie(<<"sessionid">>, ID, #{
 SetCookie = cow_cookie:setcookie(<<"sessionid">>, ID, #{
     http_only => true,
     http_only => true,
-    secure => true
+    secure    => true
 }).
 }).
 ----
 ----
 
 

+ 42 - 54
src/cow_cookie.erl

@@ -17,11 +17,14 @@
 -export([parse_cookie/1]).
 -export([parse_cookie/1]).
 -export([setcookie/3]).
 -export([setcookie/3]).
 
 
--type cookie_option() :: {max_age, non_neg_integer()}
-	| {domain, binary()} | {path, binary()}
-	| {secure, boolean()} | {http_only, boolean()}
-	| {same_site, lax | strict}.
--type cookie_opts() :: [cookie_option()].
+-type cookie_opts() :: #{
+	domain => binary(),
+	http_only => boolean(),
+	max_age => non_neg_integer(),
+	path => binary(),
+	same_site => lax | strict,
+	secure => boolean()
+}.
 -export_type([cookie_opts/0]).
 -export_type([cookie_opts/0]).
 
 
 %% @doc Parse a cookie header string and return a list of key/values.
 %% @doc Parse a cookie header string and return a list of key/values.
@@ -186,69 +189,54 @@ setcookie(Name, Value, Opts) ->
 			<<$\s>>, <<$\t>>, <<$\r>>, <<$\n>>, <<$\013>>, <<$\014>>]),
 			<<$\s>>, <<$\t>>, <<$\r>>, <<$\n>>, <<$\013>>, <<$\014>>]),
 	nomatch = binary:match(iolist_to_binary(Value), [<<$,>>, <<$;>>,
 	nomatch = binary:match(iolist_to_binary(Value), [<<$,>>, <<$;>>,
 			<<$\s>>, <<$\t>>, <<$\r>>, <<$\n>>, <<$\013>>, <<$\014>>]),
 			<<$\s>>, <<$\t>>, <<$\r>>, <<$\n>>, <<$\013>>, <<$\014>>]),
-	MaxAgeBin = case lists:keyfind(max_age, 1, Opts) of
-		false -> <<>>;
-		{_, 0} ->
-			%% MSIE requires an Expires date in the past to delete a cookie.
-			<<"; Expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0">>;
-		{_, MaxAge} when is_integer(MaxAge), MaxAge > 0 ->
-			UTC = calendar:universal_time(),
-			Secs = calendar:datetime_to_gregorian_seconds(UTC),
-			Expires = calendar:gregorian_seconds_to_datetime(Secs + MaxAge),
-			[<<"; Expires=">>, cow_date:rfc2109(Expires),
-				<<"; Max-Age=">>, integer_to_list(MaxAge)]
-	end,
-	DomainBin = case lists:keyfind(domain, 1, Opts) of
-		false -> <<>>;
-		{_, Domain} -> [<<"; Domain=">>, Domain]
-	end,
-	PathBin = case lists:keyfind(path, 1, Opts) of
-		false -> <<>>;
-		{_, Path} -> [<<"; Path=">>, Path]
-	end,
-	SecureBin = case lists:keyfind(secure, 1, Opts) of
-		false -> <<>>;
-		{_, false} -> <<>>;
-		{_, true} -> <<"; Secure">>
-	end,
-	HttpOnlyBin = case lists:keyfind(http_only, 1, Opts) of
-		false -> <<>>;
-		{_, false} -> <<>>;
-		{_, true} -> <<"; HttpOnly">>
-	end,
-	SameSiteBin = case lists:keyfind(same_site, 1, Opts) of
-		false -> <<>>;
-		{_, lax} -> <<"; SameSite=Lax">>;
-		{_, strict} -> <<"; SameSite=Strict">>
-	end,
-	[Name, <<"=">>, Value, <<"; Version=1">>,
-		MaxAgeBin, DomainBin, PathBin, SecureBin, HttpOnlyBin, SameSiteBin].
+	[Name, <<"=">>, Value, <<"; Version=1">>, attributes(maps:to_list(Opts))].
+
+attributes([]) -> [];
+attributes([{domain, Domain}|Tail]) -> [<<"; Domain=">>, Domain|attributes(Tail)];
+attributes([{http_only, false}|Tail]) -> attributes(Tail);
+attributes([{http_only, true}|Tail]) -> [<<"; HttpOnly">>|attributes(Tail)];
+%% MSIE requires an Expires date in the past to delete a cookie.
+attributes([{max_age, 0}|Tail]) ->
+	[<<"; Expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0">>|attributes(Tail)];
+attributes([{max_age, MaxAge}|Tail]) when is_integer(MaxAge), MaxAge > 0 ->
+	Secs = calendar:datetime_to_gregorian_seconds(calendar:universal_time()),
+	Expires = cow_date:rfc2109(calendar:gregorian_seconds_to_datetime(Secs + MaxAge)),
+	[<<"; Expires=">>, Expires, <<"; Max-Age=">>, integer_to_list(MaxAge)|attributes(Tail)];
+attributes([Opt={max_age, _}|_]) ->
+	error({badarg, Opt});
+attributes([{path, Path}|Tail]) -> [<<"; Path=">>, Path|attributes(Tail)];
+attributes([{secure, false}|Tail]) -> attributes(Tail);
+attributes([{secure, true}|Tail]) -> [<<"; Secure">>|attributes(Tail)];
+attributes([{same_site, lax}|Tail]) -> [<<"; SameSite=Lax">>|attributes(Tail)];
+attributes([{same_site, strict}|Tail]) -> [<<"; SameSite=Strict">>|attributes(Tail)];
+%% Skip unknown options.
+attributes([_|Tail]) -> attributes(Tail).
 
 
 -ifdef(TEST).
 -ifdef(TEST).
 setcookie_test_() ->
 setcookie_test_() ->
 	%% {Name, Value, Opts, Result}
 	%% {Name, Value, Opts, Result}
 	Tests = [
 	Tests = [
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
-			[{http_only, true}, {domain, <<"acme.com">>}],
+			#{http_only => true, domain => <<"acme.com">>},
 			<<"Customer=WILE_E_COYOTE; Version=1; "
 			<<"Customer=WILE_E_COYOTE; Version=1; "
 				"Domain=acme.com; HttpOnly">>},
 				"Domain=acme.com; HttpOnly">>},
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
-			[{path, <<"/acme">>}],
+			#{path => <<"/acme">>},
 			<<"Customer=WILE_E_COYOTE; Version=1; Path=/acme">>},
 			<<"Customer=WILE_E_COYOTE; Version=1; Path=/acme">>},
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
-			[{secure, true}],
+			#{secure => true},
 			<<"Customer=WILE_E_COYOTE; Version=1; Secure">>},
 			<<"Customer=WILE_E_COYOTE; Version=1; Secure">>},
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
-			[{secure, false}, {http_only, false}],
+			#{secure => false, http_only => false},
 			<<"Customer=WILE_E_COYOTE; Version=1">>},
 			<<"Customer=WILE_E_COYOTE; Version=1">>},
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
-			[{same_site, lax}],
+			#{same_site => lax},
 			<<"Customer=WILE_E_COYOTE; Version=1; SameSite=Lax">>},
 			<<"Customer=WILE_E_COYOTE; Version=1; SameSite=Lax">>},
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
-			[{same_site, strict}],
+			#{same_site => strict},
 			<<"Customer=WILE_E_COYOTE; Version=1; SameSite=Strict">>},
 			<<"Customer=WILE_E_COYOTE; Version=1; SameSite=Strict">>},
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
 		{<<"Customer">>, <<"WILE_E_COYOTE">>,
-			[{path, <<"/acme">>}, {badoption, <<"negatory">>}],
+			#{path => <<"/acme">>, badoption => <<"negatory">>},
 			<<"Customer=WILE_E_COYOTE; Version=1; Path=/acme">>}
 			<<"Customer=WILE_E_COYOTE; Version=1; Path=/acme">>}
 	],
 	],
 	[{R, fun() -> R = iolist_to_binary(setcookie(N, V, O)) end}
 	[{R, fun() -> R = iolist_to_binary(setcookie(N, V, O)) end}
@@ -264,20 +252,20 @@ setcookie_max_age_test() ->
 		<<" Expires=", _/binary>>,
 		<<" Expires=", _/binary>>,
 		<<" Max-Age=111">>,
 		<<" Max-Age=111">>,
 		<<" Secure">>] = F(<<"Customer">>, <<"WILE_E_COYOTE">>,
 		<<" Secure">>] = F(<<"Customer">>, <<"WILE_E_COYOTE">>,
-			[{max_age, 111}, {secure, true}]),
-	case catch F(<<"Customer">>, <<"WILE_E_COYOTE">>, [{max_age, -111}]) of
-		{'EXIT', {{case_clause, {max_age, -111}}, _}} -> ok
+			#{max_age => 111, secure => true}),
+	case catch F(<<"Customer">>, <<"WILE_E_COYOTE">>, #{max_age => -111}) of
+		{'EXIT', {{badarg, {max_age, -111}}, _}} -> ok
 	end,
 	end,
 	[<<"Customer=WILE_E_COYOTE">>,
 	[<<"Customer=WILE_E_COYOTE">>,
 		<<" Version=1">>,
 		<<" Version=1">>,
 		<<" Expires=", _/binary>>,
 		<<" Expires=", _/binary>>,
 		<<" Max-Age=86417">>] = F(<<"Customer">>, <<"WILE_E_COYOTE">>,
 		<<" Max-Age=86417">>] = F(<<"Customer">>, <<"WILE_E_COYOTE">>,
-			 [{max_age, 86417}]),
+			 #{max_age => 86417}),
 	ok.
 	ok.
 
 
 setcookie_failures_test_() ->
 setcookie_failures_test_() ->
 	F = fun(N, V) ->
 	F = fun(N, V) ->
-		try setcookie(N, V, []) of
+		try setcookie(N, V, #{}) of
 			_ ->
 			_ ->
 				false
 				false
 		catch _:_ ->
 		catch _:_ ->