x.vweb: support HTTP 1.1 persistent connections (#20658)

vlib/x/vweb/context.v

@@ -35,6 +35,8 @@ mut:
 	// how the http response should be handled by vweb's backend
 	return_type ContextReturnType = .normal
 	return_file string
+	// If the `Connection: close` header is present the connection should always be closed
+	client_wants_to_close bool
 pub:
 	// TODO: move this to `handle_request`
 	// time.ticks() from start of vweb connection handle.
@@ -103,9 +105,9 @@ pub fn (mut ctx Context) send_response_to_client(mimetype string, response strin
 	}
 	// send vweb's closing headers
 	ctx.res.header.set(.server, 'VWeb')
-	// sent `Connection: close header` by default, if the user hasn't specified that the
-	// connection should not be closed.
-	if !ctx.takeover {
+	if !ctx.takeover && ctx.client_wants_to_close {
+		// Only sent the `Connection: close` header when the client wants to close
+		// the connection. This typically happens when the client only supports HTTP 1.0
 		ctx.res.header.set(.connection, 'close')
 	}
 	// set the http version

vlib/x/vweb/tests/persistent_connection_test.v

@@ -0,0 +1,126 @@
+import net
+import net.http
+import io
+import os
+import time
+import x.vweb
+
+const exit_after = time.second * 10
+const port = 13009
+const localserver = 'localhost:${port}'
+const tcp_r_timeout = 2 * time.second
+const tcp_w_timeout = 2 * time.second
+const max_retries = 4
+
+const default_request = 'GET / HTTP/1.1
+User-Agent: VTESTS
+Accept: */*
+\r\n'
+
+const response_body = 'intact!'
+
+pub struct Context {
+	vweb.Context
+}
+
+pub struct App {
+mut:
+	started chan bool
+	counter int
+}
+
+pub fn (mut app App) before_accept_loop() {
+	app.started <- true
+}
+
+pub fn (mut app App) index(mut ctx Context) vweb.Result {
+	app.counter++
+	return ctx.text('${response_body}:${app.counter}')
+}
+
+pub fn (mut app App) reset(mut ctx Context) vweb.Result {
+	app.counter = 0
+	return ctx.ok('')
+}
+
+fn testsuite_begin() {
+	os.chdir(os.dir(@FILE))!
+	mut app := &App{}
+
+	spawn vweb.run_at[App, Context](mut app, port: port, timeout_in_seconds: 5)
+	_ := <-app.started
+
+	spawn fn () {
+		time.sleep(exit_after)
+		assert true == false, 'timeout reached!'
+		exit(1)
+	}()
+}
+
+fn test_conn_remains_intact() {
+	http.get('http://${localserver}/reset')!
+
+	mut conn := simple_tcp_client()!
+	conn.write_string(default_request)!
+
+	mut read := io.read_all(reader: conn)!
+	mut response := read.bytestr()
+	assert response.contains('Connection: close') == false, '`Connection` header should NOT be present!'
+	assert response.ends_with('${response_body}:1') == true, 'read response: ${response}'
+
+	// send request again over the same connection
+	conn.write_string(default_request)!
+
+	read = io.read_all(reader: conn)!
+	response = read.bytestr()
+	assert response.contains('Connection: close') == false, '`Connection` header should NOT be present!'
+	assert response.ends_with('${response_body}:2') == true, 'read response: ${response}'
+
+	conn.close() or {}
+}
+
+fn test_support_http_1() {
+	http.get('http://${localserver}/reset')!
+	// HTTP 1.0 always closes the connection after each request, so the client must
+	// send the Connection: close header. If that header is present the connection
+	// needs to be closed and a `Connection: close` header needs to be send back
+	mut x := http.fetch(http.FetchConfig{
+		url: 'http://${localserver}/'
+		header: http.new_header_from_map({
+			.connection: 'close'
+		})
+	})!
+	assert x.status() == .ok
+	if conn_header := x.header.get(.connection) {
+		assert conn_header == 'close'
+	} else {
+		assert false, '`Connection: close` header should be present!'
+	}
+}
+
+// utility code:
+
+fn simple_tcp_client() !&net.TcpConn {
+	mut client := &net.TcpConn(unsafe { nil })
+	mut tries := 0
+	for tries < max_retries {
+		tries++
+		eprintln('> client retries: ${tries}')
+		client = net.dial_tcp(localserver) or {
+			eprintln('dial error: ${err.msg()}')
+			if tries > max_retries {
+				return err
+			}
+			time.sleep(100 * time.millisecond)
+			continue
+		}
+		break
+	}
+	if client == unsafe { nil } {
+		eprintln('could not create a tcp client connection to http://${localserver} after ${max_retries} retries')
+		exit(1)
+	}
+	client.set_read_timeout(tcp_r_timeout)
+	client.set_write_timeout(tcp_w_timeout)
+	return client
+}

vlib/x/vweb/tests/vweb_test.v

@@ -111,7 +111,6 @@ fn assert_common_http_headers(x http.Response) ! {
 	assert x.status() == .ok
 	assert x.header.get(.server)! == 'VWeb'
 	assert x.header.get(.content_length)!.int() > 0
-	assert x.header.get(.connection)! == 'close'
 }
 
 fn test_http_client_index() {
@@ -119,6 +118,7 @@ fn test_http_client_index() {
 	assert_common_http_headers(x)!
 	assert x.header.get(.content_type)! == 'text/plain'
 	assert x.body == 'Welcome to VWeb'
+	assert x.header.get(.connection)! == 'close'
 }
 
 fn test_http_client_404() {
@@ -327,6 +327,7 @@ fn simple_tcp_client(config SimpleTcpClientConfig) !string {
 Host: ${config.host}
 User-Agent: ${config.agent}
 Accept: */*
+Connection: close
 ${config.headers}
 ${config.content}'
 	$if debug_net_socket_client ? {

vlib/x/vweb/vweb.v

@@ -29,8 +29,7 @@ pub fn no_result() Result {
 pub const methods_with_form = [http.Method.post, .put, .patch]
 
 pub const headers_close = http.new_custom_header_from_map({
-	'Server':                           'VWeb'
-	http.CommonHeader.connection.str(): 'close'
+	'Server': 'VWeb'
 }) or { panic('should never fail') }
 
 pub const http_302 = http.new_response(
@@ -221,10 +220,11 @@ pub struct RunParams {
 
 struct FileResponse {
 pub mut:
-	open  bool
-	file  os.File
-	total i64
-	pos   i64
+	open              bool
+	file              os.File
+	total             i64
+	pos               i64
+	should_close_conn bool
 }
 
 // close the open file and reset the struct to its default values
@@ -233,13 +233,15 @@ pub fn (mut fr FileResponse) done() {
 	fr.file.close()
 	fr.total = 0
 	fr.pos = 0
+	fr.should_close_conn = false
 }
 
 struct StringResponse {
 pub mut:
-	open bool
-	str  string
-	pos  i64
+	open              bool
+	str               string
+	pos               i64
+	should_close_conn bool
 }
 
 // free the current string and reset the struct to its default values
@@ -247,6 +249,7 @@ pub mut:
 pub fn (mut sr StringResponse) done() {
 	sr.open = false
 	sr.pos = 0
+	sr.should_close_conn = false
 	unsafe { sr.str.free() }
 }
 
@@ -418,7 +421,7 @@ fn handle_write_file(mut pv picoev.Picoev, mut params RequestParams, fd int) {
 	if params.file_responses[fd].pos == params.file_responses[fd].total {
 		// file is done writing
 		params.file_responses[fd].done()
-		pv.close_conn(fd)
+		handle_complete_request(params.file_responses[fd].should_close_conn, mut pv, fd)
 		return
 	}
 }
@@ -450,6 +453,8 @@ fn handle_write_string(mut pv picoev.Picoev, mut params RequestParams, fd int) {
 		// done writing
 		params.string_responses[fd].done()
 		pv.close_conn(fd)
+		handle_complete_request(params.string_responses[fd].should_close_conn, mut pv,
+			fd)
 		return
 	}
 }
@@ -490,6 +495,8 @@ fn handle_read[A, X](mut pv picoev.Picoev, mut params RequestParams, fd int) {
 			if err !is io.Eof {
 				eprintln('[vweb] error parsing request: ${err}')
 			}
+			// the buffered reader was empty meaning that the client probably
+			// closed the connection.
 			pv.close_conn(fd)
 			params.incomplete_requests[fd] = http.Request{}
 			return
@@ -588,7 +595,8 @@ fn handle_read[A, X](mut pv picoev.Picoev, mut params RequestParams, fd int) {
 				// See Context.send_file for why we use max_read instead of max_write.
 				if completed_context.res.body.len < vweb.max_read {
 					fast_send_resp(mut conn, completed_context.res) or {}
-					pv.close_conn(fd)
+					handle_complete_request(completed_context.client_wants_to_close, mut
+						pv, fd)
 				} else {
 					params.string_responses[fd].open = true
 					params.string_responses[fd].str = completed_context.res.body
@@ -599,7 +607,8 @@ fn handle_read[A, X](mut pv picoev.Picoev, mut params RequestParams, fd int) {
 						// should not happen
 						params.string_responses[fd].done()
 						fast_send_resp(mut conn, vweb.http_500) or {}
-						pv.close_conn(fd)
+						handle_complete_request(completed_context.client_wants_to_close, mut
+							pv, fd)
 						return
 					}
 					// no errors we can send the HTTP headers
@@ -636,6 +645,15 @@ fn handle_read[A, X](mut pv picoev.Picoev, mut params RequestParams, fd int) {
 			}
 		}
 	} else {
+		// invalid request headers/data
+		pv.close_conn(fd)
+	}
+}
+
+// close the connection when `should_close` is true.
+@[inline]
+fn handle_complete_request(should_close bool, mut pv picoev.Picoev, fd int) {
+	if should_close {
 		pv.close_conn(fd)
 	}
 }
@@ -681,6 +699,14 @@ fn handle_request[A, X](mut conn net.TcpConn, req http.Request, params &RequestP
 		files: files
 	}
 
+	if connection_header := req.header.get(.connection) {
+		// A client that does not support persistent connections MUST send the
+		// "close" connection option in every request message.
+		if connection_header.to_lower() == 'close' {
+			ctx.client_wants_to_close = true
+		}
+	}
+
 	$if A is StaticApp {
 		ctx.custom_mime_types = global_app.static_mime_types.clone()
 	}