Skip to content

Commit 72612ad

Browse files
krynjuclaude
andauthored
client: don't synthesize the default port into the Host header (#1318)
* client: don't synthesize the default port into the Host header For a URL without an explicit port, the client built the request `Host` header from the connection address, which always carries the scheme's default port (`example.com:443`). The Host header should mirror the URL authority as written, so a bare-host URL must send a bare `Host`. Sending `Host: host:443` is legal per RFC 9110 but breaks any server that treats the Host verbatim. Concretely it breaks AWS SigV4 presigned URLs: the signature is computed over the canonical host (`s3.amazonaws.com`), so a request whose Host carries `:443` is rejected with SignatureDoesNotMatch (403). Go's net/http, curl and HTTP.jl 1.x all keep the Host as written. Add a `host_header` view on the parsed URL that preserves an explicit port but never synthesizes the default one (keeping IPv6 brackets), and build `request.host` from it. The connection address is unchanged, so dialing still targets the right port. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * client: route streaming, WebSocket and redirect paths through host_header The previous commit stopped synthesizing the scheme's default port into the Host header, but only for the high-level request path. The sibling client paths still built the request from the connection address, so a default-port URL leaked Host: host:443 (and broke AWS SigV4) on: - HTTP.open / streaming requests (built from parsed.address) - HTTP and WebSocket redirects (reset Host to current_address per hop) - WebSocket handshakes (initial host=parsed.address) Wire all of them through the authority-as-written value: - Streaming and WebSocket init now use parsed.host_header. - _resolve_redirect_target also returns the next hop's host_header (parsed for a host-changing hop, current host carried through for a relative hop), and both redirect loops reset request.host from it instead of the address. The dial address is unchanged everywhere, so connections still target the right port. Matches Go's net/http, verified empirically: Go sends the URL authority as written on the initial request and derives the Host from the next URL on a redirect, never synthesizing the default port. Adds focused regressions for the streaming constructor and the redirect resolver (including explicit-default-port preservation), plus a WebSocket handshake Host assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: add WebSocket handshake Host regression for authority-as-written The streaming and redirect paths got focused host_header regressions, but the WebSocket coverage only asserted the explicit-port branch. Exercise the ws->http / wss->https mapping in `_parse_websocket_url` directly: a default-port wss/ws URL must yield a bare `Host`, while an explicit or custom port is preserved and the dial address keeps its port. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * client: accept a nothing Host when resolving a redirect target The previous commit threaded `current_request.host` into the new `current_host_header::String` parameter of `_resolve_redirect_target`. But `Request.host` is `Union{Nothing,String}` and is `nothing` for low-level `do!` callers that pin `Host` only in the request headers, so a redirect threw a MethodError instead of being followed. Loosen the parameter to `Union{Nothing,String}`. On a relative redirect with no parsed host, fall back to the dial `current_address` (carrying the port, as before this PR) instead of propagating `nothing` — `_prepare_request_for_redirect` strips the `Host` header each hop, so returning `nothing` would drop the `Host` header from the redirected request entirely. Absolute redirects keep returning the parsed `host_header`. Adds a unit case for the `nothing` current host and an end-to-end regression: a `do!` request with `Host` only in headers (`request.host === nothing`) following a relative redirect now completes and the redirected hop still carries a valid `Host`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a1eb82b commit 72612ad

7 files changed

Lines changed: 205 additions & 19 deletions

