net.http: refactor the Response struct (#10922)

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
+}

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) ?

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'
 	}

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"')
 }

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

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
+	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

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') }
 }

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 }
+	}
+}

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'
 }
 

vlib/vweb/vweb.v

@@ -17,33 +17,30 @@ 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)
-	}
+		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'
@@ -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
 }