Skip to content

Commit

Permalink
Fix: Forbid empty path params: /a/{p}/b does not handle requests fo…
Browse files Browse the repository at this point in the history
…r `/a//b`
  • Loading branch information
afflerbach committed Jun 23, 2022
1 parent 237a022 commit 6bd5b55
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 8 deletions.
27 changes: 19 additions & 8 deletions server/mux.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ type Mux struct {

const (
serverOptionsKey = "serverContextOptions"
wildcardReplacement = "/{_couper_wildcardMatch*}"
wildcardVariable = "_couper_wildcardMatch"
wildcardReplacement = "/{" + wildcardVariable + "*}"
wildcardSearch = "/**"
)

Expand Down Expand Up @@ -89,7 +90,7 @@ func (m *Mux) mustAddRoute(root *pathpattern.Node, path string, handler http.Han
func (m *Mux) FindHandler(req *http.Request) http.Handler {
var route *routers.Route

node, paramValues := m.match(m.endpointRoot, req)
node, paramValues := m.matchWithMethod(m.endpointRoot, req)
if node == nil {
node, paramValues = m.matchWithoutMethod(m.endpointRoot, req)
}
Expand Down Expand Up @@ -130,10 +131,9 @@ func (m *Mux) FindHandler(req *http.Request) http.Handler {

ctx := req.Context()

const wcm = "_couper_wildcardMatch"
if wildcardMatch, ok := pathParams[wcm]; ok {
if wildcardMatch, ok := pathParams[wildcardVariable]; ok {
ctx = context.WithValue(ctx, request.Wildcard, wildcardMatch)
delete(pathParams, wcm)
delete(pathParams, wildcardVariable)
}

ctx = context.WithValue(ctx, request.PathParams, pathParams)
Expand All @@ -142,16 +142,27 @@ func (m *Mux) FindHandler(req *http.Request) http.Handler {
return m.handler[route]
}

func (m *Mux) match(root *pathpattern.Node, req *http.Request) (*pathpattern.Node, []string) {
func (m *Mux) matchWithMethod(root *pathpattern.Node, req *http.Request) (*pathpattern.Node, []string) {
*req = *req.WithContext(context.WithValue(req.Context(), request.ServerName, m.opts.ServerOptions.ServerName))

return root.Match(req.Method + " " + req.URL.Path)
return m.match(root, req.Method+" "+req.URL.Path)
}

func (m *Mux) matchWithoutMethod(root *pathpattern.Node, req *http.Request) (*pathpattern.Node, []string) {
*req = *req.WithContext(context.WithValue(req.Context(), request.ServerName, m.opts.ServerOptions.ServerName))

return root.Match(req.URL.Path)
return m.match(root, req.URL.Path)
}

func (m *Mux) match(root *pathpattern.Node, requestLine string) (*pathpattern.Node, []string) {
node, values := root.Match(requestLine)
for i, value := range values {
if value == "" && node.VariableNames[i] != wildcardVariable+"*" {
// Path params must not be empty.
return nil, nil
}
}
return node, values
}

func (m *Mux) hasFileResponse(req *http.Request) (http.Handler, bool) {
Expand Down
20 changes: 20 additions & 0 deletions server/mux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func TestMux_FindHandler_PathParamContext(t *testing.T) {
"/{b}/a": noContent,
"/{section}/{project}": noContent,
"/project/{project}/**": noContent,
"/w/c/{param}/**": noContent,
},
FileRoutes: map[string]http.Handler{
"/htdocs/test.html": noContent,
Expand Down Expand Up @@ -59,13 +60,18 @@ func TestMux_FindHandler_PathParamContext(t *testing.T) {
{" /w 1st path param #2", newReq("/c/a"), noContent, request.PathParameter{
"b": "c",
}, ""},
{" w/o 1nd path param", newReq("//a"), http.NotFoundHandler(), nil, ""},
{" /w 2nd path param", newReq("/a/c"), noContent, request.PathParameter{
"b": "c",
}, ""},
{" w/o 2nd path param", newReq("/a"), http.NotFoundHandler(), nil, ""},
{" w/o 2nd path param", newReq("/a/"), http.NotFoundHandler(), nil, ""},
{" w/o 2nd path param", newReq("/a//"), http.NotFoundHandler(), nil, ""},
{" /w two path param", newReq("/foo/bar"), noContent, request.PathParameter{
"section": "foo",
"project": "bar",
}, ""},
{" w/o two path params", newReq("//"), http.NotFoundHandler(), nil, ""},
{" /w path param and expWildcard", newReq("/project/foo/bar/ha"), noContent, request.PathParameter{
"project": "foo",
}, "bar/ha"},
Expand All @@ -74,6 +80,20 @@ func TestMux_FindHandler_PathParamContext(t *testing.T) {
"section": "htdocs",
"project": "test.html",
}, ""},
{" w/ path param and wildcard", newReq("/w/c/my-param/wild/ca/rd"), noContent, request.PathParameter{
"param": "my-param",
}, "wild/ca/rd"},
{" w/ path param, w/o wildcard", newReq("/w/c/my-param"), noContent, request.PathParameter{
"param": "my-param",
}, ""},
{" w/ path param, trailing /, w/o wildcard", newReq("/w/c/my-param/"), noContent, request.PathParameter{
"param": "my-param",
}, ""},
{" w/o path param, w/ wildcard", newReq("/w/c//wild/card"), http.NotFoundHandler(), nil, ""},
{" w/o path param, w/o wildcard", newReq("/w/c"), http.NotFoundHandler(), nil, ""},
{" w/o path param, w/o wildcard", newReq("/w/c/"), http.NotFoundHandler(), nil, ""},
{" w/o path param, w/o wildcard", newReq("/w/c//"), http.NotFoundHandler(), nil, ""},
{" w/o path param, w/o wildcard", newReq("/w/c///"), http.NotFoundHandler(), nil, ""},
}
for _, tt := range tests {
t.Run(tt.name, func(subT *testing.T) {
Expand Down

0 comments on commit 6bd5b55

Please sign in to comment.