src/http_client.jl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,7 @@ function _do_incoming!(
895895
previous_secure = current_secure
896896
previous_address = current_address
897897
previous_target = current_request.target
898-
next_address, next_secure, next_target = _resolve_redirect_target(current_address, current_secure, location, current_request.target)
898+
next_address, next_secure, next_target, next_host_header = _resolve_redirect_target(current_address, current_secure, location, current_request.target, current_request.host)
899899
_emit_trace(
900900
trace,
901901
RedirectEvent(
@@ -940,7 +940,7 @@ function _do_incoming!(
940940
cookies = Cookie[]
941941
end
942942
end
943-
current_request.host = current_address
943+
current_request.host = next_host_header
944944
break
945945
end
946946
end
@@ -1944,7 +1944,7 @@ function request(
19441944
parsed.target;
19451945
headers=req_headers,
19461946
body=normalized_body.body,
1947-
host=parsed.address,
1947+
host=parsed.host_header,
19481948
content_length=normalized_body.content_length,
19491949
context=req_context,
19501950
)

src/http_client_redirect.jl

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,19 +199,34 @@ function _normalize_redirect_authority(authority::String, secure::Bool)::String
199199
return HostResolvers.join_host_port(authority, secure ? 443 : 80)
200200
end
201201

202-
function _resolve_redirect_target(current_address::String, current_secure::Bool, location::String, current_target::String)
202+
# Returns `(address, secure, target, host_header)` for the next hop. `address`
203+
# is the dial target (always carries a port); `host_header` is the authority as
204+
# written for the `Host` header (default port never synthesized) — see
205+
# `_urlparts_host_header!`. For a host-changing redirect both come from the
206+
# parsed Location; for a same-authority relative redirect the host is unchanged,
207+
# so `current_host_header` is carried through verbatim.
208+
#
209+
# `current_host_header` is the current hop's `request.host`, which is `nothing`
210+
# for low-level `do!` callers that pin `Host` only in the request headers. The
211+
# header is stripped on each hop by `_prepare_request_for_redirect`, so the next
212+
# `Host` is taken from this returned value; on a relative redirect with no parsed
213+
# host we fall back to the dial `address` (carrying the port, as before this
214+
# change) rather than returning `nothing`, which would drop the `Host` header
215+
# entirely. An absolute redirect always yields a proper parsed `host_header`.
216+
function _resolve_redirect_target(current_address::String, current_secure::Bool, location::String, current_target::String, current_host_header::Union{Nothing,String})
203217
scheme_match = match(r"^([A-Za-z][A-Za-z0-9+\\.-]*):", location)
204218
if scheme_match !== nothing
205219
scheme = lowercase(String(scheme_match.captures[1]))
206220
(scheme == "http" || scheme == "https") || throw(ProtocolError("unsupported redirect location scheme '$scheme'"))
207221
parsed = _parse_http_url(location)
208-
return parsed.address, parsed.secure, parsed.target
222+
return parsed.address, parsed.secure, parsed.target, parsed.host_header
209223
end
210224
if startswith(location, "//")
211225
parsed = _parse_http_url(string(current_secure ? "https:" : "http:", location))
212-
return parsed.address, parsed.secure, parsed.target
226+
return parsed.address, parsed.secure, parsed.target, parsed.host_header
213227
end
214-
return current_address, current_secure, _resolve_relative_redirect_request_target(current_target, location)
228+
next_host_header = current_host_header === nothing ? current_address : current_host_header
229+
return current_address, current_secure, _resolve_relative_redirect_request_target(current_target, location), next_host_header
215230
end
216231

217232
function _rewrite_method_for_redirect(method::String, status::Int, policy::_RedirectPolicy)::String

src/http_client_url.jl

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,29 @@ function _urlparts_url!(parts::_URLParts)::String
147147
return url
148148
end
149149

150+
# Authority for the `Host` header. This mirrors the URL as written: an explicit
151+
# port is preserved, but the scheme's default port is never synthesized. The
152+
# connection address (`_urlparts_address!`) always carries a port for dialing,
153+
# so the two intentionally differ for a default-port URL.
154+
#
155+
# Synthesizing the default port into `Host` (e.g. `s3.amazonaws.com:443`) is
156+
# legal per RFC 9110 but breaks any server that treats the Host verbatim. The
157+
# motivating case is AWS SigV4: it signs the canonical host (`s3.amazonaws.com`)
158+
# and rejects a request whose `Host` carries `:443`. Go's net/http, curl and
159+
# HTTP.jl 1.x all keep the Host authority as written; this restores that.
160+
#
161+
# For an explicit port we use the authority directly. For a default-port URL we
162+
# strip the synthesized `:<default_port>` off `address` rather than reusing
163+
# `server_name`, because `server_name` unwraps IPv6 brackets (`2001:db8::1`)
164+
# whereas a `Host` header requires them (`[2001:db8::1]`); `address` keeps the
165+
# brackets (`[2001:db8::1]:443`), so trimming the port yields the correct form.
166+
function _urlparts_host_header!(parts::_URLParts)::String
167+
getfield(parts, :has_explicit_port) && return _urlparts_address!(parts)
168+
address = _urlparts_address!(parts)
169+
suffix = string(':', Int(getfield(parts, :default_port)))
170+
return endswith(address, suffix) ? address[1:(end - length(suffix))] : address
171+
end
172+
150173
function _urlparts_authorization!(parts::_URLParts)::Union{Nothing,String}
151174
getfield(parts, :has_userinfo) || return nothing
152175
cached = getfield(parts, :authorization_cache)
@@ -166,6 +189,8 @@ function Base.getproperty(parts::_URLParts, sym::Symbol)
166189
return _urlparts_target!(parts)
167190
elseif sym === :server_name
168191
return _urlparts_server_name!(parts)
192+
elseif sym === :host_header
193+
return _urlparts_host_header!(parts)
169194
elseif sym === :url
170195
return _urlparts_url!(parts)
171196
elseif sym === :authorization
@@ -175,7 +200,8 @@ function Base.getproperty(parts::_URLParts, sym::Symbol)
175200
end
176201

177202
function Base.propertynames(::_URLParts, private::Bool=false)
178-
return private ? fieldnames(_URLParts) : (:secure, :address, :target, :server_name, :url, :authorization)
203+
return private ? fieldnames(_URLParts) :
204+
(:secure, :address, :target, :server_name, :host_header, :url, :authorization)
179205
end
180206

181207
@inline function _request_url(secure::Bool, address::String, target::String)::String

src/http_stream.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ function _client_stream_request(
2020
copy(headers),
2121
Headers(),
2222
EmptyBody(),
23-
parsed.address,
23+
parsed.host_header,
2424
Int64(0),
2525
UInt8(1),
2626
UInt8(1),

src/http_websockets.jl

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,7 @@ function _open_client_websocket(
900900
pmce_offer = compress ? _pmce_client_offer_header() : nothing
901901
key = ws_random_handshake_key()
902902
_apply_websocket_request_headers!(req_headers, key, subprotocols, pmce_offer)
903-
request = Request("GET", parsed.target; headers=req_headers, host=parsed.address, body=EmptyBody(), content_length=0)
903+
request = Request("GET", parsed.target; headers=req_headers, host=parsed.host_header, body=EmptyBody(), content_length=0)
904904
request_timeout_ns, timeout_config = _resolve_request_timeout_settings(
905905
request_timeout,
906906
connect_timeout,
@@ -990,17 +990,18 @@ function _open_client_websocket(
990990
previous_secure = current_secure
991991
previous_address = current_address
992992
previous_target = current_request.target
993-
current_address, current_secure, next_target = _resolve_redirect_target(
993+
current_address, current_secure, next_target, next_host_header = _resolve_redirect_target(
994994
current_address,
995995
current_secure,
996996
_normalize_websocket_redirect_location(location::String),
997997
current_request.target,
998+
current_request.host,
998999
)
9991000
current_server_name = _host_for_sni(current_address)
10001001
current_request = _prepare_request_for_redirect(current_request, response.status, next_target, redirect_policy)
10011002
key = ws_random_handshake_key()
10021003
_apply_websocket_request_headers!(current_request.headers, key, subprotocols, pmce_offer)
1003-
current_request.host = current_address
1004+
current_request.host = next_host_header
10041005
next_ref = _redirect_referer(previous_secure, previous_address, previous_target, current_secure, header(current_request.headers, "Referer", nothing))
10051006
if next_ref === nothing
10061007
removeheader(current_request.headers, "Referer")

test/http_client_tests.jl

Lines changed: 131 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -777,37 +777,140 @@ end
777777
end
778778

779779
@testset "HTTP client redirect absolute location default ports" begin
780-
address_h2, secure_h2, target_h2 = HT._resolve_redirect_target("origin.com:443", true, "https://www.google.com/search", "/")
780+
# `_resolve_redirect_target` returns `(address, secure, target, host_header)`.
781+
# `address` keeps the dial port; `host_header` mirrors the next hop's authority
782+
# as written (default port never synthesized), so it feeds `request.host` on a
783+
# redirect just as `parsed.host_header` does on the initial request.
784+
address_h2, secure_h2, target_h2, host_h2 = HT._resolve_redirect_target("origin.com:443", true, "https://www.google.com/search", "/", "origin.com")
781785
@test address_h2 == "www.google.com:443"
782786
@test secure_h2
783787
@test target_h2 == "/search"
788+
@test host_h2 == "www.google.com"
784789

785-
address_h1, secure_h1, target_h1 = HT._resolve_redirect_target("origin.com:80", false, "http://example.com/next", "/")
790+
address_h1, secure_h1, target_h1, host_h1 = HT._resolve_redirect_target("origin.com:80", false, "http://example.com/next", "/", "origin.com")
786791
@test address_h1 == "example.com:80"
787792
@test !secure_h1
788793
@test target_h1 == "/next"
794+
@test host_h1 == "example.com"
789795

790-
address_rel, secure_rel, target_rel = HT._resolve_redirect_target("origin.com:443", true, "//cdn.example.com/assets", "/")
796+
# An explicit default port in the Location is preserved in the Host header.
797+
address_exp, secure_exp, target_exp, host_exp = HT._resolve_redirect_target("origin.com:443", true, "https://example.com:443/next", "/", "origin.com")
798+
@test address_exp == "example.com:443"
799+
@test host_exp == "example.com:443"
800+
801+
address_rel, secure_rel, target_rel, host_rel = HT._resolve_redirect_target("origin.com:443", true, "//cdn.example.com/assets", "/", "origin.com")
791802
@test address_rel == "cdn.example.com:443"
792803
@test secure_rel
793804
@test target_rel == "/assets"
805+
@test host_rel == "cdn.example.com"
794806

795-
address_dot, secure_dot, target_dot = HT._resolve_redirect_target("origin.com:80", false, "../next", "/a/b/c")
807+
# A same-authority relative redirect carries the current host header through
808+
# verbatim (it must not regress a bare host back to a default-port form).
809+
address_dot, secure_dot, target_dot, host_dot = HT._resolve_redirect_target("origin.com:80", false, "../next", "/a/b/c", "origin.com")
796810
@test address_dot == "origin.com:80"
797811
@test !secure_dot
798812
@test target_dot == "/a/next"
813+
@test host_dot == "origin.com"
799814

800-
address_query, secure_query, target_query = HT._resolve_redirect_target("origin.com:80", false, "?q=1", "/a/b/c")
815+
address_query, secure_query, target_query, host_query = HT._resolve_redirect_target("origin.com:80", false, "?q=1", "/a/b/c", "origin.com")
801816
@test address_query == "origin.com:80"
802817
@test !secure_query
803818
@test target_query == "/a/b/c?q=1"
819+
@test host_query == "origin.com"
804820

805-
address_frag, secure_frag, target_frag = HT._resolve_redirect_target("origin.com:80", false, "#frag", "/a/b/c?x=1")
821+
address_frag, secure_frag, target_frag, host_frag = HT._resolve_redirect_target("origin.com:80", false, "#frag", "/a/b/c?x=1", "origin.com")
806822
@test address_frag == "origin.com:80"
807823
@test !secure_frag
808824
@test target_frag == "/a/b/c?x=1"
825+
@test host_frag == "origin.com"
826+
827+
@test_throws HT.ProtocolError HT._resolve_redirect_target("origin.com:80", false, "ftp://example.com/file", "/", "origin.com")
828+
829+
# Low-level `do!` callers may pin `Host` only in headers, leaving
830+
# `request.host === nothing`. `_resolve_redirect_target` must accept a
831+
# `nothing` current host (not throw a MethodError on the `::String` arg) and
832+
# never propagate `nothing` into `request.host` — otherwise the redirected
833+
# request would carry no `Host` header at all.
834+
address_nh, secure_nh, target_nh, host_nh = HT._resolve_redirect_target("origin.com:443", true, "https://www.google.com/search", "/", nothing)
835+
@test address_nh == "www.google.com:443"
836+
@test host_nh == "www.google.com" # absolute redirect: from parsed Location
837+
address_nr, secure_nr, target_nr, host_nr = HT._resolve_redirect_target("origin.com:443", true, "../next", "/a/b/c", nothing)
838+
@test address_nr == "origin.com:443"
839+
@test host_nr == "origin.com:443" # relative redirect, no parsed host: falls back to dial address
840+
end
809841

810-
@test_throws HT.ProtocolError HT._resolve_redirect_target("origin.com:80", false, "ftp://example.com/file", "/")
842+
@testset "do! redirect with header-only Host (request.host === nothing)" begin
843+
# Low-level `do!` callers may set `Host` only in the request headers, leaving
844+
# `request.host === nothing`. A relative redirect must still be followed: the
845+
# threaded `current_host_header` is `nothing` here, and `_resolve_redirect_target`
846+
# must accept it (not throw a MethodError) and re-establish a `Host` for the
847+
# next hop rather than dropping it. Regression for PR #1318 review follow-up.
848+
listener = ND.listen("tcp", "127.0.0.1:0"; backlog = 8)
849+
laddr = NC.addr(listener)::NC.SocketAddrV4
850+
address = ND.join_host_port("127.0.0.1", Int(laddr.port))
851+
seen_host_hop1 = Ref{Union{Nothing, String}}(nothing)
852+
seen_host_hop2 = Ref{Union{Nothing, String}}(nothing)
853+
server_task = errormonitor(Threads.@spawn begin
854+
# Hop 1: /start -> 302 to a *relative* Location on the same authority.
855+
conn1 = NC.accept(listener)
856+
try
857+
req1 = HT.read_request(HT._ConnReader(conn1))
858+
seen_host_hop1[] = HT.header(req1.headers, "Host", nothing)
859+
headers = HT.Headers()
860+
HT.setheader(headers, "Location", "/final")
861+
HT.setheader(headers, "Connection", "close")
862+
_send_response_client!(conn1, req1; status = 302, reason = "Found", headers = headers, close_conn = true)
863+
finally
864+
HTTP.@try_ignore NC.close(conn1)
865+
end
866+
# Hop 2: /final -> 200; capture the Host the client sent after redirect.
867+
conn2 = NC.accept(listener)
868+
try
869+
req2 = HT.read_request(HT._ConnReader(conn2))
870+
seen_host_hop2[] = HT.header(req2.headers, "Host", nothing)
871+
_send_response_client!(conn2, req2; body_text = "ok", close_conn = true)
872+
finally
873+
HTTP.@try_ignore NC.close(conn2)
874+
end
875+
return nothing
876+
end)
877+
client = HT.Client(transport = HT.Transport(max_idle_per_host = 4, max_idle_total = 4), cookiejar = nothing)
878+
try
879+
headers = HT.Headers()
880+
HT.setheader(headers, "Host", address)
881+
req = HT.Request("GET", "/start"; headers = headers, body = HT.EmptyBody(), content_length = 0)
882+
@test req.host === nothing
883+
response = HT.do!(client, address, req; redirect_limit = 1, protocol = :h1)
884+
@test response.status == 200
885+
@test String(_read_all_body_bytes_client(response.body)) == "ok"
886+
_wait_task_client!(server_task)
887+
@test seen_host_hop1[] == address
888+
# The redirected request still carries a Host header (re-established from
889+
# the dial address since the caller provided no parsed host).
890+
@test seen_host_hop2[] == address
891+
finally
892+
close(client.transport)
893+
HTTP.@try_ignore NC.close(listener)
894+
end
895+
end
896+
897+
@testset "streaming request Host mirrors URL authority as written" begin
898+
# `HTTP.open`/streaming builds its `Request` from `_client_stream_request`,
899+
# which must use `host_header` (authority as written), not the dial
900+
# `address` (which always carries a port). Otherwise a default-port URL
901+
# leaks `Host: example.com:443` and breaks AWS SigV4, exactly as the
902+
# high-level path did.
903+
bare = HT._client_stream_request("PUT", HT._parse_http_url("https://example.com/upload"), HT.Headers(), Int64(0), nothing)
904+
@test bare.host == "example.com"
905+
906+
explicit = HT._client_stream_request("PUT", HT._parse_http_url("https://example.com:443/upload"), HT.Headers(), Int64(0), nothing)
907+
@test explicit.host == "example.com:443"
908+
909+
custom = HT._client_stream_request("PUT", HT._parse_http_url("http://minio:9000/bucket/key"), HT.Headers(), Int64(0), nothing)
910+
@test custom.host == "minio:9000"
911+
912+
ipv6 = HT._client_stream_request("GET", HT._parse_http_url("https://[2001:db8::1]/x"), HT.Headers(), Int64(0), nothing)
913+
@test ipv6.host == "[2001:db8::1]"
811914
end
812915

813916
@testset "HTTP high-level redirect derives TLS server name per hop" begin
@@ -1281,6 +1384,24 @@ end
12811384
@test parsed_query.server_name == "example.com"
12821385
@test parsed_query.url == "https://example.com:443/?x=1&y=2"
12831386
@test parsed_query.authorization === nothing
1387+
# `host_header` mirrors the URL authority: the synthesized default port
1388+
# is never added (so the `Host` header stays `example.com`, matching the
1389+
# bare host servers like AWS SigV4 sign over), while `address` keeps the
1390+
# port for dialing.
1391+
@test parsed_query.host_header == "example.com"
1392+
1393+
parsed_default_port = HT._parse_http_url("https://example.com:443/explicit")
1394+
@test parsed_default_port.address == "example.com:443"
1395+
# An explicitly-written default port is preserved verbatim (as in Go).
1396+
@test parsed_default_port.host_header == "example.com:443"
1397+
1398+
parsed_http_default = HT._parse_http_url("http://example.com/plain")
1399+
@test parsed_http_default.address == "example.com:80"
1400+
@test parsed_http_default.host_header == "example.com"
1401+
1402+
parsed_custom_port = HT._parse_http_url("http://minio:9000/bucket/key")
1403+
@test parsed_custom_port.address == "minio:9000"
1404+
@test parsed_custom_port.host_header == "minio:9000"
12841405

12851406
parsed_query_uri = HT._parse_http_url(HT.URI("https://example.com?x=1"), Dict("y" => 2))
12861407
@test parsed_query_uri.secure
@@ -1302,11 +1423,14 @@ end
13021423
@test parsed_ipv6.server_name == "2001:db8::1"
13031424
@test parsed_ipv6.url == "https://[2001:db8::1]:443/ipv6"
13041425
@test parsed_ipv6.authorization === nothing
1426+
# IPv6 Host headers keep their brackets (unlike `server_name`).
1427+
@test parsed_ipv6.host_header == "[2001:db8::1]"
13051428

13061429
parsed_ipv6_port = HT._parse_http_url("https://[2001:db8::1]:8443/ipv6")
13071430
@test parsed_ipv6_port.address == "[2001:db8::1]:8443"
13081431
@test parsed_ipv6_port.server_name == "2001:db8::1"
13091432
@test parsed_ipv6_port.url == "https://[2001:db8::1]:8443/ipv6"
1433+
@test parsed_ipv6_port.host_header == "[2001:db8::1]:8443"
13101434

13111435
parsed_ipv6_uri = HT._parse_http_url(HT.URI("https://[2001:db8::1]/ipv6"))
13121436
@test parsed_ipv6_uri.address == "[2001:db8::1]:443"

0 commit comments

Comments
 (0)