cors icon indicating copy to clipboard operation
cors copied to clipboard

Allowing methods that are not uppercase should be possible but isn't

Open jub0bs opened this issue 2 years ago • 1 comments

Although method names are case-sensitive, go-chi/cors takes the unusual approach of normalising method names by uppercasing them:

// Spec says: Since the list of methods can be unbounded, simply returning the method indicated
// by Access-Control-Request-Method (if supported) can be enough

Such unwarranted case normalisation causes problems for clients that send requests whose method is not uppercase—and not some case-insensitive match for one of DELETE, GET, HEAD, OPTIONS, POST, or PUT, names for which the Fetch standard carves out an exception. Here is an example:

c := cors.New(cors.Options{
  AllowedOrigins: []string{"https://example.com"},
  AllowedMethods: []string{"patch"},
})

Assume then that a client running in the context of Web origin https://example.com in a Fetch-compliant browser sends a patch request (note the lower case) to the server. As go-chi/cors internally normalises method names to uppercase, the preflight response would contain

Access-Control-Allow-Methods: PATCH

as opposed to

Access-Control-Allow-Methods: patch

Browsers rule this as a mismatch and fail CORS preflight with an error message of this kind:

Access to fetch at [REDACTED] from origin ‘https://example.com’ has been blocked by CORS policy: Method patch is not allowed by Access-Control-Allow-Methods in preflight response.

For this reason, go-chi/cors should not normalise method names. More about this in one of my recent blog posts.

jub0bs avatar Dec 15 '23 21:12 jub0bs

Here is a (failing) test case that illustrates the issue:

func TestHandlePreflightLowercaseAllowedMethod(t *testing.T) {
	const (
		origin = "https://foo.com"
		method = "patch"
	)
	s := New(Options{
		AllowedOrigins: []string{origin},
		AllowedMethods: []string{method},
	})
	res := httptest.NewRecorder()
	req, _ := http.NewRequest(http.MethodOptions, "http://example.com/foo", nil)
	req.Header.Add("Origin", origin)
	req.Header.Add("Access-Control-Request-Method", method)

	s.handlePreflight(res, req)

	assertHeaders(t, res.Result().Header, map[string]string{
		"Vary":                         "Origin, Access-Control-Request-Method, Access-Control-Request-Headers",
		"Access-Control-Allow-Origin":  origin,
		"Access-Control-Allow-Methods": method,
	})
}

Result:

$ go test -run ^TestHandlePreflightLowercaseAllowedMethod$ github.com/go-chi/cors 
--- FAIL: TestHandlePreflightLowercaseAllowedMethod (0.00s)
    cors_test.go:30: Response header "Access-Control-Allow-Methods" = "PATCH", want "patch"
FAIL
FAIL    github.com/go-chi/cors  0.277s
FAIL

jub0bs avatar Dec 17 '23 08:12 jub0bs