Browse Source

Accept sendfile tuple with 0 length in cowboy_req

This will result in no data being sent. It's simply easier to
do this than to have to handle 0 size cases in user code.
Loïc Hoguin 7 years ago
parent
commit
4fd6e2f7cd
4 changed files with 34 additions and 12 deletions
  1. 4 11
      doc/src/manual/cowboy_req.asciidoc
  2. 6 1
      src/cowboy_req.erl
  3. 3 0
      test/handlers/resp_h.erl
  4. 21 0
      test/req_SUITE.erl

+ 4 - 11
doc/src/manual/cowboy_req.asciidoc

@@ -156,7 +156,7 @@ resp_body() :: iodata()
     | {sendfile, Offset, Length, Filename}
     | {sendfile, Offset, Length, Filename}
 
 
 Offset   :: non_neg_integer()
 Offset   :: non_neg_integer()
-Length   :: pos_integer()
+Length   :: non_neg_integer()
 Filename :: file:name_all()
 Filename :: file:name_all()
 ----
 ----
 
 
@@ -180,16 +180,9 @@ order they should be sent:
 Hello world!
 Hello world!
 ----
 ----
 
 
-When using the sendfile tuple, the `Length` value is mandatory
-and must be higher than 0. It is sent with the response in the
-content-length header.
-
-// @todo Make sure we have a test with an empty file...
-// @todo cowboy_static should probably NOT return a sendfile tuple if size is 0.
-
-//%% While sendfile allows a Len of 0 that means "everything past Offset",
-//%% Cowboy expects the real length as it is used as metadata.
-//%% @todo We should probably explicitly reject it.
+Note that the length must be greater than zero for any data
+to be sent. Cowboy will send an empty body when the length
+is zero.
 
 
 == See also
 == See also
 
 

+ 6 - 1
src/cowboy_req.erl

@@ -98,7 +98,7 @@
 %% Cowboy expects the real length as it is used as metadata.
 %% Cowboy expects the real length as it is used as metadata.
 %% @todo We should probably explicitly reject it.
 %% @todo We should probably explicitly reject it.
 -type resp_body() :: iodata()
 -type resp_body() :: iodata()
-	| {sendfile, non_neg_integer(), pos_integer(), file:name_all()}.
+	| {sendfile, non_neg_integer(), non_neg_integer(), file:name_all()}.
 -export_type([resp_body/0]).
 -export_type([resp_body/0]).
 
 
 -type push_opts() :: #{
 -type push_opts() :: #{
@@ -635,6 +635,11 @@ reply(Status, Headers, Req) ->
 	-> Req when Req::req().
 	-> Req when Req::req().
 reply(_, _, _, #{has_sent_resp := _}) ->
 reply(_, _, _, #{has_sent_resp := _}) ->
 	error(function_clause);
 	error(function_clause);
+reply(Status, Headers, Sendfile = {sendfile, _, 0, _}, Req)
+		when is_integer(Status); is_binary(Status) ->
+	do_reply(Status, Headers#{
+		<<"content-length">> => <<"0">>
+	}, <<>>, Req);
 reply(Status, Headers, SendFile = {sendfile, _, Len, _}, Req)
 reply(Status, Headers, SendFile = {sendfile, _, Len, _}, Req)
 		when is_integer(Status); is_binary(Status) ->
 		when is_integer(Status); is_binary(Status) ->
 	do_reply(Status, Headers#{
 	do_reply(Status, Headers#{

+ 3 - 0
test/handlers/resp_h.erl

@@ -58,6 +58,9 @@ do(<<"resp_headers_empty">>, Req, Opts) ->
 do(<<"set_resp_body">>, Req0, Opts) ->
 do(<<"set_resp_body">>, Req0, Opts) ->
 	Arg = cowboy_req:binding(arg, Req0),
 	Arg = cowboy_req:binding(arg, Req0),
 	Req1 = case Arg of
 	Req1 = case Arg of
+		<<"sendfile0">> ->
+			AppFile = code:where_is_file("cowboy.app"),
+			cowboy_req:set_resp_body({sendfile, 0, 0, AppFile}, Req0);
 		<<"sendfile">> ->
 		<<"sendfile">> ->
 			AppFile = code:where_is_file("cowboy.app"),
 			AppFile = code:where_is_file("cowboy.app"),
 			cowboy_req:set_resp_body({sendfile, 0, filelib:file_size(AppFile), AppFile}, Req0);
 			cowboy_req:set_resp_body({sendfile, 0, filelib:file_size(AppFile), AppFile}, Req0);

+ 21 - 0
test/req_SUITE.erl

@@ -509,6 +509,27 @@ set_resp_body(Config) ->
 	{200, _, AppFile} = do_get("/resp/set_resp_body/sendfile", Config),
 	{200, _, AppFile} = do_get("/resp/set_resp_body/sendfile", Config),
 	ok.
 	ok.
 
 
+set_resp_body_sendfile0(Config) ->
+	doc("Response using set_resp_body with a sendfile of length 0."),
+	Path = "/resp/set_resp_body/sendfile0",
+	ConnPid = gun_open(Config),
+	%% First request.
+	Ref1 = gun:get(ConnPid, Path, [{<<"accept-encoding">>, <<"gzip">>}]),
+	{response, IsFin, 200, _} = gun:await(ConnPid, Ref1),
+	{ok, <<>>} = case IsFin of
+		nofin -> gun:await_body(ConnPid, Ref1);
+		fin -> {ok, <<>>}
+	end,
+	%% Second request will confirm everything works as intended.
+	Ref2 = gun:get(ConnPid, Path, [{<<"accept-encoding">>, <<"gzip">>}]),
+	{response, IsFin, 200, _} = gun:await(ConnPid, Ref2),
+	{ok, <<>>} = case IsFin of
+		nofin -> gun:await_body(ConnPid, Ref2);
+		fin -> {ok, <<>>}
+	end,
+	gun:close(ConnPid),
+	ok.
+
 has_resp_header(Config) ->
 has_resp_header(Config) ->
 	doc("Has response header?"),
 	doc("Has response header?"),
 	{200, Headers, <<"OK">>} = do_get("/resp/has_resp_header", Config),
 	{200, Headers, <<"OK">>} = do_get("/resp/has_resp_header", Config),