BUG/MEDIUM: http: fix authority parsing for absolute-form URI with empty path

http_parse_authority() only stopped scanning at '/', not '?' or '#'.
For an absolute-form request-target with no path but a query string
(e.g. "http://host?token=..."), the authority scan ran to the end of
the URI and swallowed the query string into the authority. This
caused http_scheme_based_normalize() to see an empty path and append
'/' after the query string instead of between the host and the
query, corrupting the request on the wire. The same corruption
happens with a literal '#' in the request-target when HTTP
violations are tolerated (option accept-unsafe-violations-in-http-
request), since it is not rejected by the request-line parser in
that mode either.

Per RFC 3986 section 3.2, authority terminates at '/', '?', or '#',
or at the end of the URI, so also stop at these two delimiters.

Add cases to h1_host_normalization.vtc covering an empty path with a
query string, with and without a port needing normalization. Add a
new h1_authority_fragment_char.vtc covering the '#' terminator
specifically, since it requires accept-unsafe-violations-in-http-
request to reach the parser at all and doesn't belong in the shared
frontend used by the other host-normalization cases.

This should be backported to all stable versions.

Should fix issue #3460.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

diff --git a/reg-tests/http-messaging/h1_authority_fragment_char.vtc b/reg-tests/http-messaging/h1_authority_fragment_char.vtc
new file mode 100644 (file)
index 0000000..d43c7a3
--- /dev/null
+++ b/reg-tests/http-messaging/h1_authority_fragment_char.vtc
@@ -0,0 +1,72 @@
+varnishtest "H1 absolute-form URI whose authority is terminated by '#' (rfc3986 3.2)"
+
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.6-dev0)'"
+feature ignore_unknown_macro
+
+barrier b1 cond 2 -cyclic
+
+syslog S1 -level info {
+       # C1
+       recv
+       expect ~ "^.* uri: GET http://hostname#23frag HTTP/1.1; host: {hostname}$"
+       barrier b1 sync
+
+       # C2
+       recv
+       expect ~ "^.* uri: GET http://hostname/abc#23frag HTTP/1.1; host: {hostname}$"
+} -start
+
+haproxy h1 -conf {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+       defaults
+               mode http
+               timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+               timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+               timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+       frontend fe
+               bind "fd@${fe}"
+
+               # a raw '#' in the request-target is rejected by default; relax this
+               # so the case reaches authority parsing.
+               option accept-unsafe-violations-in-http-request
+
+               http-request capture req.hdr(host) len 512
+               log-format "uri: %r; host: %hr"
+               log ${S1_addr}:${S1_port} len 2048 local0 debug err
+
+               http-request return status 200
+} -start
+
+# no path, authority terminated by '#' rather than '/' or '?' => must not be
+# absorbed into the authority, and the uri must be passed through untouched
+client c1 -connect ${h1_fe_sock} {
+       txreq \
+         -req "GET" \
+         -url "http://hostname#frag" \
+         -hdr "host: hostname"
+
+       rxresp
+       expect resp.status == 200
+} -run
+
+# Wait matching on log message
+barrier b1 sync
+
+# a path is present before the '#', so authority is terminated by '/' as usual;
+# the trailing fragment must be preserved as part of the path
+client c2 -connect ${h1_fe_sock} {
+       txreq \
+         -req "GET" \
+         -url "http://hostname/abc#frag" \
+         -hdr "host: hostname"
+
+       rxresp
+       expect resp.status == 200
+} -run
+
+syslog S1 -wait

diff --git a/reg-tests/http-messaging/h1_host_normalization.vtc b/reg-tests/http-messaging/h1_host_normalization.vtc
index 0d61127055eae6054be9fb3a96f2e27bac0363b3..048ad95bdebb9e9e2e7ebb6a0a84671f8391e55d 100644 (file)
--- a/reg-tests/http-messaging/h1_host_normalization.vtc
+++ b/reg-tests/http-messaging/h1_host_normalization.vtc
@@ -231,6 +231,16 @@ syslog S1 -level info {
     # C43
     recv
     expect ~ "^.* uri: GET https://hostname:444/ HTTP/1.1; host: {hostname:444}$"
+    barrier b1 sync
+
+    # C44
+    recv
+    expect ~ "^.* uri: GET http://hostname\\?a=b HTTP/1.1; host: {hostname}$"
+    barrier b1 sync
+
+    # C45
+    recv
+    expect ~ "^.* uri: GET http://hostname\\?c=d HTTP/1.1; host: {hostname}$"
 
 } -start
 
@@ -908,4 +918,32 @@ client c43 -connect ${h1_fe_sock} {
     expect resp.status == 200
 } -run
 
+# Wait matching on log message
+barrier b1 sync
+
+# empty path with a query string, no port => query must be preserved
+client c44 -connect ${h1_fe_sock} {
+    txreq \
+      -req "GET" \
+      -url "http://hostname?a=b" \
+      -hdr "host: hostname"
+
+    rxresp
+    expect resp.status == 200
+} -run
+
+# Wait matching on log message
+barrier b1 sync
+
+# empty path with a query string and a default port => no stray '/' after query
+client c45 -connect ${h1_fe_sock} {
+    txreq \
+      -req "GET" \
+      -url "http://hostname:80?c=d" \
+      -hdr "host: hostname:80"
+
+    rxresp
+    expect resp.status == 200
+} -run
+
 syslog S1 -wait

diff --git a/src/http.c b/src/http.c
index e7e78300da978948c8b17e21406202b72d6d06c9..c29f77572f9cb760d41def356daf82cb33f955b8 100644 (file)
--- a/src/http.c
+++ b/src/http.c
@@ -682,12 +682,12 @@ struct ist http_parse_authority(struct http_uri_parser *parser, int no_userinfo)
        ptr = start = istptr(parser->uri);
        end = istend(parser->uri);
 
-       while (ptr < end && *ptr != '/') {
+       while (ptr < end && *ptr != '/' && *ptr != '?' && *ptr != '#') {
                if (*ptr++ == '@' && no_userinfo)
                        start = ptr;
        }
 
-       /* OK, ptr point on the '/' or the end */
+       /* OK, ptr point on the '/', the '?', the '#' or the end */
 
   authority:
        parser->uri = ist2(ptr, end - ptr);