-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Closed
Labels
Description
Go 1.7 adds native support for context.Context by adding Context and WithContext methods to http.Request.
The expectation is that, if you want to add anything to the context of the request, you use http.Request.WithContext, which returns a shallow copy of the request that you would pass down. Unfortunately, this seems break the way gorilla mux stores vars.
Here's a simple Go program that demonstrates the problem:
package main
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"time"
"github.com/gorilla/mux"
)
func withTimeout(h http.Handler, timeout time.Duration) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ctx, cancel := context.WithTimeout(r.Context(), timeout)
defer cancel()
r = r.WithContext(ctx)
h.ServeHTTP(w, r)
})
}
func ping(w http.ResponseWriter, r *http.Request) {
fmt.Println(mux.Vars(r))
}
func main() {
r := mux.NewRouter()
r.Handle("/broken/{var}", withTimeout(http.HandlerFunc(ping), time.Second))
r.Handle("/ok/{var}", http.HandlerFunc(ping))
resp := httptest.NewRecorder()
req, _ := http.NewRequest("GET", "/ok/foo", nil)
r.ServeHTTP(resp, req)
// Prints:
// map[var:foo]
resp = httptest.NewRecorder()
req, _ = http.NewRequest("GET", "/broken/foo", nil)
r.ServeHTTP(resp, req)
// Prints:
// map[]
}Once Go 1.7 is out, basically anything that uses middleware to set context values, cancellations or deadlines is going to break mux.
It would be ideal of the vars were actually stored in the context.Context, which may be possible without breaking the API?
Reactions are currently unavailable