Skip to content

Commit b240749

Browse files
authored
RFC Conformance: Fix HTTP/1.0 streaming response framing
I had Claude help validate the code against various RFC spec for conformance, and it flagged below. Claude helped with the desc. ## Nonconformance `Bandit.HTTP1.Socket.send_headers/4` added `Transfer-Encoding: chunked` for every streamed response without a `Content-Length`, including responses to HTTP/1.0 requests. [RFC 9112 Section 6.1](https://www.rfc-editor.org/rfc/rfc9112.html#section-6.1) says that a server **MUST NOT** send a response containing `Transfer-Encoding` unless the corresponding request indicates HTTP/1.1 or later. [RFC 9112 Section 6.3, rule 8](https://www.rfc-editor.org/rfc/rfc9112.html#section-6.3-8) defines a response without `Transfer-Encoding` or `Content-Length` as close-delimited. ## Change For an HTTP/1.0 streamed response whose length is not known in advance, Bandit now: - sends the body bytes without HTTP/1.1 chunk framing; - omits `Transfer-Encoding`, including a field supplied by the application; - sends `Connection: close`; and - disables connection reuse so that connection closure delimits the body. HTTP/1.1 streaming and content-length-delimited streaming are unchanged. ## Regression test The new protocol test sends an HTTP/1.0 request with `Connection: keep-alive` to a chunked Plug response. It reads through connection closure and verifies that: - the decoded body is complete; - no `Transfer-Encoding` field is present; and - `Connection: close` is present. A companion test verifies that Bandit removes an application-supplied `Transfer-Encoding` field from an HTTP/1.0 streamed response. Run: ```console mix test test/bandit/http1/protocol_test.exs ``` Verified against Bandit `main` at `b084b37` with Erlang/OTP 27.3 and Elixir 1.18.4: - `mix format --check-formatted` - `MIX_ENV=test mix compile --warnings-as-errors` - Full non-slow suite: 816 tests pass (the suite's one IPv6 server binding test needs an IPv6-capable host) - `mix credo --strict` and `mix dialyzer` pass on the combined tree of all nineteen prepared Bandit changes
1 parent b084b37 commit b240749

2 files changed

Lines changed: 69 additions & 0 deletions

File tree

lib/bandit/http1/socket.ex

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,7 @@ defmodule Bandit.HTTP1.Socket do
455455
resp_line = "#{socket.version} #{status} #{Plug.Conn.Status.reason_phrase(status)}\r\n"
456456

457457
{headers, socket} = handle_keepalive(status, headers, socket)
458+
headers = remove_http_1_0_transfer_encoding(headers, socket)
458459

459460
has_content_length = Bandit.Headers.get_header(headers, "content-length") != nil
460461

@@ -465,6 +466,15 @@ defmodule Bandit.HTTP1.Socket do
465466
# call. This makes a _substantial_ difference in practice
466467
%{socket | write_state: :writing, send_buffer: [resp_line | encode_headers(headers)]}
467468

469+
:chunk_encoded when not has_content_length and socket.version == :"HTTP/1.0" ->
470+
# HTTP/1.0 has no chunked transfer coding. Delimit the response body by closing the
471+
# connection, even if the client requested a persistent connection.
472+
headers =
473+
[{"connection", "close"} | Enum.reject(headers, &(elem(&1, 0) == "connection"))]
474+
475+
send!(socket.socket, [resp_line | encode_headers(headers)])
476+
%{socket | write_state: :chunk_streaming, keepalive: false}
477+
468478
:chunk_encoded when not has_content_length ->
469479
headers = [{"transfer-encoding", "chunked"} | headers]
470480
send!(socket.socket, [resp_line | encode_headers(headers)])
@@ -484,6 +494,14 @@ defmodule Bandit.HTTP1.Socket do
484494
end
485495
end
486496

497+
# RFC9112§6.1 prohibits Transfer-Encoding in every response to an HTTP/1.0 request,
498+
# including a field supplied explicitly by the application.
499+
defp remove_http_1_0_transfer_encoding(headers, %@for{version: :"HTTP/1.0"}) do
500+
Enum.reject(headers, &(elem(&1, 0) == "transfer-encoding"))
501+
end
502+
503+
defp remove_http_1_0_transfer_encoding(headers, _socket), do: headers
504+
487505
defp handle_keepalive(status, headers, socket) do
488506
response_connection_header = safe_downcase(Bandit.Headers.get_header(headers, "connection"))
489507

test/bandit/http1/protocol_test.exs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,6 +1551,57 @@ defmodule HTTP1ProtocolTest do
15511551
conn
15521552
end
15531553

1554+
test "uses close-delimited streaming for an HTTP/1.0 response", context do
1555+
client = SimpleHTTP1Client.tcp_client(context)
1556+
1557+
SimpleHTTP1Client.send(
1558+
client,
1559+
"GET",
1560+
"/send_chunked_200",
1561+
["host: localhost", "connection: keep-alive"],
1562+
"1.0"
1563+
)
1564+
1565+
response = recv_until_closed(client)
1566+
assert {:ok, "200 OK", headers, "OK"} = SimpleHTTP1Client.parse_response(client, response)
1567+
1568+
refute Bandit.Headers.get_header(headers, :"transfer-encoding")
1569+
assert Bandit.Headers.get_header(headers, :connection) == "close"
1570+
end
1571+
1572+
defp recv_until_closed(client, response \\ "") do
1573+
case Transport.recv(client, 0) do
1574+
{:ok, data} -> recv_until_closed(client, response <> data)
1575+
{:error, :closed} -> response
1576+
end
1577+
end
1578+
1579+
test "removes an application-supplied transfer-encoding from HTTP/1.0", context do
1580+
client = SimpleHTTP1Client.tcp_client(context)
1581+
1582+
SimpleHTTP1Client.send(
1583+
client,
1584+
"GET",
1585+
"/send_chunked_200_with_transfer_encoding",
1586+
["host: localhost"],
1587+
"1.0"
1588+
)
1589+
1590+
response = recv_until_closed(client)
1591+
assert {:ok, "200 OK", headers, "OK"} = SimpleHTTP1Client.parse_response(client, response)
1592+
refute Bandit.Headers.get_header(headers, :"transfer-encoding")
1593+
end
1594+
1595+
def send_chunked_200_with_transfer_encoding(conn) do
1596+
{:ok, conn} =
1597+
conn
1598+
|> put_resp_header("transfer-encoding", "chunked")
1599+
|> send_chunked(200)
1600+
|> chunk("OK")
1601+
1602+
conn
1603+
end
1604+
15541605
test "streams a content-length delimited response if content-length is set before chunking",
15551606
context do
15561607
response = Req.get!(context.req, url: "/send_chunked_200_with_content_length")

0 commit comments

Comments
 (0)