Skip to content

Commit

Permalink
Guess the scheme if r.URL.Scheme is unset
Browse files Browse the repository at this point in the history
It's not expected that the request's URL is fully populated when used on
the server-side (it's more of a client-side field), so we shouldn't
expect it to be present.

In practice, it's only rarely set at all on the server, making mux's
`Schemes` matcher tricky to use as it is.

This commit adds a test which would have failed before demonstrating the
problem, as well as a fix which I think makes `.Schemes` match what
users expect.
  • Loading branch information
euank committed May 16, 2019
1 parent c5c6c98 commit 16409f3
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 5 deletions.
46 changes: 46 additions & 0 deletions mux_httpserver_test.go
@@ -0,0 +1,46 @@
// +build go1.9

package mux

import (
"bytes"
"io/ioutil"
"net/http"
"net/http/httptest"
"testing"
)

func TestSchemeMatchers(t *testing.T) {
router := NewRouter()
router.HandleFunc("/", func(rw http.ResponseWriter, r *http.Request) {
rw.Write([]byte("hello world"))
}).Schemes("http", "https")

assertHelloWorldResponse := func(t *testing.T, s *httptest.Server) {
resp, err := s.Client().Get(s.URL)
if err != nil {
t.Fatalf("unexpected error getting from server: %v", err)
}
if resp.StatusCode != 200 {
t.Fatalf("expected a status code of 200, got %v", resp.StatusCode)
}
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
t.Fatalf("unexpected error reading body: %v", err)
}
if !bytes.Equal(body, []byte("hello world")) {
t.Fatalf("response should be hello world, was: %q", string(body))
}
}

t.Run("httpServer", func(t *testing.T) {
s := httptest.NewServer(router)
defer s.Close()
assertHelloWorldResponse(t, s)
})
t.Run("httpsServer", func(t *testing.T) {
s := httptest.NewTLSServer(router)
defer s.Close()
assertHelloWorldResponse(t, s)
})
}
6 changes: 2 additions & 4 deletions mux_test.go
Expand Up @@ -10,6 +10,7 @@ import (
"errors"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"reflect"
"strings"
Expand Down Expand Up @@ -2837,10 +2838,7 @@ func newRequestWithHeaders(method, url string, headers ...string) *http.Request

// newRequestHost a new request with a method, url, and host header
func newRequestHost(method, url, host string) *http.Request {
req, err := http.NewRequest(method, url, nil)
if err != nil {
panic(err)
}
req := httptest.NewRequest(method, url, nil)
req.Host = host
return req
}
15 changes: 14 additions & 1 deletion route.go
Expand Up @@ -412,7 +412,20 @@ func (r *Route) Queries(pairs ...string) *Route {
type schemeMatcher []string

func (m schemeMatcher) Match(r *http.Request, match *RouteMatch) bool {
return matchInArray(m, r.URL.Scheme)
scheme := r.URL.Scheme
// https://golang.org/pkg/net/http/#Request
// "For [most] server requests, fields other than Path and RawQuery will be
// empty."
// Since we're an http muxer, the scheme is either going to be http or https
// though, so we can just set it based on the tls termination state.
if scheme == "" {
if r.TLS == nil {
scheme = "http"
} else {
scheme = "http"
}
}
return matchInArray(m, scheme)
}

// Schemes adds a matcher for URL schemes.
Expand Down

0 comments on commit 16409f3

Please sign in to comment.