Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[bug] CORSMethodMiddleware can add the allowed methods of the wrong routes #534

Closed
stephenpaulger opened this issue Nov 14, 2019 · 3 comments · Fixed by #535
Closed

[bug] CORSMethodMiddleware can add the allowed methods of the wrong routes #534

stephenpaulger opened this issue Nov 14, 2019 · 3 comments · Fixed by #535
Labels

Comments

@stephenpaulger
Copy link

Describe the bug
When using CORSMethodMiddleware in a Subrouter it can add allowed methods from other routes that shouldn't be present.

The issue seems to be that in getAllMethodsForRoute the matcher matches substrings.

Versions

Go version: go version

% go version
go version go1.12.1 darwin/amd64

package version: run git rev-parse HEAD inside the repo

Sorry, I'm not using the repo of mux, but my go.mod has

github.com/gorilla/mux v1.7.3

Steps to Reproduce

How can the bug be triggered?

Create a PathPrefix with a sub router, add two routes with paths where one is a substring of the other. Eg. /hello and /hello/name. Add different allowed methods to these routes.

You can see the allowed methods of both routes when requesting the route with the longer path.

Using the code example below the response header for /test/hello/name looks like

Access-Control-Allow-Methods: GET,OPTIONS,POST,GET,OPTIONS

Expected behavior

What output or behaviour were you expecting instead?

I would expect to see only the allowed methods of the route in the Access-Control-Allow-Methods which would be Access-Control-Allow-Methods: GET,OPTIONS

Code Snippets

A minimum viable code snippet can be useful! (use backticks to format it).

package main

import (
	"fmt"
	"net/http"

	"github.com/gorilla/mux"
)

func Hello(w http.ResponseWriter, r *http.Request) {
	fmt.Fprint(w, "Hello")
}

func HelloName(w http.ResponseWriter, r *http.Request) {
	fmt.Fprint(w, "Hello")
}

func main() {
	router := mux.NewRouter().StrictSlash(true)

	subrouter := router.PathPrefix("/test").Subrouter()
	subrouter.HandleFunc("/hello", Hello).Methods(http.MethodGet, http.MethodOptions, http.MethodPost)
	subrouter.HandleFunc("/hello/{name}", HelloName).Methods(http.MethodGet, http.MethodOptions)

	subrouter.Use(mux.CORSMethodMiddleware(subrouter))

	http.ListenAndServe(":8081", router)
}
@fharding1
Copy link
Contributor

@stephenpaulger thanks, this definitely looks like the wrong behavior. Looking into it 👍

fharding1 added a commit to fharding1/mux that referenced this issue Nov 14, 2019
* Adds a test case for the repro given in issue gorilla#534
* Fixes the logic in CORSMethodMiddleware to handle matching routes
better
@stephenpaulger
Copy link
Author

@fharding1 thanks for jumping on that so fast.

I find your new version of getAllMethodsForRoute easier to read too.

fharding1 added a commit to fharding1/mux that referenced this issue Nov 15, 2019
* Adds a test case for the repro given in issue gorilla#534
* Fixes the logic in CORSMethodMiddleware to handle matching routes
better
@stephenpaulger
Copy link
Author

I notice this is fixed in 1.7.4, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants