CORS: OPTIONS call not handled

What version of Go are you running?
go version go1.10.2 darwin/amd64

What version of gorilla/mux are you at?
c856192

Describe your problem

Create a router
Register a route with .Methods("GET")
Make the router use the new CORSMiddleware
Call OPTIONS on the route
Expect a 200 OK with the Access-Control-Allowed-Methods set to GET,OPTIONS; get instead a 405.

The code that is supposed to handle OPTIONS requests (below) is never reached, unless OPTIONS is a registered method for the route.

mux/middleware.go

Lines 64 to 66 in c856192

    
           if req.Method == "OPTIONS" { 
        
           	return 
        
           }

Paste a minimal, runnable, reproduction of your issue below

Add this test to middleware_test.go

func TestCORSMiddlewareOPTIONS(t *testing.T) {
	router := NewRouter()

	cases := []struct {
		path                   string
		response               string
		method                 string
		testURL                string
		expectedAllowedMethods string
	}{
		{"/g/{o}", "a", "POST", "/g/asdf", "POST,PUT,GET,OPTIONS"},
		{"/g/{o}", "b", "PUT", "/g/bla", "POST,PUT,GET,OPTIONS"},
		{"/g/{o}", "c", "GET", "/g/orilla", "POST,PUT,GET,OPTIONS"},
		{"/g", "d", "POST", "/g", "POST,OPTIONS"},
	}

	for _, tt := range cases {
		router.HandleFunc(tt.path, stringHandler(tt.response)).Methods(tt.method)
	}

	router.Use(CORSMethodMiddleware(router))

	for _, tt := range cases {
		rr := httptest.NewRecorder()
		req := newRequest("OPTIONS", tt.testURL)

		router.ServeHTTP(rr, req)

		allowedMethods := rr.HeaderMap.Get("Access-Control-Allow-Methods")

		if want, have := 200, rr.Result().StatusCode; have != want {
			t.Errorf("Expected status code %d, found %d", want, have)
		}

		if allowedMethods != tt.expectedAllowedMethods {
			t.Errorf("Expected Access-Control-Allow-Methods '%s', found '%s'", tt.expectedAllowedMethods, allowedMethods)
		}
	}
}

Note that this new test passes if OPTIONS is explicitly set as a method in the test-table:

		{"/g/{o}", "a", "POST", "/g/asdf", "POST,PUT,GET,OPTIONS"},
		{"/g/{o}", "b", "PUT", "/g/bla", "POST,PUT,GET,OPTIONS"},
		{"/g/{o}", "c", "GET", "/g/orilla", "POST,PUT,GET,OPTIONS"},
		{"/g", "d", "POST", "/g", "POST,OPTIONS"},
		{"/g", "", "OPTIONS", "/g", "POST,OPTIONS"},
		{"/g/{o}", "", "OPTIONS", "/g/orilla", "POST,PUT,GET,OPTIONS"},

I suspect this line to be the problem:

mux/mux.go

Line 88 in c856192

if match.MatchErr == nil {

In Router#Match, the middlewares are only triggered if the method matches a route.

@pierreprinetti - we're looking at expanding middleware to run even without a match, as per #378

If you'd like to give that PR a cursory review/feedback, I'd appreciate. Currently traveling and may not have time to address for a few weeks.

@elithrar Thanks for the fast response!

Just one thing:

With the current code, setting "OPTIONS" explicitly as a verb when registering a route will have the effect of allowing the CORSMethodMiddleware to handle it.

I think it's a totally valid solution! If this is the intended behaviour? If it is, then maybe we could add a short line in the docs about it.

Let’s definitely document that against the middleware. There are cases where you may not want to expose routes via CORS, and that seems like a sensible compromise without enabling too much magic.

…

On Thu, May 24, 2018 at 8:54 AM Pierre Prinetti ***@***.***> wrote: @elithrar <https://github.com/elithrar> Thanks for the fast response! Just one thing: With the current code, setting "OPTIONS" explicitly as a verb when registering a route will have the effect of allowing the CORSMethodMiddleware to handle it. I think it's a totally valid solution! If this is the intended behaviour? If it is, then maybe we could add a short line in the docs about it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#381 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABIcCSAERMIqCzSVlg9-ES5Toi7e7jvks5t1mcbgaJpZM4UKzLf> .

Sorry what is the status of this? I am trying to get my code working with CORS

I can't see how to use the CORSMethodMiddleware method

Somehow I get this error, even if I have the latest release code. Hmmm

Oh I changed my code to

r := mux.NewRouter()
mux.CORSMethodMiddleware(r)
r.HandleFunc("/test", testFunc)
http.Handle("/", r)

Is it how to use it ? Because I'm still facing errors on OPTIONS request :/

Can you post a reproducible example, and include what your requests are, and what errors are returned?

I m using Google App Engine to host my service.
Sending my payload through POSTMAN doesn't trigger the OPTIONS method, hence it goes straight to POST and is successful.

However, utimately the API is to be trigerred from a Angular6 frontend, and Chrome (or Safari) are sending those POST request first, the POST and the JSON payload don't even reach the server as the OPTIONS call fails.

Here is the trace from the Chrome debug tools

Here is how I setup the router in my Go file.

Did I miss one step in the configuration here ?

Yes: you need to explicitly allow the origin by adding an Access-Control-Allow-Origin header, as per the error message. localhost is not a valid origin.

I recommend taking a look at https://github.com/rs/cors

But isnt the whole purpose of the CORS middleware func ,mentioned above, to add on those requires headers and handle OPTIONS method ? What puzzles me is that even when the request goes through that middle function , it dooesnt seem to set the proper headers. Le lun. 3 sept. 2018 à 12:32, Matt Silverlock <notifications@github.com> a écrit :

…

Yes: you need to explicitly allow the origin by adding an Access-Control-Allow-Origin header, as per the error message. localhost is not a valid origin. I recommend taking a look at https://github.com/rs/cors — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#381 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJj5aeUvvthjmaJ5Vr0eSLeWXEfqjYhcks5uXLDSgaJpZM4UKzLf> .

The middleware here is designed to dynamically handle the Methods you add to a route, which other middleware can't easily do without integrating into mux.

Your application doesn't respond with the ACAM header unless you explicitly ask the route to handle OPTIONS requests.

(Closing this as we generally agreed on leaving as-is; happy to answer questions but best to open a new issue in future)