Skip to content

Commit 2ae0ca8

Browse files
committed
fix(net.http): guard QUIC/ngtcp2 dependency, fix compilation and correctness issues
- Guard net.http.v3/net.quic imports behind -d use_ngtcp2 compile flag so HTTP/1.1/2 users do not require ngtcp2 native libraries (P1) - Forward TLS verify/cert/cert_key/validate settings to HTTP/2 client path, fixing silent certificate validation bypass (P2-security) - Fix QUIC receive callback to capture stream data instead of discarding data/datalen parameters (P1) - Fix double-slash path parsing regression using parse_request_uri instead of urllib.parse for request-targets (P2) - Fix redirect 303 to switch method to GET and drop body (P2) - Fix HTTP/2 integration_full_test.v: headers -> header field name (P2) - Harden C/V struct interop: use i32 for QuicStreamEvents fields, add struct size assertion test - Sanitize DebugHandler to not echo sensitive request data
1 parent f70d902 commit 2ae0ca8

22 files changed

Lines changed: 671 additions & 157 deletions

vlib/net/http/request.v

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,12 @@ pub fn (req &Request) do() !Response {
100100
mut rurl := url
101101
mut resp := Response{}
102102
mut nredirects := 0
103+
mut method := req.method
103104
for {
104105
if nredirects == max_redirects {
105106
return error('http.request.do: maximum number of redirects reached (${max_redirects})')
106107
}
107-
qresp := req.method_and_url_to_response(req.method, rurl)!
108+
qresp := req.method_and_url_to_response(method, rurl)!
108109
resp = qresp
109110
if !req.allow_redirect {
110111
break
@@ -113,6 +114,18 @@ pub fn (req &Request) do() !Response {
113114
.permanent_redirect] {
114115
break
115116
}
117+
// Per HTTP spec, 303 See Other requires switching to GET and dropping the body.
118+
// 301/302 historically also switch to GET in practice (browser behavior).
119+
if resp.status() in [.moved_permanently, .found, .see_other] {
120+
method = .get
121+
// Drop the request body for GET redirects by temporarily clearing req.data.
122+
// build_request_headers reads req.data for Content-Length and body content.
123+
unsafe {
124+
mut mreq := req
125+
mreq.data = ''
126+
}
127+
}
128+
// 307/308 preserve the original method and body (no change needed)
116129
// follow any redirects
117130
mut redirect_url := resp.header.get(.location) or { '' }
118131
if redirect_url.len > 0 && redirect_url[0] == `/` {
@@ -615,7 +628,7 @@ fn parse_request_line(line string) !(Method, urllib.URL, Version) {
615628
method := method_from_str_known(method_str) or {
616629
return error('unsupported method')
617630
}
618-
target := urllib.parse(target_str)!
631+
target := urllib.parse_request_uri(target_str)!
619632
// println('before version_str="${version_str}"')
620633
version := version_from_str(version_str)
621634
// println('VERSION="${version}"')

vlib/net/http/request_headers_internal_test.v

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,21 @@ fn test_build_request_headers_respects_case_insensitive_existing_headers() {
2424
assert lower.contains('user-agent: already-present')
2525
assert lower.contains('content-length: 999')
2626
}
27+
28+
fn test_build_request_headers_method_override_for_redirect() {
29+
// Verifies that build_request_headers uses the method parameter (not req.method).
30+
// This is critical for 303 redirect handling where do() passes .get
31+
// even though req.method is .post.
32+
req := Request{
33+
method: .post
34+
url: 'http://example.com'
35+
data: 'post_body'
36+
user_agent: 'v.http'
37+
}
38+
// When method parameter is GET (simulating 303 redirect), headers should show GET
39+
headers_get := req.build_request_headers(.get, 'example.com', 80, '/redirected')
40+
assert headers_get.starts_with('GET /redirected HTTP/1.1\r\n')
41+
// When method parameter is POST (original or 307/308), headers should show POST
42+
headers_post := req.build_request_headers(.post, 'example.com', 80, '/original')
43+
assert headers_post.starts_with('POST /original HTTP/1.1\r\n')
44+
}

vlib/net/http/request_test.v

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,3 +406,51 @@ fn test_parse_request_with_limit_rejects_truncated_body() {
406406
}
407407
assert false, 'expected error for truncated body via parse_request_with_limit'
408408
}
409+
410+
fn test_parse_request_line_double_slash_path() {
411+
// Regression: GET //another.html HTTP/1.1 should preserve //another.html as path,
412+
// not interpret 'another.html' as a host (authority) due to the // prefix.
413+
method, target, version := http.parse_request_line('GET //another.html HTTP/1.1') or {
414+
panic('did not parse: ${err}')
415+
}
416+
assert method == .get
417+
assert version == .v1_1
418+
// The path must be //another.html, not empty or misinterpreted
419+
assert target.path == '//another.html'
420+
assert target.host == ''
421+
}
422+
423+
fn test_parse_request_double_slash_full_request() {
424+
// Full request parsing with double-slash path
425+
mut r := reader('GET //another.html HTTP/1.1\r\nHost: example.com\r\n\r\n')
426+
req := http.parse_request(mut r) or { panic('did not parse: ${err}') }
427+
assert req.method == .get
428+
assert req.url == '//another.html'
429+
}
430+
431+
fn test_parse_request_line_double_slash_with_query() {
432+
// Double-slash path with query string
433+
method, target, version := http.parse_request_line('GET //page?key=val HTTP/1.1') or {
434+
panic('did not parse: ${err}')
435+
}
436+
assert method == .get
437+
assert version == .v1_1
438+
assert target.path == '//page'
439+
assert target.host == ''
440+
}
441+
442+
fn test_redirect_303_method_and_body_handling() {
443+
// Verify that the Request struct supports the method/data fields needed
444+
// for redirect handling. The do() function now uses mutable local variables
445+
// for method (and unsafe mutation for data) to correctly handle 303 See Other
446+
// redirects by switching to GET and dropping the body.
447+
// Full integration testing of 303 redirects requires a live HTTP server.
448+
req := http.Request{
449+
method: .post
450+
data: 'original_body'
451+
allow_redirect: true
452+
}
453+
assert req.method == .post
454+
assert req.data == 'original_body'
455+
assert req.allow_redirect == true
456+
}

vlib/net/http/request_version.v

Lines changed: 24 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -3,33 +3,12 @@ module http
33
// HTTP version negotiation and multi-version request dispatch.
44
// Alt-Svc cache integration: do_http2 checks for Alt-Svc headers in responses
55
// and negotiate_version consults the cache before defaulting to HTTP/2.
6+
//
7+
// v3 (QUIC) support lives in request_version_d_use_ngtcp2.v and is only
8+
// compiled when `-d use_ngtcp2` is passed. The fallback stubs are in
9+
// request_version_notd_use_ngtcp2.v.
610
import net.urllib
711
import net.http.v2
8-
import net.http.v3
9-
10-
// negotiate_version selects the HTTP version for a request.
11-
// Checks Alt-Svc cache for h3 entries before defaulting to v2 for HTTPS.
12-
fn (req &Request) negotiate_version(url urllib.URL) Version {
13-
if req.version != .unknown {
14-
return req.version
15-
}
16-
17-
if url.scheme != 'https' {
18-
return .v1_1
19-
}
20-
21-
if req.alt_svc_cache != unsafe { nil } {
22-
origin := normalized_origin(url)
23-
mut cache := unsafe { req.alt_svc_cache }
24-
if _ := cache.get_h3_endpoint(origin) {
25-
return .v3_0
26-
}
27-
}
28-
29-
return .v2_0
30-
}
31-
32-
// Method is now unified via common.Method — direct cast replaces manual mapping.
3312

3413
// build_client_header prepares the request header for v2/v3 clients,
3514
// adding user-agent and content-length if not already set.
@@ -61,42 +40,11 @@ fn normalized_origin(url urllib.URL) string {
6140
return '${url.scheme}://${url.hostname()}:${port}'
6241
}
6342

64-
fn (req &Request) do_http2(url urllib.URL) !Response {
65-
host_name := url.hostname()
66-
mut nport := url.port().int()
67-
if nport == 0 {
68-
nport = 443
69-
}
70-
71-
address := '${host_name}:${nport}'
72-
73-
mut client := v2.new_client(address) or { return error('HTTP/2 connection failed: ${err}') }
74-
75-
defer {
76-
client.close()
77-
}
78-
79-
v2_req := v2.Request{
80-
method: v2.Method(req.method)
81-
url: build_request_path(url)
82-
host: host_name
83-
data: req.data
84-
header: req.build_client_header()
85-
}
86-
87-
v2_resp := client.request(v2_req) or { return error('HTTP/2 request failed: ${err}') }
88-
89-
actual_resp := if v2.is_misdirected(v2_resp) {
90-
client.close()
91-
v2.handle_misdirected(address, v2_req) or {
92-
return error('HTTP/2 misdirected retry failed: ${err}')
93-
}
94-
} else {
95-
v2_resp
96-
}
97-
43+
// maybe_store_alt_svc checks the response for an Alt-Svc header and, if
44+
// present, parses and stores the entries in the Alt-Svc cache.
45+
fn (req &Request) maybe_store_alt_svc(url urllib.URL, resp_header Header) {
9846
if req.alt_svc_cache != unsafe { nil } {
99-
if alt_svc_val := actual_resp.header.get_custom('alt-svc') {
47+
if alt_svc_val := resp_header.get_custom('alt-svc') {
10048
entries := parse_alt_svc(alt_svc_val)
10149
if entries.len > 0 {
10250
origin := normalized_origin(url)
@@ -105,15 +53,9 @@ fn (req &Request) do_http2(url urllib.URL) !Response {
10553
}
10654
}
10755
}
108-
109-
return Response{
110-
body: actual_resp.body
111-
status_code: actual_resp.status_code
112-
header: actual_resp.header
113-
}
11456
}
11557

116-
fn (req &Request) do_http3(url urllib.URL) !Response {
58+
fn (req &Request) do_http2(url urllib.URL) !Response {
11759
host_name := url.hostname()
11860
mut nport := url.port().int()
11961
if nport == 0 {
@@ -122,41 +64,38 @@ fn (req &Request) do_http3(url urllib.URL) !Response {
12264

12365
address := '${host_name}:${nport}'
12466

125-
mut client := v3.new_client(address) or { return error('HTTP/3 connection failed: ${err}') }
67+
mut client := v2.new_client_with_config(address, v2.ClientConfig{
68+
verify: req.verify
69+
cert: req.cert
70+
cert_key: req.cert_key
71+
validate: req.validate
72+
in_memory_verification: req.in_memory_verification
73+
}) or { return error('HTTP/2 connection failed: ${err}') }
12674

12775
defer {
12876
client.close()
12977
}
13078

131-
v3_req := v3.Request{
132-
method: v3.Method(req.method)
79+
v2_req := v2.Request{
80+
method: v2.Method(req.method)
13381
url: build_request_path(url)
13482
host: host_name
13583
data: req.data
13684
header: req.build_client_header()
13785
}
13886

139-
v3_resp := client.request(v3_req) or { return error('HTTP/3 request failed: ${err}') }
87+
v2_resp := client.request(v2_req) or { return error('HTTP/2 request failed: ${err}') }
14088

141-
actual_resp := if v3.is_misdirected(v3_resp) {
89+
actual_resp := if v2.is_misdirected(v2_resp) {
14290
client.close()
143-
v3.handle_misdirected(address, v3_req) or {
144-
return error('HTTP/3 misdirected retry failed: ${err}')
91+
v2.handle_misdirected(address, v2_req) or {
92+
return error('HTTP/2 misdirected retry failed: ${err}')
14593
}
14694
} else {
147-
v3_resp
95+
v2_resp
14896
}
14997

150-
if req.alt_svc_cache != unsafe { nil } {
151-
if alt_svc_val := actual_resp.header.get_custom('alt-svc') {
152-
entries := parse_alt_svc(alt_svc_val)
153-
if entries.len > 0 {
154-
origin := normalized_origin(url)
155-
mut cache := unsafe { req.alt_svc_cache }
156-
cache.store(origin, entries)
157-
}
158-
}
159-
}
98+
req.maybe_store_alt_svc(url, actual_resp.header)
16099

161100
return Response{
162101
body: actual_resp.body
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
module http
2+
3+
// HTTP/3 (QUIC) request support — compiled only with `-d use_ngtcp2`.
4+
import net.urllib
5+
import net.http.v3
6+
7+
// negotiate_version selects the HTTP version for a request.
8+
// Checks Alt-Svc cache for h3 entries before defaulting to v2 for HTTPS.
9+
fn (req &Request) negotiate_version(url urllib.URL) Version {
10+
if req.version != .unknown {
11+
return req.version
12+
}
13+
14+
if url.scheme != 'https' {
15+
return .v1_1
16+
}
17+
18+
if req.alt_svc_cache != unsafe { nil } {
19+
origin := normalized_origin(url)
20+
mut cache := unsafe { req.alt_svc_cache }
21+
if _ := cache.get_h3_endpoint(origin) {
22+
return .v3_0
23+
}
24+
}
25+
26+
return .v2_0
27+
}
28+
29+
fn (req &Request) do_http3(url urllib.URL) !Response {
30+
host_name := url.hostname()
31+
mut nport := url.port().int()
32+
if nport == 0 {
33+
nport = 443
34+
}
35+
36+
address := '${host_name}:${nport}'
37+
38+
mut client := v3.new_client(address) or { return error('HTTP/3 connection failed: ${err}') }
39+
40+
defer {
41+
client.close()
42+
}
43+
44+
v3_req := v3.Request{
45+
method: v3.Method(req.method)
46+
url: build_request_path(url)
47+
host: host_name
48+
data: req.data
49+
header: req.build_client_header()
50+
}
51+
52+
v3_resp := client.request(v3_req) or { return error('HTTP/3 request failed: ${err}') }
53+
54+
actual_resp := if v3.is_misdirected(v3_resp) {
55+
client.close()
56+
v3.handle_misdirected(address, v3_req) or {
57+
return error('HTTP/3 misdirected retry failed: ${err}')
58+
}
59+
} else {
60+
v3_resp
61+
}
62+
63+
req.maybe_store_alt_svc(url, actual_resp.header)
64+
65+
return Response{
66+
body: actual_resp.body
67+
status_code: actual_resp.status_code
68+
header: actual_resp.header
69+
}
70+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
module http
2+
3+
// Fallback stubs when QUIC/ngtcp2 is not available.
4+
import net.urllib
5+
6+
// negotiate_version selects the HTTP version for a request.
7+
// Without ngtcp2, HTTP/3 is never negotiated.
8+
fn (req &Request) negotiate_version(url urllib.URL) Version {
9+
if req.version != .unknown {
10+
return req.version
11+
}
12+
13+
if url.scheme != 'https' {
14+
return .v1_1
15+
}
16+
17+
return .v2_0
18+
}
19+
20+
fn (req &Request) do_http3(url urllib.URL) !Response {
21+
return error('HTTP/3 requires -d use_ngtcp2')
22+
}

0 commit comments

Comments
 (0)