veb: fix handling of default CorsOptions.allowed_headers (#24703)

Author: einar-hjortdal

vlib/veb/middleware.v

@@ -177,7 +177,7 @@ interface HasBeforeRequest {
 }
 
 pub const cors_safelisted_response_headers = [http.CommonHeader.cache_control, .content_language,
-	.content_length, .content_type, .expires, .last_modified, .pragma].map(it.str())
+	.content_length, .content_type, .expires, .last_modified, .pragma].map(it.str()).join(',')
 
 // CorsOptions is used to set CORS response headers.
 // See https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#the_http_response_headers
@@ -190,7 +190,7 @@ pub:
 	// ;`Access-Control-Allow-Credentials`
 	allow_credentials bool
 	// allowed HTTP headers for a cross-origin request; `Access-Control-Allow-Headers`
-	allowed_headers []string = ['*']
+	allowed_headers []string
 	// allowed HTTP methods for a cross-origin request; `Access-Control-Allow-Methods`
 	allowed_methods []http.Method
 	// indicate if clients are able to access other headers than the "CORS-safelisted"
@@ -225,7 +225,7 @@ pub fn (options &CorsOptions) set_headers(mut ctx Context) {
 	} else if _ := ctx.req.header.get(.access_control_request_headers) {
 		// a server must respond with `Access-Control-Allow-Headers` if
 		// `Access-Control-Request-Headers` is present in a preflight request
-		ctx.set_header(.access_control_allow_headers, cors_safelisted_response_headers.join(','))
+		ctx.set_header(.access_control_allow_headers, cors_safelisted_response_headers)
 	}
 
 	if options.allowed_methods.len > 0 {
@@ -260,6 +260,10 @@ pub fn (options &CorsOptions) validate_request(mut ctx Context) bool {
 	ctx.set_header(.access_control_allow_origin, origin)
 	ctx.set_header(.vary, 'Origin, Access-Control-Request-Headers')
 
+	if options.allow_credentials {
+		ctx.set_header(.access_control_allow_credentials, 'true')
+	}
+
 	// validate request method
 	if ctx.req.method !in options.allowed_methods {
 		ctx.res.set_status(.method_not_allowed)
@@ -305,18 +309,12 @@ pub fn (options &CorsOptions) validate_request(mut ctx Context) bool {
 pub fn cors[T](options CorsOptions) MiddlewareOptions[T] {
 	return MiddlewareOptions[T]{
 		handler: fn [options] [T](mut ctx T) bool {
-			if ctx.req.method == .options {
-				// preflight request
+			if ctx.req.method == .options { // preflight
 				options.set_headers(mut ctx.Context)
 				ctx.text('ok')
 				return false
 			} else {
-				// check if there is a cross-origin request
-				if options.validate_request(mut ctx.Context) == false {
-					return false
-				}
-				// no cross-origin request / valid cross-origin request
-				return true
+				return options.validate_request(mut ctx.Context)
 			}
 		}
 	}

vlib/veb/tests/cors_regression_test.v

@@ -0,0 +1,159 @@
+import veb
+import net.http
+import time
+import os
+
+const base_port = 13013
+const exit_after = time.second * 10
+const allowed_origin = 'https://vlang.io'
+
+fn get_port_and_url(test_number int) (int, string) {
+	p := base_port + test_number
+	return p, 'http://localhost:${p}'
+}
+
+pub struct Context {
+	veb.Context
+}
+
+pub struct App {
+	veb.Middleware[Context]
+mut:
+	started chan bool
+}
+
+pub fn (mut app App) before_accept_loop() {
+	app.started <- true
+}
+
+pub fn (app &App) index(mut ctx Context) veb.Result {
+	return ctx.text('index')
+}
+
+fn setup(port int, o veb.CorsOptions) ! {
+	os.chdir(os.dir(@FILE))!
+	go fn () {
+		time.sleep(exit_after)
+		assert false, 'timeout reached!'
+		exit(1)
+	}()
+
+	mut app := &App{}
+	app.use(veb.cors[Context](o))
+
+	go veb.run_at[App, Context](mut app, port: port, timeout_in_seconds: 2)
+	// app startup time
+	_ := <-app.started
+}
+
+fn test_no_user_provided_allowed_headers() {
+	port, localserver := get_port_and_url(1)
+	setup(port, veb.CorsOptions{
+		origins: [allowed_origin]
+	})!
+
+	x := http.fetch(http.FetchConfig{
+		url:    localserver
+		method: http.Method.options
+		header: http.new_header_from_map({
+			http.CommonHeader.origin: allowed_origin
+		})
+	})!
+
+	assert x.status() == http.Status.ok
+	if header := x.header.get(.access_control_allow_headers) {
+		assert false, 'Header should not be set'
+	}
+}
+
+fn test_user_provided_allowed_header() {
+	port, localserver := get_port_and_url(2)
+	setup(port, veb.CorsOptions{
+		origins:         [allowed_origin]
+		allowed_headers: ['content-type']
+	})!
+
+	x := http.fetch(http.FetchConfig{
+		url:    localserver
+		method: http.Method.options
+		header: http.new_header_from_map({
+			http.CommonHeader.origin: allowed_origin
+		})
+	})!
+
+	assert x.status() == http.Status.ok
+	if header := x.header.get(.access_control_allow_headers) {
+		assert header == 'content-type'
+	} else {
+		assert false, 'Header not set'
+	}
+}
+
+fn test_user_provided_allowed_header_wildcard() {
+	port, localserver := get_port_and_url(3)
+	setup(port, veb.CorsOptions{
+		origins:         [allowed_origin]
+		allowed_headers: ['*']
+	})!
+
+	x := http.fetch(http.FetchConfig{
+		url:    localserver
+		method: http.Method.options
+		header: http.new_header_from_map({
+			http.CommonHeader.origin: allowed_origin
+		})
+	})!
+
+	assert x.status() == http.Status.ok
+	if header := x.header.get(.access_control_allow_headers) {
+		assert header == '*'
+	} else {
+		assert false, 'Header not set'
+	}
+}
+
+fn test_request_has_access_control_request_headers() {
+	port, localserver := get_port_and_url(4)
+	setup(port, veb.CorsOptions{
+		origins: [allowed_origin]
+	})!
+
+	x := http.fetch(http.FetchConfig{
+		url:    localserver
+		method: http.Method.options
+		header: http.new_header_from_map({
+			http.CommonHeader.origin:                         allowed_origin
+			http.CommonHeader.access_control_request_headers: 'any-value'
+		})
+	})!
+
+	assert x.status() == http.Status.ok
+	if header := x.header.get(http.CommonHeader.access_control_allow_headers) {
+		assert header == veb.cors_safelisted_response_headers
+	} else {
+		assert false, 'Header not set'
+	}
+}
+
+fn test_allow_credentials_non_preflight() {
+	port, localserver := get_port_and_url(5)
+	setup(port, veb.CorsOptions{
+		origins:           [allowed_origin]
+		allowed_methods:   [http.Method.get]
+		allow_credentials: true
+	})!
+
+	x := http.fetch(http.FetchConfig{
+		url:    localserver
+		header: http.new_header_from_map({
+			http.CommonHeader.origin: allowed_origin
+		})
+	})!
+
+	assert x.status() == http.Status.ok
+	if header := x.header.get(http.CommonHeader.access_control_allow_credentials) {
+		assert header == 'true'
+	} else {
+		assert false, 'Header not set'
+	}
+}