Tests and fixes for the generate_etag/2 callback

The return value from the generate_etag/2 callback is expected to be a
binary tagged with either weak or strong. This binary is quoted, and
may be prefixed with W/ before it is set as the value of the ETag header
in the response.

For backwards compatibility with older handlers where the return value
was expected to be a quoted binary a function has been added to parse any
return values that are untagged binaries. All untagged binaries are expected
to be a valid value for the ETag header.

1 files changed, 20 insertions, 11 deletions

diff --git a/src/cowboy_http_rest.erl b/src/cowboy_http_rest.erl
index ee67b36..8bce1d4 100644
--- a/src/cowboy_http_rest.erl
+++ b/src/cowboy_http_rest.erl
@@ -41,7 +41,7 @@
 	charset_a :: undefined | binary(),
 
 	%% Cached resource calls.
-	etag :: undefined | no_call | binary(),
+	etag :: undefined | no_call | {strong | weak, binary()},
 	last_modified :: undefined | no_call | calendar:datetime(),
 	expires :: undefined | no_call | calendar:datetime()
 }).
@@ -487,14 +487,10 @@ if_match_exists(Req, State) ->
 
 if_match(Req, State, EtagsList) ->
 	{Etag, Req2, State2} = generate_etag(Req, State),
-	case Etag of
-		no_call ->
-			precondition_failed(Req2, State2);
-		Etag ->
-			case lists:member(Etag, EtagsList) of
-				true -> if_unmodified_since_exists(Req2, State2);
-				false -> precondition_failed(Req2, State2)
-			end
+	case lists:member(Etag, EtagsList) of
+		true -> if_unmodified_since_exists(Req2, State2);
+		%% Etag may be `undefined' which cannot be a member.
+		false -> precondition_failed(Req2, State2)
 	end.
 
 if_match_musnt_exist(Req, State) ->
@@ -534,7 +530,7 @@ if_none_match_exists(Req, State) ->
 if_none_match(Req, State, EtagsList) ->
 	{Etag, Req2, State2} = generate_etag(Req, State),
 	case Etag of
-		no_call ->
+		undefined ->
 			precondition_failed(Req2, State2);
 		Etag ->
 			case lists:member(Etag, EtagsList) of
@@ -810,10 +806,14 @@ set_resp_etag(Req, State) ->
 			{Req2, State2};
 		Etag ->
 			{ok, Req3} = cowboy_http_req:set_resp_header(
-				<<"Etag">>, Etag, Req2),
+				<<"ETag">>, encode_etag(Etag), Req2),
 			{Req3, State2}
 	end.
 
+-spec encode_etag({strong | weak, binary()}) -> iolist().
+encode_etag({strong, Etag}) -> [$",Etag,$"];
+encode_etag({weak, Etag}) -> ["W/\"",Etag,$"].
+
 set_resp_expires(Req, State) ->
 	{Expires, Req2, State2} = expires(Req, State),
 	case Expires of
@@ -834,6 +834,15 @@ generate_etag(Req, State=#state{etag=undefined}) ->
 	case call(Req, State, generate_etag) of
 		no_call ->
 			{undefined, Req, State#state{etag=no_call}};
+		%% Previously the return value from the generate_etag/2 callback was set
+		%% as the value of the ETag header in the response. Therefore the only
+		%% valid return type was `binary()'. If a handler returns a `binary()'
+		%% it must be mapped to the expected type or it'll always fail to
+		%% compare equal to any entity tags present in the request headers.
+		%% @todo Remove support for binary return values after 0.6.
+		{Etag, Req2, HandlerState} when is_binary(Etag) ->
+			[Etag2] = cowboy_http:entity_tag_match(Etag),
+			{Etag2, Req2, State#state{handler_state=HandlerState, etag=Etag2}};
 		{Etag, Req2, HandlerState} ->
 			{Etag, Req2, State#state{handler_state=HandlerState, etag=Etag}}
 	end;