Skip to content

Commit f11fc7b

Browse files
pepicrftmtrudel
andauthored
Skip body draining when Connection: close is set (#546)
* Skip body draining when Connection: close is set When a response includes a Connection: close header (either from the request or response), there's no need to drain the unread request body since the connection will be closed anyway and won't be reused. This change adds a clause to ensure_completed that returns early when keepalive is false, avoiding the body read timeout that would otherwise occur when returning early errors for requests with large bodies. This is particularly useful for scenarios where: - A client sends a request with a large body (e.g., file upload) - The server returns an error early (e.g., 401 Unauthorized) - The server sets Connection: close to signal the connection will end Previously, Bandit would wait for the body read timeout before closing the connection. Now it closes immediately. * Minor formatting change to please the linter --------- Co-authored-by: Mat Trudel <[email protected]>
1 parent 4a24bda commit f11fc7b

File tree

2 files changed

+40
-0
lines changed

2 files changed

+40
-0
lines changed

lib/bandit/http1/socket.ex

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,7 @@ defmodule Bandit.HTTP1.Socket do
429429
end
430430

431431
def ensure_completed(%@for{read_state: :read} = socket), do: socket
432+
def ensure_completed(%@for{keepalive: false} = socket), do: socket
432433

433434
def ensure_completed(%@for{} = socket) do
434435
case read_data(socket, []) do

test/bandit/http1/protocol_test.exs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,45 @@ defmodule HTTP1ProtocolTest do
242242
|> send_resp(200, "OK")
243243
end
244244

245+
test "connection: close skips body draining when plug does not read body", context do
246+
# Use a longer read timeout so we can verify the connection closes quickly
247+
# (i.e., doesn't wait for the timeout to drain the body)
248+
context =
249+
context
250+
|> http_server(thousand_island_options: [read_timeout: 500])
251+
|> Enum.into(context)
252+
253+
client = SimpleHTTP1Client.tcp_client(context)
254+
255+
# Send headers with a large content-length but don't send any body data
256+
Transport.send(
257+
client,
258+
"POST /close_connection_with_unread_body HTTP/1.1\r\nhost: localhost\r\ncontent-length: 10000000\r\n\r\n"
259+
)
260+
261+
# The plug returns immediately with Connection: close without reading the body
262+
# With the fix, this should return quickly without waiting for body read timeout
263+
start_time = System.monotonic_time(:millisecond)
264+
assert {:ok, "200 OK", headers, _body} = SimpleHTTP1Client.recv_reply(client)
265+
266+
assert Enum.any?(headers, fn {k, v} -> k == :connection && String.downcase(v) == "close" end)
267+
268+
elapsed = System.monotonic_time(:millisecond) - start_time
269+
270+
# Should complete well before the 500ms read timeout
271+
assert elapsed < 200, "Expected quick response but took #{elapsed}ms"
272+
273+
# Connection should be closed
274+
assert SimpleHTTP1Client.connection_closed_for_reading?(client)
275+
end
276+
277+
def close_connection_with_unread_body(conn) do
278+
# Return immediately with Connection: close without reading the body
279+
conn
280+
|> put_resp_header("connection", "close")
281+
|> send_resp(200, "OK")
282+
end
283+
245284
test "keepalive mixed-case header connections are respected in HTTP/1.0", context do
246285
client = SimpleHTTP1Client.tcp_client(context)
247286

0 commit comments

Comments
 (0)