Implement gun_cookies:gc/1 and :session_gc/1

2 files changed, 68 insertions, 21 deletions

diff --git a/src/gun_cookies.erl b/src/gun_cookies.erl
index 44f4ab7..5167069 100644
--- a/src/gun_cookies.erl
+++ b/src/gun_cookies.erl
@@ -33,7 +33,7 @@
 	path := binary(),
 	creation_time := calendar:datetime(),
 	last_access_time := calendar:datetime(),
-	expiry_time := calendar:datetime(),
+	expiry_time := calendar:datetime() | infinity,
 	persistent := boolean(),
 	host_only => boolean(),
 	secure_only := boolean(),
@@ -84,11 +84,9 @@ domain_match(String, DomainString) ->
 	end.
 
 -spec gc(Store) -> {ok, Store} when Store::store().
-gc(Store) ->
-%The user agent MUST evict all expired cookies from the cookie store if, at any time, an expired cookie exists in the cookie store.
-%At any time, the user agent MAY “remove excess cookies” from the cookie store if the number of cookies sharing a domain field exceeds some implementation-defined upper bound (such as 50 cookies).
-%At any time, the user agent MAY “remove excess cookies” from the cookie store if the cookie store exceeds some predetermined upper bound (such as 3000 cookies).
-	{todo, Store}.
+gc({Mod, State0}) ->
+	{ok, State} = Mod:gc(State0),
+	{ok, {Mod, State}}.
 
 -spec path_match(binary(), binary()) -> boolean().
 path_match(Path, Path) ->
@@ -140,16 +138,16 @@ query({Mod, State0}, URI) ->
 	{ok, Cookies, {Mod, State}}.
 
 -spec session_gc(Store) -> {ok, Store} when Store::store().
-session_gc(Store) ->
-%When “the current session is over” (as defined by the user agent), the user agent MUST remove from the cookie store all cookies with the persistent-flag set to false.
-	{todo, Store}.
+session_gc({Mod, State0}) ->
+	{ok, State} = Mod:session_gc(State0),
+	{ok, {Mod, State}}.
 
 %% @todo Not cookie_opts()
 %% @todo The given URI must be normalized.
 -spec set_cookie(Store, uri_string:uri_map(), binary(), binary(), cow_cookie:cookie_opts())
 	-> {ok, Store} | {error, any()} when Store::store().
 set_cookie(Store, URI=#{host := Host}, Name, Value, Attrs) ->
-	%% @todo This is where we would add a feature to block cookies (like a blacklist).
+	%% This is where we would add a feature to block cookies (like a blacklist).
 	CurrentTime = erlang:universaltime(),
 	Cookie0 = #{
 		name => Name,
@@ -219,7 +217,7 @@ set_cookie(Store, URI, Attrs, Cookie0) ->
 				secure_only => SecureOnly,
 				http_only => maps:get(http_only, Attrs, false)
 			},
-			%% @todo This is where we would drop cookies from non-HTTP APIs.
+			%% This is where we would drop cookies from non-HTTP APIs.
 			set_cookie1(Store, URI, Attrs, Cookie)
 	end.
 
@@ -248,7 +246,7 @@ set_cookie_secure_match({Mod, State}, Match) ->
 
 set_cookie2(Store, _URI, Attrs, Cookie0) ->
 	Cookie = Cookie0#{same_site => maps:get(same_site, Attrs, none)},
-	%% @todo This is where we would perform the same-site checks.
+	%% This is where we would perform the same-site checks.
 	%%
 	%% It seems that an option would need to be added to Gun
 	%% in order to define the "site for cookies" value. It is
