Treat an empty Transfer-Encoding list as no transfer codings

lib/mint/http1.ex

@@ -865,7 +865,7 @@ defmodule Mint.HTTP1 do
   # treated as faulty, so the connection is closed after it without processing the
   # final response, and the current request fails along with the queued ones.
   defp decode_body(:informational, %{request: request} = conn, _data, _request_ref, responses)
-       when request.version < {1, 1} and request.transfer_encoding != [] do
+       when request.version < {1, 1} and request.transfer_encoding != nil do
     {conn, responses} = close_after_response(conn, responses, conn.transport.wrap_error(:closed))
     {:ok, conn, responses}
   end
@@ -884,7 +884,7 @@ defmodule Mint.HTTP1 do
         data_buffer: [],
         content_length: nil,
         connection: [],
-        transfer_encoding: [],
+        transfer_encoding: nil,
         body: nil
     }
 
@@ -1143,9 +1143,11 @@ defmodule Mint.HTTP1 do
          do: {:ok, %{request | connection: connection ++ connection_header}}
   end
 
+  # Transfer-Encoding values are kept unparsed, nil meaning there's no such field.
+  # They're only parsed when they decide the framing of the body, so they don't
+  # fail responses that have no body.
   defp store_header(%{transfer_encoding: transfer_encoding} = request, "transfer-encoding", value) do
-    with {:ok, transfer_encoding_header} <- Parse.transfer_encoding_header(value),
-         do: {:ok, %{request | transfer_encoding: transfer_encoding ++ transfer_encoding_header}}
+    {:ok, %{request | transfer_encoding: List.wrap(transfer_encoding) ++ [value]}}
   end
 
   defp store_header(_request, "content-length", _value) do
@@ -1179,7 +1181,7 @@ defmodule Mint.HTTP1 do
 
       # RFC 9112 6.1: the framing of an HTTP/1.0 message with Transfer-Encoding is
       # treated as faulty, so the connection is closed after it even if kept alive.
-      "keep-alive" in request.connection and request.transfer_encoding == [] ->
+      "keep-alive" in request.connection and request.transfer_encoding == nil ->
         {conn, responses}
 
       true ->
@@ -1266,19 +1268,24 @@ defmodule Mint.HTTP1 do
       method == "CONNECT" and status in 200..299 ->
         {:ok, :none}
 
-      request.transfer_encoding != [] && request.content_length ->
+      request.transfer_encoding != nil && request.content_length ->
         {:error, :transfer_encoding_and_content_length}
 
       # RFC9112 6.3:
       # > If a Transfer-Encoding header field is present in a response and the
       # > chunked transfer coding is not the final encoding, the message body
       # > length is determined by reading the connection until it is closed by
       # > the server.
-      "chunked" == List.last(request.transfer_encoding) ->
-        {:ok, {:chunked, nil}}
-
-      request.transfer_encoding != [] ->
-        {:ok, :until_closed}
+      # A Transfer-Encoding field with no codings leaves the response without
+      # framing, so it's read until close as well.
+      request.transfer_encoding != nil ->
+        with {:ok, codings} <- transfer_codings(request.transfer_encoding) do
+          if List.last(codings) == "chunked" do
+            {:ok, {:chunked, nil}}
+          else
+            {:ok, :until_closed}
+          end
+        end
 
       request.content_length ->
         {:ok, {:content_length, request.content_length}}
@@ -1331,7 +1338,7 @@ defmodule Mint.HTTP1 do
       data_buffer: [],
       content_length: nil,
       connection: [],
-      transfer_encoding: [],
+      transfer_encoding: nil,
       body: nil
     }
   end
@@ -1391,13 +1398,18 @@ defmodule Mint.HTTP1 do
   end
 
   # Adds chunked as the final transfer coding, after the codings of the last
-  # Transfer-Encoding field.
+  # Transfer-Encoding field. A last field with no codings is replaced.
   defp append_chunked_coding(headers) do
     headers = Enum.reverse(headers)
     {name, "transfer-encoding", value} = List.keyfind(headers, "transfer-encoding", 1)
 
+    value =
+      if Parse.transfer_encoding_header(value) == {:ok, []},
+        do: "chunked",
+        else: value <> ",chunked"
+
     headers
-    |> Headers.replace(name, "transfer-encoding", value <> ",chunked")
+    |> Headers.replace(name, "transfer-encoding", value)
     |> Enum.reverse()
   end
 

lib/mint/http1/parse.ex

@@ -138,13 +138,11 @@ defmodule Mint.HTTP1.Parse do
   # RFC 9110 10.1.4: transfer-coding = token *( OWS ";" OWS transfer-parameter )
   # and transfer-parameter = token BWS "=" BWS ( token / quoted-string ). Only the
   # coding names are returned. RFC 9112 7.1: chunked defines no parameters and
