From a0e27d3fd9c8858e3918b5c40f92f568ba0fea3f Mon Sep 17 00:00:00 2001 From: Miccah Date: Sat, 24 Jul 2021 12:47:45 -0500 Subject: [PATCH] net.http: refactor the Response struct (#10922) --- vlib/net/http/cookie.v | 185 +++++++++++++++--------------- vlib/net/http/download.v | 2 +- vlib/net/http/http_httpbin_test.v | 6 +- vlib/net/http/http_test.v | 6 +- vlib/net/http/request.v | 4 +- vlib/net/http/response.v | 114 ++++++++++++++---- vlib/net/http/server.v | 12 +- vlib/net/http/version.v | 10 ++ vlib/vweb/tests/vweb_test.v | 10 +- vlib/vweb/vweb.v | 53 ++++----- 10 files changed, 242 insertions(+), 160 deletions(-) diff --git a/vlib/net/http/cookie.v b/vlib/net/http/cookie.v index e678d93540..d647b3daf6 100644 --- a/vlib/net/http/cookie.v +++ b/vlib/net/http/cookie.v @@ -48,96 +48,8 @@ pub fn read_set_cookies(h map[string][]string) []&Cookie { } mut cookies := []&Cookie{} for _, line in cookies_s { - mut parts := line.trim_space().split(';') - if parts.len == 1 && parts[0] == '' { - continue - } - parts[0] = parts[0].trim_space() - keyval := parts[0].split('=') - if keyval.len != 2 { - continue - } - name := keyval[0] - raw_value := keyval[1] - if !is_cookie_name_valid(name) { - continue - } - value := parse_cookie_value(raw_value, true) or { continue } - mut c := &Cookie{ - name: name - value: value - raw: line - } - for i, _ in parts { - parts[i] = parts[i].trim_space() - if parts[i].len == 0 { - continue - } - mut attr := parts[i] - mut raw_val := '' - if attr.contains('=') { - pieces := attr.split('=') - attr = pieces[0] - raw_val = pieces[1] - } - lower_attr := attr.to_lower() - val := parse_cookie_value(raw_val, false) or { - c.unparsed << parts[i] - continue - } - match lower_attr { - 'samesite' { - lower_val := val.to_lower() - match lower_val { - 'lax' { c.same_site = .same_site_lax_mode } - 'strict' { c.same_site = .same_site_strict_mode } - 'none' { c.same_site = .same_site_none_mode } - else { c.same_site = .same_site_default_mode } - } - } - 'secure' { - c.secure = true - continue - } - 'httponly' { - c.http_only = true - continue - } - 'domain' { - c.domain = val - continue - } - 'max-age' { - mut secs := val.int() - if secs != 0 && val[0] != `0` { - break - } - if secs <= 0 { - secs = -1 - } - c.max_age = secs - continue - } - // TODO: Fix this once time works better - // 'expires' { - // c.raw_expires = val - // mut exptime := time.parse_iso(val) - // if exptime.year == 0 { - // exptime = time.parse_iso('Mon, 02-Jan-2006 15:04:05 MST') - // } - // c.expires = exptime - // continue - // } - 'path' { - c.path = val - continue - } - else { - c.unparsed << parts[i] - } - } - } - cookies << c + c := parse_cookie(line) or { continue } + cookies << &c } return cookies } @@ -406,3 +318,96 @@ fn is_cookie_name_valid(name string) bool { } return true } + +fn parse_cookie(line string) ?Cookie { + mut parts := line.trim_space().split(';') + if parts.len == 1 && parts[0] == '' { + return error('malformed cookie') + } + parts[0] = parts[0].trim_space() + keyval := parts[0].split('=') + if keyval.len != 2 { + return error('malformed cookie') + } + name := keyval[0] + raw_value := keyval[1] + if !is_cookie_name_valid(name) { + return error('malformed cookie') + } + value := parse_cookie_value(raw_value, true) or { return error('malformed cookie') } + mut c := Cookie{ + name: name + value: value + raw: line + } + for i, _ in parts { + parts[i] = parts[i].trim_space() + if parts[i].len == 0 { + continue + } + mut attr := parts[i] + mut raw_val := '' + if attr.contains('=') { + pieces := attr.split('=') + attr = pieces[0] + raw_val = pieces[1] + } + lower_attr := attr.to_lower() + val := parse_cookie_value(raw_val, false) or { + c.unparsed << parts[i] + continue + } + match lower_attr { + 'samesite' { + lower_val := val.to_lower() + match lower_val { + 'lax' { c.same_site = .same_site_lax_mode } + 'strict' { c.same_site = .same_site_strict_mode } + 'none' { c.same_site = .same_site_none_mode } + else { c.same_site = .same_site_default_mode } + } + } + 'secure' { + c.secure = true + continue + } + 'httponly' { + c.http_only = true + continue + } + 'domain' { + c.domain = val + continue + } + 'max-age' { + mut secs := val.int() + if secs != 0 && val[0] != `0` { + break + } + if secs <= 0 { + secs = -1 + } + c.max_age = secs + continue + } + // TODO: Fix this once time works better + // 'expires' { + // c.raw_expires = val + // mut exptime := time.parse_iso(val) + // if exptime.year == 0 { + // exptime = time.parse_iso('Mon, 02-Jan-2006 15:04:05 MST') + // } + // c.expires = exptime + // continue + // } + 'path' { + c.path = val + continue + } + else { + c.unparsed << parts[i] + } + } + } + return c +} diff --git a/vlib/net/http/download.v b/vlib/net/http/download.v index 18cf585ae0..455c1e0507 100644 --- a/vlib/net/http/download.v +++ b/vlib/net/http/download.v @@ -10,7 +10,7 @@ pub fn download_file(url string, out string) ? { println('download file url=$url out=$out') } s := get(url) or { return err } - if s.status_code != 200 { + if s.status() != .ok { return error('received http code $s.status_code') } os.write_file(out, s.text) ? diff --git a/vlib/net/http/http_httpbin_test.v b/vlib/net/http/http_httpbin_test.v index f896ebc921..20230992a0 100644 --- a/vlib/net/http/http_httpbin_test.v +++ b/vlib/net/http/http_httpbin_test.v @@ -37,7 +37,7 @@ fn test_http_fetch_bare() { } responses := http_fetch_mock([], FetchConfig{}) or { panic(err) } for response in responses { - assert response.status_code == 200 + assert response.status() == .ok } } @@ -68,7 +68,7 @@ fn test_http_fetch_with_params() { // payload := json.decode(HttpbinResponseBody,response.text) or { // panic(err) // } - assert response.status_code == 200 + assert response.status() == .ok // TODO // assert payload.args['a'] == 'b' // assert payload.args['c'] == 'd' @@ -88,7 +88,7 @@ fn test_http_fetch_with_headers() ? { // payload := json.decode(HttpbinResponseBody,response.text) or { // panic(err) // } - assert response.status_code == 200 + assert response.status() == .ok // TODO // assert payload.headers['Test-Header'] == 'hello world' } diff --git a/vlib/net/http/http_test.v b/vlib/net/http/http_test.v index 0aa628e53e..8b68073b9f 100644 --- a/vlib/net/http/http_test.v +++ b/vlib/net/http/http_test.v @@ -16,7 +16,7 @@ fn test_http_get_from_vlang_utc_now() { for url in urls { println('Test getting current time from $url by http.get') res := http.get(url) or { panic(err) } - assert 200 == res.status_code + assert res.status() == .ok assert res.text.len > 0 assert res.text.int() > 1566403696 println('Current time is: $res.text.int()') @@ -38,7 +38,7 @@ fn test_public_servers() { for url in urls { println('Testing http.get on public url: $url ') res := http.get(url) or { panic(err) } - assert 200 == res.status_code + assert res.status() == .ok assert res.text.len > 0 } } @@ -50,7 +50,7 @@ fn test_relative_redirects() { return } // tempfix periodic: httpbin relative redirects are broken res := http.get('https://httpbin.org/relative-redirect/3?abc=xyz') or { panic(err) } - assert 200 == res.status_code + assert res.status() == .ok assert res.text.len > 0 assert res.text.contains('"abc": "xyz"') } diff --git a/vlib/net/http/request.v b/vlib/net/http/request.v index 453b30b98b..4664659ade 100644 --- a/vlib/net/http/request.v +++ b/vlib/net/http/request.v @@ -56,7 +56,9 @@ pub fn (req &Request) do() ?Response { } qresp := req.method_and_url_to_response(req.method, rurl) ? resp = qresp - if resp.status_code !in [301, 302, 303, 307, 308] { + if resp.status() !in [.moved_permanently, .found, .see_other, .temporary_redirect, + .permanent_redirect, + ] { break } // follow any redirects diff --git a/vlib/net/http/response.v b/vlib/net/http/response.v index 48543d0ddf..caa8228e64 100644 --- a/vlib/net/http/response.v +++ b/vlib/net/http/response.v @@ -4,19 +4,20 @@ module http import net.http.chunked +import strconv // Response represents the result of the request pub struct Response { pub mut: - text string - header Header - cookies map[string]string - status_code int - version Version + text string + header Header + status_code int + status_msg string + http_version string } fn (mut resp Response) free() { - unsafe { resp.header.data.free() } + unsafe { resp.header.free() } } // Formats resp to bytes suitable for HTTP response transmission @@ -27,42 +28,109 @@ pub fn (resp Response) bytes() []byte { // Formats resp to a string suitable for HTTP response transmission pub fn (resp Response) bytestr() string { - // TODO: cookies - return ('$resp.version $resp.status_code ${status_from_int(resp.status_code).str()}\r\n' + '${resp.header.render( - version: resp.version + return ('HTTP/$resp.http_version $resp.status_code $resp.status_msg\r\n' + '${resp.header.render( + version: resp.version() )}\r\n' + '$resp.text') } // Parse a raw HTTP response into a Response object pub fn parse_response(resp string) ?Response { - // TODO: Cookie data type - mut cookies := map[string]string{} - first_header := resp.all_before('\n') - mut status_code := 0 - if first_header.contains('HTTP/') { - val := first_header.find_between(' ', ' ') - status_code = val.int() - } + version, status_code, status_msg := parse_status_line(resp.all_before('\n')) ? // Build resp header map and separate the body start_idx, end_idx := find_headers_range(resp) ? header := parse_headers(resp.substr(start_idx, end_idx)) ? mut text := resp.substr(end_idx, resp.len) - // set cookies - for cookie in header.values(.set_cookie) { - parts := cookie.split_nth('=', 2) - cookies[parts[0]] = parts[1] - } if header.get(.transfer_encoding) or { '' } == 'chunked' { text = chunked.decode(text) } return Response{ + http_version: version status_code: status_code + status_msg: status_msg header: header - cookies: cookies text: text } } +// parse_status_line parses the first HTTP response line into the HTTP +// version, status code, and reason phrase +fn parse_status_line(line string) ?(string, int, string) { + if line.len < 5 || line[..5].to_lower() != 'http/' { + return error('response does not start with HTTP/') + } + data := line.split_nth(' ', 3) + if data.len != 3 { + return error('expected at least 3 tokens') + } + version := data[0].substr(5, data[0].len) + // validate version is 1*DIGIT "." 1*DIGIT + digits := version.split_nth('.', 3) + if digits.len != 2 { + return error('HTTP version malformed') + } + for digit in digits { + strconv.atoi(digit) or { return error('HTTP version must contain only integers') } + } + return version, strconv.atoi(data[1]) ?, data[2] +} + +// cookies parses the Set-Cookie headers into Cookie objects +pub fn (r Response) cookies() []Cookie { + mut cookies := []Cookie{} + for cookie in r.header.values(.set_cookie) { + cookies << parse_cookie(cookie) or { continue } + } + return cookies +} + +// status parses the status_code into a Status struct +pub fn (r Response) status() Status { + return status_from_int(r.status_code) +} + +// set_status sets the status_code and status_msg of the response +pub fn (mut r Response) set_status(s Status) { + r.status_code = s.int() + r.status_msg = s.str() +} + +// version parses the version +pub fn (r Response) version() Version { + return version_from_str('HTTP/$r.http_version') +} + +// set_version sets the http_version string of the response +pub fn (mut r Response) set_version(v Version) { + if v == .unknown { + r.http_version = '' + return + } + maj, min := v.protos() + r.http_version = '${maj}.$min' +} + +pub struct ResponseConfig { + version Version = .v1_1 + status Status = .ok + header Header + text string +} + +// new_response creates a Response object from the configuration. This +// function will add a Content-Length header if text is not empty. +pub fn new_response(conf ResponseConfig) Response { + mut resp := Response{ + text: conf.text + header: conf.header + } + if conf.text.len > 0 && !resp.header.contains(.content_length) { + resp.header.add(.content_length, conf.text.len.str()) + } + resp.set_status(conf.status) + resp.set_version(conf.version) + return resp +} + // find_headers_range returns the start (inclusive) and end (exclusive) // index of the headers in the string, including the trailing newlines. This // helper function expects the first line in `data` to be the HTTP status line diff --git a/vlib/net/http/server.v b/vlib/net/http/server.v index 80ecd7cabc..edb2eaf491 100644 --- a/vlib/net/http/server.v +++ b/vlib/net/http/server.v @@ -24,13 +24,13 @@ pub fn (mut s Server) listen_and_serve() ? { } $else { eprintln('[$time.now()] $req.method $req.url - 200') } - return Response{ - version: req.version + mut r := Response{ text: req.data header: req.header - cookies: req.cookies - status_code: int(Status.ok) } + r.set_status(.ok) + r.set_version(req.version) + return r } } mut l := net.listen_tcp(.ip6, ':$s.port') ? @@ -64,8 +64,8 @@ fn (mut s Server) parse_and_respond(mut conn net.TcpConn) { return } mut resp := s.handler(req) - if resp.version == .unknown { - resp.version = req.version + if resp.version() == .unknown { + resp.set_version(req.version) } conn.write(resp.bytes()) or { eprintln('error sending response: $err') } } diff --git a/vlib/net/http/version.v b/vlib/net/http/version.v index 95a13bf2b7..f4388a3c04 100644 --- a/vlib/net/http/version.v +++ b/vlib/net/http/version.v @@ -28,3 +28,13 @@ pub fn version_from_str(v string) Version { else { Version.unknown } } } + +// protos returns the version major and minor numbers +pub fn (v Version) protos() (int, int) { + match v { + .v1_1 { return 1, 1 } + .v2_0 { return 2, 0 } + .v1_0 { return 1, 0 } + .unknown { return 0, 0 } + } +} diff --git a/vlib/vweb/tests/vweb_test.v b/vlib/vweb/tests/vweb_test.v index 2c4a52313f..f858e15d86 100644 --- a/vlib/vweb/tests/vweb_test.v +++ b/vlib/vweb/tests/vweb_test.v @@ -109,7 +109,7 @@ fn test_a_simple_tcp_client_html_page() { // net.http client based tests follow: fn assert_common_http_headers(x http.Response) ? { - assert x.status_code == 200 + assert x.status() == .ok assert x.header.get(.server) ? == 'VWeb' assert x.header.get(.content_length) ?.int() > 0 assert x.header.get(.connection) ? == 'close' @@ -130,7 +130,7 @@ fn test_http_client_404() ? { ] for url in url_404_list { res := http.get(url) or { panic(err) } - assert res.status_code == 404 + assert res.status() == .not_found } } @@ -168,7 +168,7 @@ fn test_http_client_user_repo_settings_page() ? { assert y.text == 'username: kent | repository: golang' // z := http.get('http://127.0.0.1:$sport/missing/golang/settings') or { panic(err) } - assert z.status_code == 404 + assert z.status() == .not_found } struct User { @@ -231,7 +231,7 @@ fn test_http_client_shutdown_does_not_work_without_a_cookie() { assert err.msg == '' return } - assert x.status_code == 404 + assert x.status() == .not_found assert x.text == '404 Not Found' } @@ -247,7 +247,7 @@ fn testsuite_end() { assert err.msg == '' return } - assert x.status_code == 200 + assert x.status() == .ok assert x.text == 'good bye' } diff --git a/vlib/vweb/vweb.v b/vlib/vweb/vweb.v index 635c416cd9..32a2436cc4 100644 --- a/vlib/vweb/vweb.v +++ b/vlib/vweb/vweb.v @@ -17,34 +17,31 @@ pub const ( http.CommonHeader.connection.str(): 'close' }) or { panic('should never fail') } - http_400 = http.Response{ - version: .v1_1 - status_code: 400 + http_400 = http.new_response( + status: .bad_request text: '400 Bad Request' - header: http.new_header_from_map(map{ - http.CommonHeader.content_type: 'text/plain' - http.CommonHeader.content_length: '15' - }).join(headers_close) - } - http_404 = http.Response{ - version: .v1_1 - status_code: 404 + header: http.new_header( + key: .content_type + value: 'text/plain' + ).join(headers_close) + ) + http_404 = http.new_response( + status: .not_found text: '404 Not Found' - header: http.new_header_from_map(map{ - http.CommonHeader.content_type: 'text/plain' - http.CommonHeader.content_length: '13' - }).join(headers_close) - } - http_500 = http.Response{ - version: .v1_1 - status_code: 500 + header: http.new_header( + key: .content_type + value: 'text/plain' + ).join(headers_close) + ) + http_500 = http.new_response( + status: .internal_server_error text: '500 Internal Server Error' - header: http.new_header_from_map(map{ - http.CommonHeader.content_type: 'text/plain' - http.CommonHeader.content_length: '25' - }).join(headers_close) - } - mime_types = map{ + header: http.new_header( + key: .content_type + value: 'text/plain' + ).join(headers_close) + ) + mime_types = map{ '.css': 'text/css; charset=utf-8' '.gif': 'image/gif' '.htm': 'text/html; charset=utf-8' @@ -134,12 +131,12 @@ pub fn (mut ctx Context) send_response_to_client(mimetype string, res string) bo http.CommonHeader.content_length: res.len.str() }).join(ctx.header) - resp := http.Response{ - version: .v1_1 - status_code: ctx.status.int() // TODO: change / remove ctx.status + mut resp := http.Response{ header: header.join(vweb.headers_close) text: res } + resp.set_version(.v1_1) + resp.set_status(http.status_from_int(ctx.status.int())) send_string(mut ctx.conn, resp.bytestr()) or { return false } return true }