Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow not validating target #456

Merged
merged 5 commits into from
Nov 9, 2024
Merged

Allow not validating target #456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
404a702
Introduce new HTTP1 option: skip_target_validation
PragTob Nov 8, 2024
573a863
Centralize mint_user_agent in tests
PragTob Nov 8, 2024
f8fbd26
Update lib/mint/http1.ex
PragTob Nov 8, 2024
5f2e805
Use module attribute for `mint_user_agent`
PragTob Nov 8, 2024
6ec9faa
Move target validation to `Mint.HTTP1`
PragTob Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion lib/mint/http1.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ defmodule Mint.HTTP1 do
:mode,
:scheme_as_string,
:case_sensitive_headers,
:skip_target_validation,
requests: :queue.new(),
state: :closed,
buffer: "",
Expand Down Expand Up @@ -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()) ::
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
23 changes: 0 additions & 23 deletions lib/mint/http1/request.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
63 changes: 51 additions & 12 deletions test/mint/http1/conn_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a small convenience change I made in a separate commit - hope it's not too much noise but I also didn't wanna do it in a separate PR. Happy to roll it back!


\
""")
Expand All @@ -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()}

\
""")
Expand All @@ -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\
""")
Expand All @@ -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()}

\
""")
Expand All @@ -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\
Expand Down Expand Up @@ -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)
PragTob marked this conversation as resolved.
Show resolved Hide resolved

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
Expand All @@ -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()}

\
""")
Expand All @@ -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

\
Expand All @@ -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

\
Expand All @@ -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

\
Expand All @@ -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

\
Expand All @@ -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()}

\
""")
Expand All @@ -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()}

\
""")
Expand Down Expand Up @@ -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
10 changes: 0 additions & 10 deletions test/mint/http1/request_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"}}
Expand Down
Loading