Merge pull request #4 from aquasecurity/ido-route-overlap

[fix #3] overlapping routes may not be matched

lmdrouter.go

@@ -269,6 +269,11 @@ func (l *Router) matchRequest(req *events.APIGatewayProxyRequest) (
 	// remove trailing slash from request path
 	req.Path = strings.TrimSuffix(req.Path, "/")
 
+	negErr := HTTPError{
+		Code:    http.StatusNotFound,
+		Message: "No such resource",
+	}
+
 	// find a route that matches the request
 	for _, r := range l.routes {
 		// does the path match?
@@ -281,10 +286,13 @@ func (l *Router) matchRequest(req *events.APIGatewayProxyRequest) (
 		var ok bool
 		rsrc, ok = r.methods[req.HTTPMethod]
 		if !ok {
-			return rsrc, HTTPError{
+			// we matched a route, but it didn't support this method. Mark negErr
+			// with a 405 error, but continue, we might match another route
+			negErr = HTTPError{
 				Code:    http.StatusMethodNotAllowed,
 				Message: fmt.Sprintf("%s requests not supported by this resource", req.HTTPMethod),
 			}
+			continue
 		}
 
 		// process path parameters
@@ -306,8 +314,5 @@ func (l *Router) matchRequest(req *events.APIGatewayProxyRequest) (
 		return rsrc, nil
 	}
 
-	return rsrc, HTTPError{
-		Code:    http.StatusNotFound,
-		Message: "No such resource",
-	}
+	return rsrc, negErr
 }

lmdrouter_test.go

@@ -159,6 +159,41 @@ func TestRouter(t *testing.T) {
 			assert.Equal(t, "[INF] [GET /api] [200]", log[len(log)-1], "Last long line must be correct")
 		})
 	})
+
+	t.Run("Overlapping routes", func(t *testing.T) {
+		router := NewRouter("")
+		router.Route("GET", "/foo/:id", func(_ context.Context, _ events.APIGatewayProxyRequest) (res events.APIGatewayProxyResponse, err error) {
+			res.Body = "/foo/:id"
+			return res, nil
+		})
+		router.Route("POST", "/foo/bar", func(_ context.Context, _ events.APIGatewayProxyRequest) (res events.APIGatewayProxyResponse, err error) {
+			res.Body = "/foo/bar"
+			return res, nil
+		})
+
+		// call POST /foo/bar in a loop. We do this because the router iterates
+		// over a map to match routes, which is non-deterministic, meaning
+		// sometimes we may match the route and sometimes not
+		for i := 1; i <= 10; i++ {
+			res, _ := router.Handler(context.Background(), events.APIGatewayProxyRequest{
+				HTTPMethod: "POST",
+				Path:       "/foo/bar",
+			})
+			assert.Equal(t, "/foo/bar", res.Body, "request must match /foo/bar route")
+		}
+
+		res, _ := router.Handler(context.Background(), events.APIGatewayProxyRequest{
+			HTTPMethod: "DELETE",
+			Path:       "/foo/bar",
+		})
+		assert.Equal(t, http.StatusMethodNotAllowed, res.StatusCode, "Status code must be 405")
+
+		res, _ = router.Handler(context.Background(), events.APIGatewayProxyRequest{
+			HTTPMethod: "GET",
+			Path:       "/foo/bar2",
+		})
+		assert.Equal(t, "/foo/:id", res.Body, "Body must match")
+	})
 }
 
 func listSomethings(ctx context.Context, req events.APIGatewayProxyRequest) (