Move log_protocol_errors to a shared option

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ### Changes
 
+* **BREAKING CHANGE** Move `log_protocol_errors` configuration option into
+ shared `http_options` top-level config (and apply it to HTTP/2 errors as well)
 * **BREAKING CHANGE** Remove `origin_telemetry_span_context` from WebSocket
  telemetry events
 * **BREAKING CHANGE** Remove `stream_id` from HTTP/2 telemetry events

lib/bandit.ex

@@ -93,10 +93,13 @@ defmodule Bandit do
  [RFC9110§8.4](https://www.rfc-editor.org/rfc/rfc9110.html#section-8.4). Defaults to true
  * `deflate_options`: A keyword list of options to set on the deflate library. A complete list can
  be found at `t:deflate_options/0`
+ * `log_protocol_errors`: Whether or not to log protocol errors such as malformed requests.
+ Defaults to `true`
  """
  @type http_options :: [
  compress: boolean(),
- deflate_opions: deflate_options()
+ deflate_opions: deflate_options(),
+ log_protocol_errors: boolean()
  ]
 
  @typedoc """
@@ -116,8 +119,6 @@ defmodule Bandit do
  every 5 requests). This option is currently experimental, and may change at any time
  * `log_unknown_messages`: Whether or not to log unknown messages sent to the handler process.
  Defaults to `false`
- * `log_protocol_errors`: Whether or not to log protocol errors such as malformed requests.
- Defaults to `true`
  """
  @type http_1_options :: [
  enabled: boolean(),
@@ -126,8 +127,7 @@ defmodule Bandit do
  max_header_count: pos_integer(),
  max_requests: pos_integer(),
  gc_every_n_keepalive_requests: pos_integer(),
- log_unknown_messages: boolean(),
- log_protocol_errors: boolean()
+ log_unknown_messages: boolean()
  ]
 
  @typedoc """
@@ -196,8 +196,8 @@ defmodule Bandit do
  end
 
  @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 deflate_options)a
- @http_1_keys ~w(enabled max_request_line_length max_header_length max_header_count max_requests gc_every_n_keepalive_requests log_unknown_messages log_protocol_errors)a
+ @http_keys ~w(compress deflate_options log_protocol_errors)a
+ @http_1_keys ~w(enabled max_request_line_length max_header_length max_header_count max_requests gc_every_n_keepalive_requests log_unknown_messages)a
  @http_2_keys ~w(enabled max_header_block_size max_requests default_local_settings)a
  @websocket_keys ~w(enabled max_frame_size validate_text_frames compress)a
  @thousand_island_keys ThousandIsland.ServerConfig.__struct__()

lib/bandit/pipeline.ex

@@ -53,7 +53,7 @@ defmodule Bandit.Pipeline do
  ] ->
  Bandit.Telemetry.stop_span(span, %{}, %{error: error.message})
 
- if Keyword.get(opts.http_1, :log_protocol_errors, true),
+ if Keyword.get(opts.http, :log_protocol_errors, true),
  do: Logger.error(Exception.format(:error, error, __STACKTRACE__))
 
  Bandit.HTTPTransport.send_on_error(transport, error)
@@ -73,7 +73,7 @@ defmodule Bandit.Pipeline do
  span = Bandit.Telemetry.start_span(:request, measurements, metadata)
  Bandit.Telemetry.stop_span(span, %{}, %{error: error.message})
 
- if Keyword.get(opts.http_1, :log_protocol_errors, true),
+ if Keyword.get(opts.http, :log_protocol_errors, true),
  do: Logger.error(Exception.format(:error, error, __STACKTRACE__))
 
  Bandit.HTTPTransport.send_on_error(transport, error)

test/bandit/http1/request_test.exs

@@ -35,7 +35,7 @@ defmodule HTTP1RequestTest do
 
  describe "suppressing protocol error logging" do
  test "errors are not logged if so configured", context do
- context = http_server(context, http_1_options: [log_protocol_errors: false])
+ context = http_server(context, http_options: [log_protocol_errors: false])
 
  output =
  capture_log(fn ->

test/bandit/http2/protocol_test.exs

@@ -112,6 +112,27 @@ defmodule HTTP2ProtocolTest do
  "(Bandit.HTTP2.Errors.StreamError) Received trailers with pseudo headers"
  end
 
+ test "stream errors are not logged if so configured", context do
+ context =
+ context
+ |> https_server(http_options: [log_protocol_errors: false])
+ |> Enum.into(context)
+
+ socket = SimpleH2Client.tls_client(context)
+ SimpleH2Client.exchange_prefaces(socket)
+
+ output =
+ 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 output == ""
+ end
+
  @tag capture_log: true
  test "it should shut down the connection after read timeout has been reached with no initial data sent",
  context do