Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Access-Control-Allowed-Methods with mux.Route.Methods() #353

Closed
fharding1 opened this issue Mar 12, 2018 · 6 comments
Closed

Add Access-Control-Allowed-Methods with mux.Route.Methods() #353

fharding1 opened this issue Mar 12, 2018 · 6 comments

Comments

@fharding1
Copy link
Contributor

fharding1 commented Mar 12, 2018

What version of Go are you running? (Paste the output of go version)
go version go1.9.3 linux/amd64

What version of gorilla/mux are you at? (Paste the output of git rev-parse HEAD inside $GOPATH/src/github.com/gorilla/mux)
c0091a0

Describe your problem (and what you have tried so far)
When using mux.Route.Methods() to set allowed methods, there is no easy way to add CORS. I had a middleware for this, but you run into problems when you have two of "the same" URLs, for example /reports/{eventKey}/{matchKey} (PUT) and /reports/{eventKey}/{team} (GET). It would be nice if there was a simple way to integrate CORS with .Methods(), or if there is one already if it were more clear in the documentation.

r.Handle("/reports/{eventKey}/{matchKey}", submitReport).Methods("PUT")
r.Handle("/reports/{eventKey}/{team}", getReportsByEventAndTeam).Methods("GET")

An OPTIONS request or request to either endpoint should have Access-Control-Allow-Methods: "GET", "PUT", "OPTIONS".

Paste a minimal, runnable, reproduction of your issue below (use backticks to format it)
N/A

@fharding1 fharding1 changed the title Add Access-Control-Allowed-Methods with .Methods() Add Access-Control-Allowed-Methods with mux.Route.Methods() Mar 12, 2018
@elithrar elithrar self-assigned this Mar 12, 2018
@elithrar
Copy link
Contributor

You could implement this as middleware by calling route.GetMethods() and then setting the response header. Does that not achieve your need to dynamically set the header?

e.g.

func AutoCORS(next http.Handler) http.Handler {
	fn := func(w http.ResponseWriter, r *http.Request) {
		route := mux.CurrentRoute(r)
		methods, err := route.GetMethods()
		if err != nil {
			http.Error(w, err.Error(), http.StatusInternalServerError)
			return
		}

		w.Header().Set("Access-Control-Allow-Methods", strings.Join(methods,
			","))
		next.ServeHTTP(w, r)
	}

	return http.HandlerFunc(fn)
}

@fharding1
Copy link
Contributor Author

@elithrar Thank you for the response! That somewhat works I think, but OPTIONS requests still don't work right. Because the two URL's are the same format, an OPTIONS request to /reports/{eventKey}/{matchKey} or /reports/{eventKey}/{team} returns either PUT, OPTIONS or GET, OPTIONS (assuming both .Methods() include OPTIONS) when they should return GET, PUT, OPTIONS. Is there any way around this? To accumulate all methods on "the same" format URL's?

@fharding1
Copy link
Contributor Author

bump

@elithrar
Copy link
Contributor

@fharding1

Thinking on this more:

  • You'd probably need to build your own map of routes via Router.Walk
  • With that in hand, you'd likely want to match on prefix and then use middleware to handle OPTIONS requests

I'd be open to a PR that can do this as a feature on Router.

@fharding1
Copy link
Contributor Author

@elithrar I would be into trying to get that working myself.

@elithrar
Copy link
Contributor

That'd be great @fharding1 - if you can get a PR up, I'll do my best to be more responsive to review!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants