From 4fd6e2f7cdca1c1adf1ba0bd76a0702328c380f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= <essen@ninenines.eu>
Date: Mon, 4 Sep 2017 20:48:07 +0200
Subject: 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.
---
 doc/src/manual/cowboy_req.asciidoc | 15 ++++-----------
 src/cowboy_req.erl                 |  7 ++++++-
 test/handlers/resp_h.erl           |  3 +++
 test/req_SUITE.erl                 | 21 +++++++++++++++++++++
 4 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/doc/src/manual/cowboy_req.asciidoc b/doc/src/manual/cowboy_req.asciidoc
index 2cac8d5..7f026c3 100644
--- a/doc/src/manual/cowboy_req.asciidoc
+++ b/doc/src/manual/cowboy_req.asciidoc
@@ -156,7 +156,7 @@ resp_body() :: iodata()
     | {sendfile, Offset, Length, Filename}
 
 Offset   :: non_neg_integer()
-Length   :: pos_integer()
+Length   :: non_neg_integer()
 Filename :: file:name_all()
 ----
 
@@ -180,16 +180,9 @@ order they should be sent:
 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
 
diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl
index 09e1ee5..936f6bb 100644
--- a/src/cowboy_req.erl
+++ b/src/cowboy_req.erl
@@ -98,7 +98,7 @@
 %% Cowboy expects the real length as it is used as metadata.
 %% @todo We should probably explicitly reject it.
 -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]).
 
 -type push_opts() :: #{
@@ -635,6 +635,11 @@ reply(Status, Headers, Req) ->
 	-> Req when Req::req().
 reply(_, _, _, #{has_sent_resp := _}) ->
 	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)
 		when is_integer(Status); is_binary(Status) ->
 	do_reply(Status, Headers#{
diff --git a/test/handlers/resp_h.erl b/test/handlers/resp_h.erl
index 694f04a..9b94e3f 100644
--- a/test/handlers/resp_h.erl
+++ b/test/handlers/resp_h.erl
@@ -58,6 +58,9 @@ do(<<"resp_headers_empty">>, Req, Opts) ->
 do(<<"set_resp_body">>, Req0, Opts) ->
 	Arg = cowboy_req:binding(arg, Req0),
 	Req1 = case Arg of
+		<<"sendfile0">> ->
+			AppFile = code:where_is_file("cowboy.app"),
+			cowboy_req:set_resp_body({sendfile, 0, 0, AppFile}, Req0);
 		<<"sendfile">> ->
 			AppFile = code:where_is_file("cowboy.app"),
 			cowboy_req:set_resp_body({sendfile, 0, filelib:file_size(AppFile), AppFile}, Req0);
diff --git a/test/req_SUITE.erl b/test/req_SUITE.erl
index d155aa8..f7682cc 100644
--- a/test/req_SUITE.erl
+++ b/test/req_SUITE.erl
@@ -509,6 +509,27 @@ set_resp_body(Config) ->
 	{200, _, AppFile} = do_get("/resp/set_resp_body/sendfile", Config),
 	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) ->
 	doc("Has response header?"),
 	{200, Headers, <<"OK">>} = do_get("/resp/has_resp_header", Config),
-- 
cgit v1.2.3