-  # their presence should be treated as an error.
+  # their presence should be treated as an error. The list may have no elements
+  # (RFC 9110 5.6.1), which adds no transfer coding.
   def transfer_encoding_header(string) do
-    case transfer_coding_list(string, []) do
-      {:ok, []} -> {:error, :empty_token_list}
-      {:ok, codings} -> {:ok, codings}
-      :error -> {:error, {:invalid_token_list, string}}
-    end
+    with :error <- transfer_coding_list(string, []),
+         do: {:error, {:invalid_token_list, string}}
   end
 
   defp transfer_coding_list(<<>>, acc), do: {:ok, :lists.reverse(acc)}

test/mint/http1/conn_test.exs

@@ -1239,11 +1239,133 @@
       "HTTP/1.1 200 OK\r\ntransfer-encoding: chunked; level=1\r\n\r\n5\r\nhello\r\n0\r\n\r\n"
 
     assert {:error, conn, %HTTPError{reason: {:invalid_token_list, "chunked; level=1"}},
-            [{:status, ^ref, 200}]} = HTTP1.stream(conn, {:tcp, conn.socket, response})
+            [{:status, ^ref, 200}, {:headers, ^ref, _}]} =
+             HTTP1.stream(conn, {:tcp, conn.socket, response})
+
+    refute HTTP1.open?(conn)
+  end
+
+  test "a response with only an empty transfer-encoding is read until close", %{conn: conn} do
+    {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil)
+
+    response = "HTTP/1.1 200 OK\r\ntransfer-encoding: , \r\n\r\nhello"
+
+    assert {:ok, conn, [{:status, ^ref, 200}, {:headers, ^ref, _}, {:data, ^ref, "hello"}]} =
+             HTTP1.stream(conn, {:tcp, conn.socket, response})
+
+    assert {:ok, conn, [{:data, ^ref, "more"}]} = HTTP1.stream(conn, {:tcp, conn.socket, "more"})
+    assert {:ok, conn, [{:done, ^ref}]} = HTTP1.stream(conn, {:tcp_closed, conn.socket})
+    refute HTTP1.open?(conn)
+  end
+
+  test "an empty transfer-encoding field adds no transfer coding", %{conn: conn} do
+    {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil)
+
+    response =
+      "HTTP/1.1 200 OK\r\ntransfer-encoding: gzip\r\ntransfer-encoding: ,\r\n" <>
+        "transfer-encoding: chunked\r\n\r\n5\r\nhello\r\n0\r\n\r\n"
+
+    assert {:ok, conn, responses} = HTTP1.stream(conn, {:tcp, conn.socket, response})
+
+    assert [{:status, ^ref, 200}, {:headers, ^ref, _}, {:data, ^ref, "hello"}, {:done, ^ref}] =
+             responses
+
+    assert HTTP1.open?(conn)
+  end
+
+  test "an HTTP/1.0 response with an empty transfer-encoding closes the connection",
+       %{conn: conn} do
+    {:ok, conn, ref1} = HTTP1.request(conn, "HEAD", "/", [], nil)
+    {:ok, conn, ref2} = HTTP1.request(conn, "GET", "/", [], nil)
+
+    response =
+      "HTTP/1.0 200 OK\r\nconnection: keep-alive\r\ntransfer-encoding:\r\n\r\n" <>
+        "HTTP/1.1 200 OK\r\ncontent-length: 2\r\n\r\nok"
+
+    assert {:ok, conn, responses} = HTTP1.stream(conn, {:tcp, conn.socket, response})
+
+    assert [
+             {:status, ^ref1, 200},
+             {:headers, ^ref1, _},
+             {:done, ^ref1},
+             {:error, ^ref2, %TransportError{reason: :closed}}
+           ] = responses
+
+    refute HTTP1.open?(conn)
+  end
+
+  test "an HTTP/1.0 1xx response with an empty transfer-encoding closes the connection",
+       %{conn: conn} do
+    {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil)
+
+    response =
+      "HTTP/1.0 103 Early Hints\r\ntransfer-encoding:\r\n\r\n" <>
+        "HTTP/1.1 200 OK\r\ncontent-length: 2\r\n\r\nok"
+
+    assert {:ok, conn, responses} = HTTP1.stream(conn, {:tcp, conn.socket, response})
+
+    assert [
+             {:status, ^ref, 103},
+             {:headers, ^ref, _},
+             {:error, ^ref, %TransportError{reason: :closed}}
+           ] = responses
 
     refute HTTP1.open?(conn)
   end
 
