feat: http2 sendfile streaming (#565)

* Stream HTTP/2 sendfile in chunks

* test: cover chunked HTTP/2 sendfile

* http2: make sendfile chunk size configurable

* fix: remove unneeded boolean

* chore: lint

Author: elibosley

lib/bandit.ex

@@ -172,6 +172,8 @@ defmodule Bandit do
     Specified as a tuple of `{count, milliseconds}` where `count` is the maximum number of
     RST_STREAM frames allowed within the time window of `milliseconds`. Defaults to `{500, 10_000}`
     (500 resets per 10 seconds). Setting this to `nil` disables rate limiting
+  * `sendfile_chunk_size`: The maximum number of bytes read per sendfile chunk when streaming
+    HTTP/2 responses. Defaults to 1_048_576 (1 MiB)
   * `default_local_settings`: Options to override the default values for local HTTP/2
     settings. Values provided here will override the defaults specified in RFC9113§6.5.2
   """
@@ -180,6 +182,7 @@ defmodule Bandit do
           | {:max_header_block_size, pos_integer()}
           | {:max_requests, pos_integer()}
           | {:max_reset_stream_rate, {pos_integer(), pos_integer()} | nil}
+          | {:sendfile_chunk_size, pos_integer()}
           | {:default_local_settings, keyword()}
         ]
 
@@ -241,7 +244,7 @@ defmodule Bandit do
   @top_level_keys ~w(plug scheme port ip keyfile certfile otp_app cipher_suite display_plug startup_log thousand_island_options http_options http_1_options http_2_options websocket_options)a
   @http_keys ~w(compress response_encodings deflate_options zstd_options log_exceptions_with_status_codes log_protocol_errors log_client_closures)a
   @http_1_keys ~w(enabled max_request_line_length max_header_length max_header_count max_requests clear_process_dict gc_every_n_keepalive_requests log_unknown_messages)a
-  @http_2_keys ~w(enabled max_header_block_size max_requests max_reset_stream_rate default_local_settings)a
+  @http_2_keys ~w(enabled max_header_block_size max_requests max_reset_stream_rate sendfile_chunk_size default_local_settings)a
   @websocket_keys ~w(enabled max_frame_size validate_text_frames compress deflate_options primitive_ops_module)a
   @thousand_island_keys ThousandIsland.ServerConfig.__struct__()
                         |> Map.from_struct()

lib/bandit/http2/connection.ex

@@ -227,11 +227,15 @@ defmodule Bandit.HTTP2.Connection do
       :new ->
         new_stream!(connection, stream_id)
 
+        sendfile_chunk_size =
+          Keyword.get(connection.opts.http_2, :sendfile_chunk_size, 1_048_576)
+
         stream =
           Bandit.HTTP2.Stream.init(
             self(),
             stream_id,
-            connection.remote_settings.initial_window_size
+            connection.remote_settings.initial_window_size,
+            sendfile_chunk_size
           )
 
         case Bandit.HTTP2.StreamProcess.start_link(

lib/bandit/http2/stream.ex

@@ -45,6 +45,7 @@ defmodule Bandit.HTTP2.Stream do
             state: :idle,
             recv_window_size: 65_535,
             send_window_size: nil,
+            sendfile_chunk_size: nil,
             bytes_remaining: nil,
             read_timeout: 15_000
 
@@ -64,15 +65,17 @@ defmodule Bandit.HTTP2.Stream do
           state: state(),
           recv_window_size: non_neg_integer(),
           send_window_size: non_neg_integer(),
+          sendfile_chunk_size: pos_integer(),
           bytes_remaining: non_neg_integer() | nil,
           read_timeout: timeout()
         }
 
-  def init(connection_pid, stream_id, initial_send_window_size) do
+  def init(connection_pid, stream_id, initial_send_window_size, sendfile_chunk_size) do
     %__MODULE__{
       connection_pid: connection_pid,
       stream_id: stream_id,
-      send_window_size: initial_send_window_size
+      send_window_size: initial_send_window_size,
+      sendfile_chunk_size: sendfile_chunk_size
     }
   end
 
@@ -479,9 +482,10 @@ defmodule Bandit.HTTP2.Stream do
       case :file.open(path, [:raw, :binary]) do
         {:ok, fd} ->
           try do
-            case :file.pread(fd, offset, length) do
-              {:ok, data} -> send_data(stream, data, true)
-              {:error, reason} -> raise "Error reading file for sendfile: #{inspect(reason)}"
+            if length == 0 do
+              send_data(stream, "", true)
+            else
+              sendfile_loop(stream, fd, offset, length, 0)
             end
           after
             :file.close(fd)
@@ -492,6 +496,37 @@ defmodule Bandit.HTTP2.Stream do
       end
     end
 
+    defp sendfile_loop(stream, _fd, _offset, length, sent) when sent >= length do
+      stream
+    end
+
+    defp sendfile_loop(stream, fd, offset, length, sent) do
+      read_size = min(length - sent, sendfile_chunk_size(stream))
+
+      case :file.pread(fd, offset + sent, read_size) do
+        {:ok, data} ->
+          now_sent = byte_size(data)
+          end_stream = sent + now_sent >= length
+          stream = send_data(stream, data, end_stream)
+
+          if end_stream do
+            stream
+          else
+            sendfile_loop(stream, fd, offset, length, sent + now_sent)
+          end
+
+        :eof ->
+          raise "Error reading file for sendfile: :eof"
+
+        {:error, reason} ->
+          raise "Error reading file for sendfile: #{inspect(reason)}"
+      end
+    end
+
+    defp sendfile_chunk_size(%@for{sendfile_chunk_size: sendfile_chunk_size}) do
+      max(sendfile_chunk_size, 1)
+    end
+
     defp split_data(data, desired_length) do
       data_length = IO.iodata_length(data)
 

test/bandit/http2/protocol_test.exs

@@ -271,6 +271,49 @@ defmodule HTTP2ProtocolTest do
       conn |> send_resp(200, String.duplicate("a", 50_000))
     end
 
+    test "sendfile streams in multiple DATA frames when max frame size allows large frames",
+         context do
+      size = 1_500_000
+
+      tmp_path =
+        Path.join(
+          System.tmp_dir!(),
+          "bandit-sendfile-large-#{System.unique_integer([:positive])}"
+        )
+
+      File.write!(tmp_path, :binary.copy(<<"a">>, size))
+      :persistent_term.put({__MODULE__, :large_sendfile_path}, tmp_path)
+
+      on_exit(fn ->
+        :persistent_term.erase({__MODULE__, :large_sendfile_path})
+        File.rm(tmp_path)
+      end)
+
+      socket = SimpleH2Client.tls_client(context)
+      SimpleH2Client.exchange_prefaces(socket)
+
+      SimpleH2Client.exchange_client_settings(
+        socket,
+        <<4::16, 3_000_000::32, 5::16, 2_000_000::32>>
+      )
+
+      SimpleH2Client.send_window_update(socket, 0, 3_000_000)
+      SimpleH2Client.send_simple_headers(socket, 1, :get, "/sendfile_large_chunked", context.port)
+
+      assert {:ok, 1, false, [{":status", "200"} | _], _ctx} =
+               SimpleH2Client.recv_headers(socket)
+
+      assert {:ok, 1, false, first} = SimpleH2Client.recv_body(socket)
+      assert byte_size(first) > 0
+      assert {:ok, 1, true, second} = SimpleH2Client.recv_body(socket)
+      assert byte_size(second) > 0
+    end
+
+    def sendfile_large_chunked(conn) do
+      path = :persistent_term.get({__MODULE__, :large_sendfile_path})
+      send_file(conn, 200, path, 0, :all)
+    end
+
     test "the server preserves existing settings which are NOT sent by the client", context do
       socket = SimpleH2Client.tls_client(context)
       SimpleH2Client.exchange_prefaces(socket)