@@ -281,7 +279,7 @@ set_cookie_store(Store0, Cookie) ->
 	Match = maps:with([name, domain, host_only, path], Cookie),
 	case set_cookie_take_exact_match(Store0, Match) of
 		{ok, #{creation_time := CreationTime}, Store} ->
-			%% @todo This is where we would reject a new non-HTTP cookie
+			%% This is where we would reject a new non-HTTP cookie
 			%% if the OldCookie has http_only set to true.
 			store(Store, Cookie#{creation_time => CreationTime});
 		error ->
@@ -306,6 +304,42 @@ store({Mod, State0}, Cookie) ->
 	end.
 
 -ifdef(TEST).
+gc_test() ->
+	URIMap = #{scheme => <<"http">>, host => <<"example.org">>, path => <<"/path/to/resource">>},
+	Store0 = gun_cookies_list:init(),
+	%% Add a cookie that expires in 1 second. GC. Cookie can be retrieved.
+	{ok, N0, V0, A0} = cow_cookie:parse_set_cookie(<<"a=b; Path=/; Max-Age=1">>),
+	{ok, Store1} = set_cookie(Store0, URIMap, N0, V0, A0),
+	{ok, Store2} = gc(Store1),
+	{ok, [_], _} = query(Store2, URIMap),
+	%% Wait 2 seconds. GC. Cookie was removed.
+	timer:sleep(2000),
+	{ok, Store} = gc(Store2),
+	{ok, [], _} = query(Store, URIMap),
+	ok.
+
+gc_expiry_time_infinity_test() ->
+	URIMap = #{scheme => <<"http">>, host => <<"example.org">>, path => <<"/path/to/resource">>},
+	Store0 = gun_cookies_list:init(),
+	%% Add a session cookie. GC. Cookie can be retrieved.
+	{ok, N0, V0, A0} = cow_cookie:parse_set_cookie(<<"a=b; Path=/">>),
+	{ok, Store1} = set_cookie(Store0, URIMap, N0, V0, A0),
+	{ok, Store} = gc(Store1),
+	{ok, [_], _} = query(Store, URIMap),
+	ok.
+
+session_gc_test() ->
+	URIMap = #{scheme => <<"http">>, host => <<"example.org">>, path => <<"/path/to/resource">>},
+	Store0 = gun_cookies_list:init(),
+	%% Add a persistent and a session cookie. GC session cookies. Only persistent can be retrieved.
+	{ok, N0, V0, A0} = cow_cookie:parse_set_cookie(<<"s=s; Path=/">>),
+	{ok, Store1} = set_cookie(Store0, URIMap, N0, V0, A0),
+	{ok, N1, V1, A1} = cow_cookie:parse_set_cookie(<<"p=p; Path=/; Max-Age=999">>),
+	{ok, Store2} = set_cookie(Store1, URIMap, N1, V1, A1),
+	{ok, Store} = session_gc(Store2),
+	{ok, [#{name := <<"p">>}], _} = query(Store, URIMap),
+	ok.
+
 %% Most of the tests for this module are converted from the
 %% Web platform test suite. At the time of writing they could
 %% be found at https://github.com/web-platform-tests/wpt/tree/master/cookies
@@ -525,7 +559,7 @@ wpt_prefix_secure_test_() ->
 		{<<"https">>, <<"__Secure-foo=bar; Secure; Path=/;">>, true},
 		{<<"https">>, <<"__Secure-foo=bar; Secure; Path=/;Max-Age=10">>, true},
 		{<<"https">>, <<"__Secure-foo=bar; Secure; Path=/;HttpOnly">>, true}
-		%% @todo Missing two SameSite cases from prefix/__secure.header.https.
+		%% Missing two SameSite cases from prefix/__secure.header.https. (Not implemented.)
 	],
 	wpt_prefix_common(Tests, <<"__Secure-foo">>).
 
@@ -547,8 +581,8 @@ wpt_prefix_common(Tests, Name) ->
 		end
 	end} || {S, H, Res} <- Tests].
 
-%% @todo WPT: samesite-none-secure/
-%% @todo WPT: samesite/
+%% WPT: samesite-none-secure/ (Not implemented.)
+%% WPT: samesite/ (Not implemented.)
 
 wpt_secure_https_test() ->
 	URIMap = #{
diff --git a/src/gun_cookies_list.erl b/src/gun_cookies_list.erl
index 2b4a666..ccd2292 100644
--- a/src/gun_cookies_list.erl
+++ b/src/gun_cookies_list.erl
@@ -21,10 +21,13 @@
 -export([set_cookie_secure_match/2]).
 -export([set_cookie_take_exact_match/2]).
 -export([store/2]).
+-export([gc/1]).
+-export([session_gc/1]).
 
 -type state() :: #{
 	cookies := [gun_cookies:cookie()]
-	%% @todo Options would go here.
+%% @todo	max_cookies_per_domain => non_neg_integer() | infinity,
+%% @todo	max_cookies => non_neg_integer() | infinity
 }.
 
 -spec init() -> {?MODULE, state()}.
@@ -42,8 +45,6 @@ query(State, _, [], _, CookieList, Cookies) ->
 	{ok, CookieList, State#{cookies => Cookies}};
 query(State, URI=#{scheme := Scheme, host := Host, path := Path},
 		[Cookie|Tail], CurrentTime, CookieList, Acc) ->
-	%% @todo This is probably not correct, says "canonicalized request-host"
-	%% and currently doesn't include the port number, it probably should?
 	Match0 = case Cookie of
 		#{host_only := true, domain := Host} ->
 			true;
@@ -60,9 +61,9 @@ query(State, URI=#{scheme := Scheme, host := Host, path := Path},
 			{#{secure_only := false}, _} -> true;
 			_ -> false
 		end,
-	%% @todo This is where we would check the http_only flag should
+	%% This is where we would check the http_only flag should
 	%% we want to implement a non-HTTP interface.
-	%% @todo This is where we would check for same-site/cross-site.
+	%% This is where we would check for same-site/cross-site.
 	case Match of
 		true ->
 			UpdatedCookie = Cookie#{last_access_time => CurrentTime},
@@ -119,3 +120,15 @@ store(State=#{cookies := Cookies}, NewCookie=#{expiry_time := ExpiryTime}) ->
 		true ->
 			{ok, State#{cookies => [NewCookie|Cookies]}}
 	end.
+
+-spec gc(State) -> {ok, State} when State::state().
+gc(State=#{cookies := Cookies0}) ->
+	CurrentTime = erlang:universaltime(),
+	Cookies = [C || C=#{expiry_time := ExpiryTime} <- Cookies0,
+		(ExpiryTime =:= infinity) orelse (ExpiryTime >= CurrentTime)],
+	{ok, State#{cookies => Cookies}}.
+
+-spec session_gc(State) -> {ok, State} when State::state().
+session_gc(State=#{cookies := Cookies0}) ->
+	Cookies = [C || C=#{persistent := true} <- Cookies0],
+	{ok, State#{cookies => Cookies}}.