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.

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

Use protocol default port in the event that no port is provided in host #228

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

mtrudel
Copy link
Owner

@mtrudel mtrudel commented Sep 12, 2023

Fixes #106

Relevant to your interests @wkirschbaum

@wkirschbaum
Copy link

Thank you, this change means that the host port won't be converted to the underlying transport port. I feel we are still missing a piece when it comes to load balancers and identifying x-forward-for headers, but outside the scope of this discussion and probably this library.

I will test this on my side in the next couple of days.

@yordis
Copy link

yordis commented Sep 14, 2023

it would be helpful if you could rephrase this as a concrete spec
question and then send it to the HTTP WG's mailing list
(https://lists.w3.org/Archives/Public/ietf-http-wg/).
FWIW, https://greenbytes.de/tech/webdav/rfc9112.html#absolute-form

I got some help from an editor of RFC 9110 after asking for some support about the topic, clarifying a bit 🤷🏻

@mtrudel
Copy link
Owner Author

mtrudel commented Sep 15, 2023

@yordis we have pretty comprehensive coverage of absolute URIs (

describe "absolute-form request target (RFC9112§3.2.2)" do
test "derives scheme from underlying transport", context do
client = SimpleHTTP1Client.tcp_client(context)
SimpleHTTP1Client.send(client, "GET", "http://banana/echo_components")
assert {:ok, "200 OK", _headers, body} = SimpleHTTP1Client.recv_reply(client)
assert Jason.decode!(body)["scheme"] == "http"
end
@tag capture_log: true
test "uses request-line scheme even if it does not match the transport", context do
client = SimpleHTTP1Client.tcp_client(context)
SimpleHTTP1Client.send(client, "GET", "https://banana/echo_components")
assert {:ok, "200 OK", _headers, body} = SimpleHTTP1Client.recv_reply(client)
assert Jason.decode!(body)["scheme"] == "https"
end
test "derives host from the URI, even if it differs from host header", context do
client = SimpleHTTP1Client.tcp_client(context)
SimpleHTTP1Client.send(client, "GET", "http://banana/echo_components", ["host: orange"])
assert {:ok, "200 OK", _headers, body} = SimpleHTTP1Client.recv_reply(client)
assert Jason.decode!(body)["host"] == "banana"
end
# Skip this test since there is a bug in :erlang.decode_packet. See https://github.com/mtrudel/bandit/pull/97
# This has been fixed upstream in OTP26+; see OTP-18540 for details. Reintroduce this test
# once we support OTP26+
@tag :skip
test "derives ipv6 host from the URI, even if it differs from host header", context do
client = SimpleHTTP1Client.tcp_client(context)
SimpleHTTP1Client.send(
client,
"GET",
"http://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]/echo_components",
["host: orange"]
)
assert {:ok, "200 OK", _headers, body} = SimpleHTTP1Client.recv_reply(client)
assert Jason.decode!(body)["host"] == "[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]"
end
@tag capture_log: true
test "does not require a host header set in HTTP/1.1 (RFC9112§3.2.2)", context do
client = SimpleHTTP1Client.tcp_client(context)
SimpleHTTP1Client.send(client, "GET", "http://banana/echo_components")
assert {:ok, "200 OK", _headers, body} = SimpleHTTP1Client.recv_reply(client)
assert Jason.decode!(body)["host"] == "banana"
end
test "derives port from the URI, even if it differs from host header", context do
client = SimpleHTTP1Client.tcp_client(context)
SimpleHTTP1Client.send(client, "GET", "http://banana:1234/echo_components", [
"host: banana:2345"
])
assert {:ok, "200 OK", _headers, body} = SimpleHTTP1Client.recv_reply(client)
assert Jason.decode!(body)["port"] == 1234
end
for HTTP/1,
describe "absolute-form request target (with :authority header, RFC9112§3.2.2)" do
test "derives scheme from :scheme pseudo header", context do
socket = SimpleH2Client.setup_connection(context)
headers = [
{":method", "GET"},
{":path", "/echo_components"},
{":scheme", "https"},
{":authority", "banana:#{context.port}"}
]
SimpleH2Client.send_headers(socket, 1, true, headers)
assert SimpleH2Client.successful_response?(socket, 1, false)
{:ok, 1, true, body} = SimpleH2Client.recv_body(socket)
assert Jason.decode!(body)["scheme"] == "https"
end
@tag capture_log: true
test "uses :scheme even if it does not match transport", context do
socket = SimpleH2Client.setup_connection(context)
headers = [
{":method", "GET"},
{":path", "/echo_components"},
{":scheme", "http"},
{":authority", "banana:#{context.port}"}
]
SimpleH2Client.send_headers(socket, 1, true, headers)
assert SimpleH2Client.successful_response?(socket, 1, false)
{:ok, 1, true, body} = SimpleH2Client.recv_body(socket)
assert Jason.decode!(body)["scheme"] == "http"
end
test "derives host from :authority header, even if it differs from host header", context do
socket = SimpleH2Client.setup_connection(context)
headers = [
{":method", "GET"},
{":path", "/echo_components"},
{":scheme", "https"},
{":authority", "banana:#{context.port}"},
{"host", "orange:#{context.port}"}
]
SimpleH2Client.send_headers(socket, 1, true, headers)
assert SimpleH2Client.successful_response?(socket, 1, false)
{:ok, 1, true, body} = SimpleH2Client.recv_body(socket)
assert Jason.decode!(body)["host"] == "banana"
end
test "derives ipv6 host from the URI, even if it differs from host header", context do
socket = SimpleH2Client.setup_connection(context)
headers = [
{":method", "GET"},
{":path", "/echo_components"},
{":scheme", "https"},
{":authority", "[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:#{context.port}"},
{"host", "orange"}
]
SimpleH2Client.send_headers(socket, 1, true, headers)
assert SimpleH2Client.successful_response?(socket, 1, false)
{:ok, 1, true, body} = SimpleH2Client.recv_body(socket)
assert Jason.decode!(body)["host"] == "[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]"
end
test "derives port from host header, even if it differs from host header", context do
socket = SimpleH2Client.setup_connection(context)
headers = [
{":method", "GET"},
{":path", "/echo_components"},
{":scheme", "https"},
{":authority", "banana:1234"},
{"host", "banana:2345"}
]
SimpleH2Client.send_headers(socket, 1, true, headers)
assert SimpleH2Client.successful_response?(socket, 1, false)
{:ok, 1, true, body} = SimpleH2Client.recv_body(socket)
assert Jason.decode!(body)["port"] == 1234
end
test "derives port from underlying transport if no port specified in host header", context do
socket = SimpleH2Client.setup_connection(context)
headers = [
{":method", "GET"},
{":path", "/echo_components"},
{":scheme", "https"},
{":authority", "banana"}
]
SimpleH2Client.send_headers(socket, 1, true, headers)
assert SimpleH2Client.successful_response?(socket, 1, false)
{:ok, 1, true, body} = SimpleH2Client.recv_body(socket)
assert Jason.decode!(body)["port"] == context.port
end
for H2).

I've re-read the relevant spec links above a few times over the last couple of days and this is sitting better and better with me. Quite confident we're doing it right here.

@mtrudel mtrudel merged commit 8e8cc1e into main Sep 15, 2023
27 checks passed
@mtrudel mtrudel deleted the fallback_to_schema_default_port branch September 15, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Host port should default to 80 if the Host header has been set
3 participants