+  for {method, status} <- [{"CONNECT", 200}, {"HEAD", 200}, {"GET", 204}, {"GET", 304}] do
+    test "an invalid transfer-encoding is ignored in a #{status} response to #{method}",
+         %{conn: conn} do
+      {:ok, conn, ref} = HTTP1.request(conn, unquote(method), "example.com:443", [], nil)
+      response = "HTTP/1.1 #{unquote(status)} Status\r\ntransfer-encoding: chunked;a=1\r\n\r\n"
+
+      assert {:ok, conn, [{:status, ^ref, unquote(status)}, {:headers, ^ref, _}, {:done, ^ref}]} =
+               HTTP1.stream(conn, {:tcp, conn.socket, response})
+
+      assert HTTP1.open?(conn)
+    end
+  end
+
+  for value <- ["", "chunked;a=1"] do
+    test "transfer-encoding #{inspect(value)} with content-length is an error", %{conn: conn} do
+      {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [], nil)
+
+      response =
+        "HTTP/1.1 200 OK\r\ntransfer-encoding: #{unquote(value)}\r\ncontent-length: 2\r\n\r\nok"
+
+      assert {:error, conn, %HTTPError{reason: :transfer_encoding_and_content_length},
+              [{:status, ^ref, 200}, {:headers, ^ref, _}]} =
+               HTTP1.stream(conn, {:tcp, conn.socket, response})
+
+      refute HTTP1.open?(conn)
+    end
+  end
+
+  test "transfer-encoding fields are combined before they're parsed", %{conn: conn} do
+    {:ok, conn, ref1} = HTTP1.request(conn, "GET", "/", [], nil)
+    {:ok, conn, ref2} = HTTP1.request(conn, "GET", "/", [], nil)
+
+    response =
+      "HTTP/1.1 200 OK\r\ntransfer-encoding: custom;p=\"a\r\n" <>
+        "transfer-encoding: b\", chunked\r\n\r\n5\r\nhello\r\n0\r\n\r\n" <>
+        "HTTP/1.1 200 OK\r\ncontent-length: 2\r\n\r\nok"
+
+    assert {:ok, conn, responses} = HTTP1.stream(conn, {:tcp, conn.socket, response})
+
+    assert [
+             {:status, ^ref1, 200},
+             {:headers, ^ref1, _},
+             {:data, ^ref1, "hello"},
+             {:done, ^ref1},
+             {:status, ^ref2, 200},
+             {:headers, ^ref2, _},
+             {:data, ^ref2, "ok"},
+             {:done, ^ref2}
+           ] = responses
+
+    assert HTTP1.open?(conn)
+  end
+
   test "close/1", %{conn: conn} do
     assert HTTP1.open?(conn)
     assert {:ok, conn} = HTTP1.close(conn)
@@ -1900,6 +2022,24 @@
       assert HTTP1.open?(conn)
     end
 
+    test "transfer-encoding is set to chunked if present but empty",
+         %{conn: conn, server_socket: server_socket, port: port} do
+      {:ok, conn, ref} = HTTP1.request(conn, "GET", "/", [{"transfer-encoding", ""}], :stream)
+
+      assert receive_request_string(server_socket) ==
+               request_string("""
+               GET / HTTP/1.1
+               host: localhost:#{port}
+               user-agent: #{mint_user_agent()}
+               transfer-encoding: chunked
+
+               \
+               """)
+
+      {:ok, conn} = HTTP1.stream_request_body(conn, ref, "hello")
+      assert receive_request_string(server_socket) == "5\r\nhello\r\n"
+    end
+
     test "chunked is appended to a transfer coding with parameters",
          %{conn: conn, server_socket: server_socket, port: port} do
       {:ok, conn, ref} =
@@ -1915,7 +2055,7 @@
                \
                """)
 
       {:ok, conn} = HTTP1.stream_request_body(conn, ref, "hello")
       assert receive_request_string(server_socket) == "5\r\nhello\r\n"
     end
 
@@ -1935,7 +2075,7 @@
                \
                """)
 
       {:ok, conn} = HTTP1.stream_request_body(conn, ref, "hello")
       assert receive_request_string(server_socket) == "5\r\nhello\r\n"
     end
 
@@ -1955,7 +2095,7 @@
                \
                """)
 
       {:ok, conn} = HTTP1.stream_request_body(conn, ref, "hello")
       assert receive_request_string(server_socket) == "hello"
     end
 

test/mint/http1/parse_test.exs

@@ -161,7 +161,8 @@ defmodule Mint.HTTP1.ParseTest do
     assert transfer_encoding_header("gzip,   Chunked  ") == {:ok, ["gzip", "chunked"]}
 
     assert transfer_encoding_header("\n") == {:error, {:invalid_token_list, "\n"}}
-    assert transfer_encoding_header("") == {:error, :empty_token_list}
+    assert transfer_encoding_header("") == {:ok, []}
+    assert transfer_encoding_header(" , ,") == {:ok, []}
   end
 
   test "transfer_encoding_header/1 with transfer coding parameters" do