Raise error on send errors for HTTP/1 responses

lib/bandit/adapter.ex

@@ -195,7 +195,14 @@ defmodule Bandit.Adapter do
     # this entire section of the API is a bit slanty regardless.
 
     validate_calling_process!(adapter)
-    {:ok, nil, send_data(adapter, chunk, IO.iodata_length(chunk) == 0)}
+
+    # chunk/2 is unique among Plug.Conn.Adapter's sending callbacks in that it can return an error
+    # tuple instead of just raising or dying on error. Rescue here to implement this
+    try do
+      {:ok, nil, send_data(adapter, chunk, IO.iodata_length(chunk) == 0)}
+    rescue
+      error -> {:error, Exception.format_banner(:error, error)}
+    end
   end
 
   @impl Plug.Conn.Adapter

lib/bandit/http1/socket.ex

@@ -332,15 +332,15 @@ defmodule Bandit.HTTP1.Socket do
 
         :chunk_encoded ->
           headers = [{"transfer-encoding", "chunked"} | headers]
-          _ = ThousandIsland.Socket.send(socket.socket, [resp_line | encode_headers(headers)])
+          send!(socket.socket, [resp_line | encode_headers(headers)])
           %{socket | write_state: :chunking}
 
         :no_body ->
-          _ = ThousandIsland.Socket.send(socket.socket, [resp_line | encode_headers(headers)])
+          send!(socket.socket, [resp_line | encode_headers(headers)])
           %{socket | write_state: :sent}
 
         :inform ->
-          _ = ThousandIsland.Socket.send(socket.socket, [resp_line | encode_headers(headers)])
+          send!(socket.socket, [resp_line | encode_headers(headers)])
           %{socket | write_state: :unsent}
       end
     end
@@ -352,23 +352,38 @@ defmodule Bandit.HTTP1.Socket do
     end
 
     def send_data(%@for{write_state: :writing} = socket, data, end_request) do
-      _ = ThousandIsland.Socket.send(socket.socket, [socket.send_buffer | data])
+      send!(socket.socket, [socket.send_buffer | data])
       write_state = if end_request, do: :sent, else: :writing
       %{socket | write_state: write_state, send_buffer: []}
     end
 
     def send_data(%@for{write_state: :chunking} = socket, data, end_request) do
       byte_size = data |> IO.iodata_length()
-      payload = [Integer.to_string(byte_size, 16), "\r\n", data, "\r\n"]
-      _ = ThousandIsland.Socket.send(socket.socket, payload)
+      send!(socket.socket, [Integer.to_string(byte_size, 16), "\r\n", data, "\r\n"])
       write_state = if end_request, do: :sent, else: :chunking
       %{socket | write_state: write_state}
     end
 
     def sendfile(%@for{write_state: :writing} = socket, path, offset, length) do
-      _ = ThousandIsland.Socket.send(socket.socket, socket.send_buffer)
-      _ = ThousandIsland.Socket.sendfile(socket.socket, path, offset, length)
-      %{socket | write_state: :sent}
+      send!(socket.socket, socket.send_buffer)
+
+      case ThousandIsland.Socket.sendfile(socket.socket, path, offset, length) do
+        {:ok, _bytes_written} -> %{socket | write_state: :sent}
+        {:error, reason} -> request_error!(reason)
+      end
+    end
+
+    @spec send!(ThousandIsland.Socket.t(), iolist()) :: :ok | no_return()
+    defp send!(socket, payload) do
+      case ThousandIsland.Socket.send(socket, payload) do
+        :ok ->
+          :ok
+
+        {:error, reason} ->
+          # Prevent error handlers from possibly trying to send again
+          send(self(), {:plug_conn, :sent})
+          request_error!(reason)
+      end
     end
 
     def ensure_completed(%@for{read_state: :read} = socket), do: socket
@@ -388,7 +403,12 @@ defmodule Bandit.HTTP1.Socket do
       after
         0 ->
           status = error |> Plug.Exception.status() |> Plug.Conn.Status.code()
-          send_headers(socket, status, [], :no_body)
+
+          try do
+            send_headers(socket, status, [], :no_body)
+          rescue
+            Bandit.HTTPError -> :ok
+          end
       end
     end
 

test/bandit/http1/request_test.exs

@@ -7,6 +7,8 @@ defmodule HTTP1RequestTest do
 
   import ExUnit.CaptureLog
 
+  require Logger
+
   setup :http_server
   setup :req_http1_client
 
@@ -2089,4 +2091,117 @@ defmodule HTTP1RequestTest do
       assert output =~ "(RuntimeError) boom"
     end
   end
