Fix DecompressResponse middleware for multiple encodings and keep updated content-length header (#809)

* fix: stop decompressing response on first unknown codec

If multiple Content-Encodings were applied restoring the original
message requires undoing all steps in reverse order.
Thus we cannot continue when encountering an unsupported codec. Just
skipping over one step will most likely just lead to decoding errors
in the next supported step or incorrect results otherwise.

* fix: keep original content length and encoding headers for HEAD requests

HEAD requests can be used to check the size of remote content
to decide ahead of time whether it is worth fetching. Of course
the size after decompression likely differs from the transfer size
indicated in the content-length header, but depending on use case
only the transfer size might be relevant.

This obsoletes the empty-body special case in decompress_body
previously added in 5bc9b82
since HEAD requests are now handled earlier. If we get an
invalid empty body in a non-HEAD request we want to fail loudly.

* fix: update existing content-length header after decompression

Depending on context presence of this header is mandatory
or at least strongly encouraged in HTTP/1.0 and HTTP/1.1
and some later processing steps might rely on or profit
from its presence

Author: TheOneric

lib/tesla/middleware/compression.ex

@@ -67,9 +67,14 @@ defmodule Tesla.Middleware.Compression do
   def decompress({:ok, env}), do: {:ok, decompress(env)}
   def decompress({:error, reason}), do: {:error, reason}
 
+  # HEAD requests may be used to obtain information on the transfer size and properties
+  # and their empty bodies are not actually valid for the possibly indicated encodings
+  # thus we want to preserve them unchanged.
+  def decompress(%Tesla.Env{method: :head} = env), do: env
+
   def decompress(env) do
     codecs = compression_algorithms(Tesla.get_header(env, "content-encoding"))
-    {decompressed_body, unknown_codecs} = decompress_body(codecs, env.body, [])
+    {decompressed_body, unknown_codecs} = decompress_body(codecs, env.body)
 
     env
     |> put_decompressed_body(decompressed_body)
@@ -84,28 +89,24 @@ defmodule Tesla.Middleware.Compression do
     Tesla.put_header(env, "content-encoding", Enum.join(unknown_codecs, ", "))
   end
 
-  defp decompress_body(_codecs, "" = body, acc) do
-    {body, acc}
-  end
-
-  defp decompress_body([gzip | rest], body, acc) when gzip in ["gzip", "x-gzip"] do
-    decompress_body(rest, :zlib.gunzip(body), acc)
+  defp decompress_body([gzip | rest], body) when gzip in ["gzip", "x-gzip"] do
+    decompress_body(rest, :zlib.gunzip(body))
   end
 
-  defp decompress_body(["deflate" | rest], body, acc) do
-    decompress_body(rest, :zlib.unzip(body), acc)
+  defp decompress_body(["deflate" | rest], body) do
+    decompress_body(rest, :zlib.unzip(body))
   end
 
-  defp decompress_body(["identity" | rest], body, acc) do
-    decompress_body(rest, body, acc)
+  defp decompress_body(["identity" | rest], body) do
+    decompress_body(rest, body)
   end
 
-  defp decompress_body([codec | rest], body, acc) do
-    decompress_body(rest, body, [codec | acc])
+  defp decompress_body([codec | rest], body) do
+    {body, Enum.reverse([codec | rest])}
   end
 
-  defp decompress_body([], body, acc) do
-    {body, acc}
+  defp decompress_body([], body) do
+    {body, []}
   end
 
   defp compression_algorithms(nil) do
@@ -123,7 +124,27 @@ defmodule Tesla.Middleware.Compression do
   defp put_decompressed_body(env, body) do
     env
     |> Tesla.put_body(body)
-    |> Tesla.delete_header("content-length")
+    |> update_content_length(body)
+  end
+
+  # The value of the content-length header wil be inaccurate after decompression.
+  # But setting it is mandatory or strongly encouraged in HTTP/1.0 and HTTP/1.1.
+  # Except, when transfer-encoding is used defining content-length is invalid.
+  # Thus we can neither just drop it nor indiscriminately add it, but will update it if it already exist.
+  # Furthermore, content-length is technically allowed to be specified mutliple times if all values match,
+  # to ensure consistency we must therefore make sure to drop any duplicate definitions while updating.
+  defp update_content_length(env, body) when is_binary(body) do
+    if Tesla.get_header(env, "content-length") != nil do
+      env
+      |> Tesla.delete_header("content-length")
+      |> Tesla.put_header("content-length", "#{byte_size(body)}")
+    else
+      env
+    end
+  end
+
+  defp update_content_length(env, _) do
+    env
   end
 end
 

test/tesla/middleware/compression_test.exs

@@ -59,11 +59,33 @@ defmodule Tesla.Middleware.CompressionTest do
             {200, [{"content-type", "text/plain"}, {"content-encoding", "deflate"}],
              :zlib.zip("decompressed deflate")}
 
+          "/multiple-encodings" ->
+            {200, [{"content-type", "text/plain"}, {"content-encoding", "gzip, zstd, gzip"}],
+             :zlib.gzip("decompressed gzip")}
+
           "/response-identity" ->
             {200, [{"content-type", "text/plain"}, {"content-encoding", "identity"}], "unchanged"}
 
           "/response-empty" ->
             {200, [{"content-type", "text/plain"}, {"content-encoding", "gzip"}], ""}
+
+          "/response-with-content-length" ->
+            body = :zlib.gzip("decompressed gzip")
+
+            {200,
+             [
+               {"content-type", "text/plain"},
+               {"content-encoding", "gzip"},
+               {"content-length", "#{byte_size(body)}"}
+             ], body}
+
+          "/response-empty-with-content-length" ->
+            {200,
+             [
+               {"content-type", "text/plain"},
+               {"content-encoding", "gzip"},
+               {"content-length", "4194304"}
+             ], ""}
         end
 
       {:ok, %{env | status: status, headers: headers, body: body}}
@@ -81,16 +103,44 @@ defmodule Tesla.Middleware.CompressionTest do
     assert env.body == "decompressed deflate"
   end
 
+  test "stops decompressing on first unsupported content-encoding" do
+    assert {:ok, env} = CompressionResponseClient.get("/multiple-encodings")
+    assert env.body == "decompressed gzip"
+    assert env.headers == [{"content-type", "text/plain"}, {"content-encoding", "gzip, zstd"}]
+  end
+
   test "return unchanged response for unsupported content-encoding" do
     assert {:ok, env} = CompressionResponseClient.get("/response-identity")
     assert env.body == "unchanged"
     assert env.headers == [{"content-type", "text/plain"}]
   end
 
-  test "return unchanged response for empty body (gzip)" do
-    assert {:ok, env} = CompressionResponseClient.get("/response-empty")
+  test "raises on invalid empty-body response (gzip)" do
+    assert_raise(ErlangError, "Erlang error: :data_error", fn ->
+      CompressionResponseClient.get("/response-empty")
+    end)
+  end
+
+  test "updates existing content-length header" do
+    expected_body = "decompressed gzip"
+    assert {:ok, env} = CompressionResponseClient.get("/response-with-content-length")
+    assert env.body == expected_body
+
+    assert env.headers == [
+             {"content-type", "text/plain"},
+             {"content-length", "#{byte_size(expected_body)}"}
+           ]
+  end
+
+  test "preserves compression headers for HEAD requests" do
+    assert {:ok, env} = CompressionResponseClient.head("/response-empty-with-content-length")
     assert env.body == ""
-    assert env.headers == [{"content-type", "text/plain"}]
+
+    assert env.headers == [
+             {"content-type", "text/plain"},
+             {"content-encoding", "gzip"},
+             {"content-length", "4194304"}
+           ]
   end
 
   defmodule CompressRequestDecompressResponseClient do