From 02a889d7a640d0ac990b3b7c6bcc05c4f26a89fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Thu, 10 Oct 2019 14:40:56 +0200 Subject: Fix cookie_opts() type in code and documentation It's supposed to be a map, not a proplist. --- doc/src/manual/cow_cookie.asciidoc | 16 ++--- doc/src/manual/cow_cookie.setcookie.asciidoc | 2 +- src/cow_cookie.erl | 96 ++++++++++++---------------- 3 files changed, 51 insertions(+), 63 deletions(-) diff --git a/doc/src/manual/cow_cookie.asciidoc b/doc/src/manual/cow_cookie.asciidoc index 0717299..2c33104 100644 --- a/doc/src/manual/cow_cookie.asciidoc +++ b/doc/src/manual/cow_cookie.asciidoc @@ -20,14 +20,14 @@ and manipulating cookie headers. [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 diff --git a/doc/src/manual/cow_cookie.setcookie.asciidoc b/doc/src/manual/cow_cookie.setcookie.asciidoc index ab2d466..96ddcac 100644 --- a/doc/src/manual/cow_cookie.setcookie.asciidoc +++ b/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, #{ http_only => true, - secure => true + secure => true }). ---- diff --git a/src/cow_cookie.erl b/src/cow_cookie.erl index 199b64b..6fd9ff3 100644 --- a/src/cow_cookie.erl +++ b/src/cow_cookie.erl @@ -17,11 +17,14 @@ -export([parse_cookie/1]). -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]). %% @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>>]), nomatch = binary:match(iolist_to_binary(Value), [<<$,>>, <<$;>>, <<$\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). setcookie_test_() -> %% {Name, Value, Opts, Result} Tests = [ {<<"Customer">>, <<"WILE_E_COYOTE">>, - [{http_only, true}, {domain, <<"acme.com">>}], + #{http_only => true, domain => <<"acme.com">>}, <<"Customer=WILE_E_COYOTE; Version=1; " "Domain=acme.com; HttpOnly">>}, {<<"Customer">>, <<"WILE_E_COYOTE">>, - [{path, <<"/acme">>}], + #{path => <<"/acme">>}, <<"Customer=WILE_E_COYOTE; Version=1; Path=/acme">>}, {<<"Customer">>, <<"WILE_E_COYOTE">>, - [{secure, true}], + #{secure => true}, <<"Customer=WILE_E_COYOTE; Version=1; Secure">>}, {<<"Customer">>, <<"WILE_E_COYOTE">>, - [{secure, false}, {http_only, false}], + #{secure => false, http_only => false}, <<"Customer=WILE_E_COYOTE; Version=1">>}, {<<"Customer">>, <<"WILE_E_COYOTE">>, - [{same_site, lax}], + #{same_site => lax}, <<"Customer=WILE_E_COYOTE; Version=1; SameSite=Lax">>}, {<<"Customer">>, <<"WILE_E_COYOTE">>, - [{same_site, strict}], + #{same_site => strict}, <<"Customer=WILE_E_COYOTE; Version=1; SameSite=Strict">>}, {<<"Customer">>, <<"WILE_E_COYOTE">>, - [{path, <<"/acme">>}, {badoption, <<"negatory">>}], + #{path => <<"/acme">>, badoption => <<"negatory">>}, <<"Customer=WILE_E_COYOTE; Version=1; Path=/acme">>} ], [{R, fun() -> R = iolist_to_binary(setcookie(N, V, O)) end} @@ -264,20 +252,20 @@ setcookie_max_age_test() -> <<" Expires=", _/binary>>, <<" Max-Age=111">>, <<" 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, [<<"Customer=WILE_E_COYOTE">>, <<" Version=1">>, <<" Expires=", _/binary>>, <<" Max-Age=86417">>] = F(<<"Customer">>, <<"WILE_E_COYOTE">>, - [{max_age, 86417}]), + #{max_age => 86417}), ok. setcookie_failures_test_() -> F = fun(N, V) -> - try setcookie(N, V, []) of + try setcookie(N, V, #{}) of _ -> false catch _:_ -> -- cgit v1.2.3