Introduce new HTTP/1 :skip_target_validation option (#456)

Closes #453.

Co-authored-by: Andrea Leopardi <[email protected]>

Author: PragTob

lib/mint/http1.ex

@@ -93,6 +93,7 @@ defmodule Mint.HTTP1 do
     :mode,
     :scheme_as_string,
     :case_sensitive_headers,
+    :skip_target_validation,
     requests: :queue.new(),
     state: :closed,
     buffer: "",
@@ -123,6 +124,10 @@ defmodule Mint.HTTP1 do
     * `:case_sensitive_headers` - (boolean) if set to `true` the case of the supplied
        headers in requests will be preserved. The default is to lowercase the headers
        because HTTP/1.1 header names are case-insensitive. *Available since v1.6.0*.
+    * `:skip_target_validation` - (boolean) if set to `true` the target of a request
+       will not be validated. You might want this if you deal with non standard-
+       conforming URIs but need to preserve them. The default is to validate the request
+       target. *Available since v1.7.0*.
 
   """
   @spec connect(Types.scheme(), Types.address(), :inet.port_number(), keyword()) ::
@@ -200,7 +205,8 @@ defmodule Mint.HTTP1 do
         scheme_as_string: Atom.to_string(scheme),
         state: :open,
         log: log?,
-        case_sensitive_headers: Keyword.get(opts, :case_sensitive_headers, false)
+        case_sensitive_headers: Keyword.get(opts, :case_sensitive_headers, false),
+        skip_target_validation: Keyword.get(opts, :skip_target_validation, false)
       }
 
       {:ok, conn}
@@ -275,6 +281,7 @@ defmodule Mint.HTTP1 do
       |> add_default_headers(conn)
 
     with {:ok, headers, encoding} <- add_content_length_or_transfer_encoding(headers, body),
+         :ok <- validate_request_target(path, conn.skip_target_validation),
          {:ok, iodata} <-
            Request.encode(
              method,
@@ -964,6 +971,32 @@ defmodule Mint.HTTP1 do
     {:ok, body}
   end
 
+  defp validate_request_target(target, skip_validation?)
+  defp validate_request_target(target, false), do: validate_target(target)
+  defp validate_request_target(_, true), do: :ok
+
+  # Percent-encoding is not case sensitive so we have to account for lowercase and uppercase.
+  @hex_characters ~c"0123456789abcdefABCDEF"
+
+  defp validate_target(target), do: validate_target(target, target)
+
+  defp validate_target(<<?%, char1, char2, rest::binary>>, original_target)
+       when char1 in @hex_characters and char2 in @hex_characters do
+    validate_target(rest, original_target)
+  end
+
+  defp validate_target(<<char, rest::binary>>, original_target) do
+    if URI.char_unescaped?(char) do
+      validate_target(rest, original_target)
+    else
+      {:error, {:invalid_request_target, original_target}}
+    end
+  end
+
+  defp validate_target(<<>>, _original_target) do
+    :ok
+  end
+
   defp new_request(ref, method, body, encoding) do
     state =
       if body == :stream do

lib/mint/http1/request.ex

@@ -17,7 +17,6 @@ defmodule Mint.HTTP1.Request do
   end
 
   defp encode_request_line(method, target) do
-    validate_target!(target)
     [method, ?\s, target, " HTTP/1.1\r\n"]
   end
 
@@ -46,28 +45,6 @@ defmodule Mint.HTTP1.Request do
     [Integer.to_string(length, 16), "\r\n", chunk, "\r\n"]
   end
 
-  # Percent-encoding is not case sensitive so we have to account for lowercase and uppercase.
-  @hex_characters ~c"0123456789abcdefABCDEF"
-
-  defp validate_target!(target), do: validate_target!(target, target)
-
-  defp validate_target!(<<?%, char1, char2, rest::binary>>, original_target)
-       when char1 in @hex_characters and char2 in @hex_characters do
-    validate_target!(rest, original_target)
-  end
-
-  defp validate_target!(<<char, rest::binary>>, original_target) do
-    if URI.char_unescaped?(char) do
-      validate_target!(rest, original_target)
-    else
-      throw({:mint, {:invalid_request_target, original_target}})
-    end
-  end
-
-  defp validate_target!(<<>>, _original_target) do
-    :ok
-  end
-
   defp validate_header_name!(name) do
     _ =
       for <<char <- name>> do

test/mint/http1/conn_test.exs

@@ -548,7 +548,7 @@ defmodule Mint.HTTP1Test do
              request_string("""
              GET / HTTP/1.1
              host: localhost:#{port}
-             user-agent: mint/#{Mix.Project.config()[:version]}
+             user-agent: #{mint_user_agent()}
 
              \
              """)
@@ -570,7 +570,7 @@ defmodule Mint.HTTP1Test do
                request_string("""
                GET / HTTP/1.1
                host: localhost
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
 
                \
                """)
@@ -591,7 +591,7 @@ defmodule Mint.HTTP1Test do
                GET / HTTP/1.1
                content-length: 4
                host: localhost:#{port}
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
 
                body\
                """)
