From f83883f03e9f8b8016817689afd3acf9bd55fe04 Mon Sep 17 00:00:00 2001
From: Mat Trudel <mat@geeky.net>
Date: Thu, 25 Jan 2024 21:19:58 -0500
Subject: [PATCH] Improve coverage of error cases & logging

---
 lib/bandit/http2/handler.ex         |  2 +
 test/bandit/http2/protocol_test.exs | 63 ++++++++++++++++++++++++++---
 2 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/lib/bandit/http2/handler.ex b/lib/bandit/http2/handler.ex
index 38437490..523ca5f5 100644
--- a/lib/bandit/http2/handler.ex
+++ b/lib/bandit/http2/handler.ex
@@ -75,6 +75,8 @@ defmodule Bandit.HTTP2.Handler do
     )
   end
 
+  def handle_error(_error, _socket, _state), do: :ok
+
   def handle_call({{:send_data, data, end_stream}, stream_id}, from, {socket, state}) do
     # In 'normal' cases where there is sufficient space in the send windows for this message to be
     # sent, Connection will call `unblock` synchronously in the `Connection.send_data` call below.
diff --git a/test/bandit/http2/protocol_test.exs b/test/bandit/http2/protocol_test.exs
index f4d62a22..90c4cbaf 100644
--- a/test/bandit/http2/protocol_test.exs
+++ b/test/bandit/http2/protocol_test.exs
@@ -42,6 +42,16 @@ defmodule HTTP2ProtocolTest do
   end
 
   describe "errors and unexpected frames" do
+    @tag capture_log: true
+    test "it should silently ignore client closes",
+         context do
+      socket = SimpleH2Client.tls_client(context)
+      SimpleH2Client.exchange_prefaces(socket)
+      SimpleH2Client.send_goaway(socket, 0, 0)
+      Transport.close(socket)
+      Process.sleep(100)
+    end
+
     @tag capture_log: true
     test "it should ignore unknown frame types", context do
       socket = SimpleH2Client.setup_connection(context)
@@ -49,14 +59,57 @@ defmodule HTTP2ProtocolTest do
       assert SimpleH2Client.connection_alive?(socket)
     end
 
-    @tag capture_log: true
-    test "it should shut down the connection gracefully when encountering a connection error",
+    test "it should shut down the connection gracefully and log when encountering a connection error",
          context do
       socket = SimpleH2Client.tls_client(context)
       SimpleH2Client.exchange_prefaces(socket)
-      # Send a bogus SETTINGS frame
-      SimpleH2Client.send_frame(socket, 4, 0, 1, <<>>)
-      assert SimpleH2Client.recv_goaway_and_close(socket) == {:ok, 0, 1}
+
+      errors =
+        capture_log(fn ->
+          # Send a bogus SETTINGS frame
+          SimpleH2Client.send_frame(socket, 4, 0, 1, <<>>)
+          assert SimpleH2Client.recv_goaway_and_close(socket) == {:ok, 0, 1}
+          Process.sleep(100)
+        end)
+
+      assert errors =~
+               "(Bandit.HTTP2.Errors.ConnectionError) Invalid SETTINGS frame (RFC9113§6.5)"
+    end
+
+    test "it should shut down the connection gracefully and log when encountering a connection error related to a stream",
+         context do
+      socket = SimpleH2Client.tls_client(context)
+      SimpleH2Client.exchange_prefaces(socket)
+
+      errors =
+        capture_log(fn ->
+          # Send a WINDOW_UPDATE on an idle stream
+          SimpleH2Client.send_window_update(socket, 1, 1234)
+
+          assert SimpleH2Client.recv_goaway_and_close(socket) == {:ok, 1, 1}
+          Process.sleep(100)
+        end)
+
+      assert errors =~
+               "(Bandit.HTTP2.Errors.ConnectionError) Received WINDOW_UPDATE in idle state"
+    end
+
+    test "it should shut down the stream gracefully and log when encountering a stream error",
+         context do
+      socket = SimpleH2Client.tls_client(context)
+      SimpleH2Client.exchange_prefaces(socket)
+
+      errors =
+        capture_log(fn ->
+          # Send trailers with pseudo headers
+          {:ok, ctx} = SimpleH2Client.send_simple_headers(socket, 1, :post, "/echo", context.port)
+          SimpleH2Client.send_headers(socket, 1, true, [{":path", "/foo"}], ctx)
+          assert SimpleH2Client.recv_rst_stream(socket) == {:ok, 1, 1}
+          Process.sleep(100)
+        end)
+
+      assert errors =~
+               "(Bandit.HTTP2.Errors.StreamError) Received trailers with pseudo headers"
     end
 
     @tag capture_log: true