From 32b4a3c0081961975112ca55a134152e2f9a713c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Casper=20K=C3=BCthe?= <43839798+Casper64@users.noreply.github.com> Date: Sat, 27 Jan 2024 06:07:00 +0100 Subject: [PATCH] x.vweb: support HTTP 1.1 persistent connections (#20658) --- vlib/x/vweb/context.v | 8 +- .../x/vweb/tests/persistent_connection_test.v | 126 ++++++++++++++++++ vlib/x/vweb/tests/vweb_test.v | 3 +- vlib/x/vweb/vweb.v | 50 +++++-- 4 files changed, 171 insertions(+), 16 deletions(-) create mode 100644 vlib/x/vweb/tests/persistent_connection_test.v diff --git a/vlib/x/vweb/context.v b/vlib/x/vweb/context.v index a8b69096211dfe..5890ce1e2def7d 100644 --- a/vlib/x/vweb/context.v +++ b/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 diff --git a/vlib/x/vweb/tests/persistent_connection_test.v b/vlib/x/vweb/tests/persistent_connection_test.v new file mode 100644 index 00000000000000..89e54e1c9a2e63 --- /dev/null +++ b/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 +} diff --git a/vlib/x/vweb/tests/vweb_test.v b/vlib/x/vweb/tests/vweb_test.v index fec4190222075b..8cb45afb44046f 100644 --- a/vlib/x/vweb/tests/vweb_test.v +++ b/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 ? { diff --git a/vlib/x/vweb/vweb.v b/vlib/x/vweb/vweb.v index f06bc6fdc3fa2f..7f018ea6459bc4 100644 --- a/vlib/x/vweb/vweb.v +++ b/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() }