@@ -607,7 +607,7 @@ defmodule Mint.HTTP1Test do
                request_string("""
                GET / HTTP/1.1
                host: localhost:#{port}
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
 
                \
                """)
@@ -626,7 +626,7 @@ defmodule Mint.HTTP1Test do
                request_string("""
                GET / HTTP/1.1
                host: localhost:#{port}
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
                content-length: 10
 
                body\
@@ -760,6 +760,42 @@ defmodule Mint.HTTP1Test do
 
                """)
     end
+
+    @invalid_request_targets ["/ /", "/%foo", "/foo%x"]
+    test "targets are validated by default", %{port: port, server_ref: server_ref} do
+      assert {:ok, conn} = HTTP1.connect(:http, "localhost", port)
+
+      assert_receive {^server_ref, _server_socket}
+
+      for invalid_target <- @invalid_request_targets do
+        assert {:error, %Mint.HTTP1{},
+                %Mint.HTTPError{reason: {:invalid_request_target, ^invalid_target}}} =
+                 HTTP1.request(conn, "GET", invalid_target, [], "")
+      end
+    end
+
+    test "target validation may be skipped based on connection options", %{
+      port: port,
+      server_ref: server_ref
+    } do
+      assert {:ok, conn} = HTTP1.connect(:http, "localhost", port, skip_target_validation: true)
+
+      assert_receive {^server_ref, server_socket}
+
+      for invalid_target <- @invalid_request_targets do
+        assert {:ok, _conn, _ref} = HTTP1.request(conn, "GET", invalid_target, [], "body")
+
+        assert receive_request_string(server_socket) ==
+                 request_string("""
+                 GET #{invalid_target} HTTP/1.1
+                 content-length: 4
+                 host: localhost:#{port}
+                 user-agent: #{mint_user_agent()}
+
+                 body\
+                 """)
+      end
+    end
   end
 
   describe "streaming requests" do
@@ -772,7 +808,7 @@ defmodule Mint.HTTP1Test do
                GET / HTTP/1.1
                transfer-encoding: chunked
                host: localhost:#{port}
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
 
                \
                """)
@@ -795,7 +831,7 @@ defmodule Mint.HTTP1Test do
                request_string("""
                GET / HTTP/1.1
                host: localhost:#{port}
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
                transfer-encoding: chunked
 
                \
@@ -815,7 +851,7 @@ defmodule Mint.HTTP1Test do
                request_string("""
                GET / HTTP/1.1
                host: localhost:#{port}
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
                transfer-encoding: gzip,chunked
 
                \
@@ -839,7 +875,7 @@ defmodule Mint.HTTP1Test do
                request_string("""
                GET / HTTP/1.1
                host: localhost:#{port}
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
                transfer-encoding: identity
 
                \
@@ -859,7 +895,7 @@ defmodule Mint.HTTP1Test do
                request_string("""
                GET / HTTP/1.1
                host: localhost:#{port}
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
                content-length: 5
 
                \
@@ -877,7 +913,7 @@ defmodule Mint.HTTP1Test do
                GET / HTTP/1.1
                transfer-encoding: chunked
                host: localhost:#{port}
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
 
                \
                """)
@@ -897,7 +933,7 @@ defmodule Mint.HTTP1Test do
                POST / HTTP/1.1
                transfer-encoding: chunked
                host: localhost:#{port}
-               user-agent: mint/#{Mix.Project.config()[:version]}
+               user-agent: #{mint_user_agent()}
 
                \
                """)
@@ -997,4 +1033,7 @@ defmodule Mint.HTTP1Test do
   defp stream_message_bytewise(<<>>, conn, responses) do
     {:ok, conn, responses}
   end
+
+  @mint_user_agent "mint/#{Mix.Project.config()[:version]}"
+  defp mint_user_agent, do: @mint_user_agent
 end

test/mint/http1/request_test.exs

@@ -33,16 +33,6 @@ defmodule Mint.HTTP1.RequestTest do
                """)
     end
 
-    test "validates request target" do
-      for invalid_target <- ["/ /", "/%foo", "/foo%x"] do
-        assert Request.encode("GET", invalid_target, [], nil) ==
-                 {:error, {:invalid_request_target, invalid_target}}
-      end
-
-      request = encode_request("GET", "/foo%20bar", [], nil)
-      assert String.starts_with?(request, request_string("GET /foo%20bar HTTP/1.1"))
-    end
-
     test "invalid header name" do
       assert Request.encode("GET", "/", [{"f oo", "bar"}], nil) ==
                {:error, {:invalid_header_name, "f oo"}}