Browse Source

Fix sending of large files with HTTP/2

Also finish implementing the relevant test, getting rid of todos.
Loïc Hoguin 8 years ago
parent
commit
4cbdfdd293
2 changed files with 44 additions and 9 deletions
  1. 20 2
      src/cowboy_http2.erl
  2. 24 7
      test/static_handler_SUITE.erl

+ 20 - 2
src/cowboy_http2.erl

@@ -441,8 +441,13 @@ commands(State=#state{socket=Socket, transport=Transport}, Stream=#stream{id=Str
 %% implementation necessarily varies between HTTP/1.1 and HTTP/2.
 commands(State=#state{socket=Socket, transport=Transport}, Stream=#stream{id=StreamID, local=nofin},
 		[{sendfile, IsFin, Offset, Bytes, Path}|Tail]) ->
-	Transport:send(Socket, cow_http2:data_header(StreamID, IsFin, Bytes)),
-	Transport:sendfile(Socket, Path, Offset, Bytes),
+	%% @todo We currently have a naive implementation without a
+	%% scheduler to prioritize frames that need to be sent.
+	%% A future update will need to queue such data frames
+	%% and only send them when there is nothing currently
+	%% being sent. We would probably also benefit from doing
+	%% asynchronous sends.
+	sendfile(Socket, Transport, StreamID, IsFin, Offset, Bytes, Path, 16384),
 	commands(State, Stream#stream{local=IsFin}, Tail);
 %% @todo sendfile when local!=nofin
 %% Send a push promise.
@@ -513,6 +518,19 @@ send_data(Socket, Transport, StreamID, IsFin, Data, Length) ->
 			Transport:send(Socket, cow_http2:data(StreamID, IsFin, Data))
 	end.
 
+%% @todo This is currently awfully slow. But at least it's correct.
+sendfile(Socket, Transport, StreamID, IsFin, Offset, Bytes, Path, Length) ->
+	if
+		Length < Bytes ->
+			Transport:send(Socket, cow_http2:data_header(StreamID, nofin, Length)),
+			Transport:sendfile(Socket, Path, Offset, Length),
+			sendfile(Socket, Transport, StreamID, IsFin,
+				Offset + Length, Bytes - Length, Path, Length);
+		true ->
+			Transport:send(Socket, cow_http2:data_header(StreamID, IsFin, Bytes)),
+			Transport:sendfile(Socket, Path, Offset, Bytes)
+	end.
+
 -spec terminate(#state{}, _) -> no_return().
 terminate(#state{socket=Socket, transport=Transport,
 		streams=Streams, children=Children}, Reason) ->

+ 24 - 7
test/static_handler_SUITE.erl

@@ -391,13 +391,30 @@ dir_html(Config) ->
 	{_, <<"text/html">>} = lists:keyfind(<<"content-type">>, 1, Headers),
 	ok.
 
-%% @todo This test results in a crash dump.
-%dir_large_file(Config) ->
-%	doc("Get a large file."),
-%	{200, Headers, _} = do_get(config(prefix, Config) ++ "/large.bin", Config),
-%	{_, <<"text/html">>} = lists:keyfind(<<"content-type">>, 1, Headers),
-%% @todo Receive body.
-%	ok.
+dir_large_file(Config) ->
+	doc("Get a large file."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, config(prefix, Config) ++ "/large.bin",
+		[{<<"accept-encoding">>, <<"gzip">>}]),
+	{response, nofin, 200, RespHeaders} = gun:await(ConnPid, Ref),
+	{_, <<"application/octet-stream">>} = lists:keyfind(<<"content-type">>, 1, RespHeaders),
+	Size = 512*1024*1024,
+	{ok, Size} = do_dir_large_file(ConnPid, Ref, 0),
+	ok.
+
+do_dir_large_file(ConnPid, Ref, N) ->
+	receive
+		{gun_data, ConnPid, Ref, nofin, Data} ->
+			do_dir_large_file(ConnPid, Ref, N + byte_size(Data));
+		{gun_data, ConnPid, Ref, fin, Data} ->
+			{ok, N + byte_size(Data)};
+		{gun_error, ConnPid, Ref, Reason} ->
+			{error, Reason};
+		{gun_error, ConnPid, Reason} ->
+			{error, Reason}
+	after 5000 ->
+		{error, timeout}
+	end.
 
 dir_text(Config) ->
 	doc("Get a .txt file. The extension is unknown by default."),