+
+  describe "connection closure / error handling" do
+    test "raises an error if client closes while headers are being read", context do
+      client = SimpleHTTP1Client.tcp_client(context)
+      Transport.send(client, "GET / HTTP/1.1\r\nHost:")
+      Transport.close(client)
+      Process.sleep(100)
+      assert ThousandIsland.connection_pids(context.server_pid) == {:ok, []}
+    end
+
+    test "raises an error if client closes while body is being read", context do
+      client = SimpleHTTP1Client.tcp_client(context)
+
+      SimpleHTTP1Client.send(client, "POST", "/expect_incomplete_body", [
+        "host: localhost",
+        "content-length: 6"
+      ])
+
+      Transport.send(client, "ABC")
+
+      output =
+        capture_log(fn ->
+          Transport.close(client)
+          Process.sleep(500)
+        end)
+
+      assert output =~ "(Bandit.HTTPError) closed"
+      refute output =~ "IMPOSSIBLE"
+      assert ThousandIsland.connection_pids(context.server_pid) == {:ok, []}
+    end
+
+    def expect_incomplete_body(conn) do
+      {:ok, _body, _conn} = Plug.Conn.read_body(conn)
+      Logger.error("IMPOSSIBLE")
+    end
+
+    test "raises an error if client closes while body is being written", context do
+      client = SimpleHTTP1Client.tcp_client(context)
+
+      SimpleHTTP1Client.send(client, "GET", "/sleep_and_send", ["host: localhost"])
+      Process.sleep(100)
+      Transport.close(client)
+
+      output = capture_log(fn -> Process.sleep(500) end)
+      assert output =~ "(Bandit.HTTPError) closed"
+      refute output =~ "IMPOSSIBLE"
+      assert ThousandIsland.connection_pids(context.server_pid) == {:ok, []}
+    end
+
+    def sleep_and_send(conn) do
+      Process.sleep(200)
+
+      conn = send_resp(conn, 200, "IMPOSSIBLE")
+
+      Logger.error("IMPOSSIBLE")
+      conn
+    end
+
+    test "returns an error if client closes while chunked body is being written", context do
+      client = SimpleHTTP1Client.tcp_client(context)
+
+      SimpleHTTP1Client.send(client, "GET", "/sleep_and_send_chunked", ["host: localhost"])
+      Process.sleep(100)
+      Transport.close(client)
+
+      output = capture_log(fn -> Process.sleep(500) end)
+      assert output == ""
+      assert ThousandIsland.connection_pids(context.server_pid) == {:ok, []}
+    end
+
+    def sleep_and_send_chunked(conn) do
+      conn = send_chunked(conn, 200)
+
+      Process.sleep(200)
+      assert chunk(conn, "IMPOSSIBLE") == {:error, "** (Bandit.HTTPError) closed"}
+
+      conn
+    end
+
+    test "raises an error if client closes before sendfile body is being written", context do
+      client = SimpleHTTP1Client.tcp_client(context)
+
+      SimpleHTTP1Client.send(client, "GET", "/sleep_and_sendfile", ["host: localhost"])
+      Process.sleep(100)
+      Transport.close(client)
+
+      output = capture_log(fn -> Process.sleep(500) end)
+      assert output =~ "(Bandit.HTTPError) closed"
+      refute output =~ "IMPOSSIBLE"
+      assert ThousandIsland.connection_pids(context.server_pid) == {:ok, []}
+    end
+
+    def sleep_and_sendfile(conn) do
+      Process.sleep(200)
+
+      conn = send_file(conn, 204, Path.join([__DIR__, "../../support/sendfile"]), 0, :all)
+
+      Logger.error("IMPOSSIBLE")
+      conn
+    end
+
+    test "silently exits if client closes during keepalive", context do
+      client = SimpleHTTP1Client.tcp_client(context)
+
+      SimpleHTTP1Client.send(client, "GET", "/hello_world", ["host: localhost"])
+      Process.sleep(100)
+      SimpleHTTP1Client.recv_reply(client)
+      Transport.close(client)
+      Process.sleep(500)
+
+      assert ThousandIsland.connection_pids(context.server_pid) == {:ok, []}
+    end
+  end
 end

test/support/transport.ex

@@ -3,7 +3,12 @@ defmodule Transport do
 
   def tcp_client(context) do
     {:ok, socket} =
-      :gen_tcp.connect(~c"localhost", context[:port], active: false, mode: :binary, nodelay: true)
+      :gen_tcp.connect(~c"localhost", context[:port],
+        active: false,
+        linger: {true, 0},
+        mode: :binary,
+        nodelay: true
+      )
 
     {:client, %{socket: socket, transport: :gen_tcp}}
   end