diff --git a/middleware/router.go b/middleware/router.go index e828653b..c0b093b2 100644 --- a/middleware/router.go +++ b/middleware/router.go @@ -357,7 +357,7 @@ func (d *defaultRouter) Lookup(method, path string) (*MatchedRoute, bool) { } } if router, ok := d.routers[mth]; ok { - if m, rp, ok := router.Lookup(fpath.Clean(path)); ok && m != nil { + if m, rp, ok := router.Lookup(fpath.Clean(escapeLiteralColons(path))); ok && m != nil { if entry, ok := m.(*routeEntry); ok { d.debugLogf("found a route for %s %s with %d parameters", method, path, len(entry.Parameters)) var params RouteParams @@ -398,7 +398,7 @@ func (d *defaultRouter) OtherMethods(method, path string) []string { var methods []string for k, v := range d.routers { if k != mn { - if _, _, ok := v.Lookup(fpath.Clean(path)); ok { + if _, _, ok := v.Lookup(fpath.Clean(escapeLiteralColons(path))); ok { methods = append(methods, k) continue } @@ -414,6 +414,14 @@ func (d *defaultRouter) SetLogger(lg logger.Logger) { // convert swagger parameters per path segment into a denco parameter as multiple parameters per segment are not supported in denco. var pathConverter = regexp.MustCompile(`{(.+?)}([^/]*)`) +// escapeLiteralColons replaces literal ':' characters with their URL-encoded +// equivalent "%3A". This prevents the denco router from misinterpreting ':' +// in URL path segments as parameter delimiters. The ':' character is valid in +// URL paths per RFC 3986 section 3.3. +func escapeLiteralColons(path string) string { + return strings.ReplaceAll(path, ":", "%3A") +} + func decodeCompositParams(name string, value string, pattern string, names []string, values []string) ([]string, []string) { pleft := strings.Index(pattern, "{") names = append(names, name) @@ -463,7 +471,7 @@ func (d *defaultRouteBuilder) AddRoute(method, path string, operation *spec.Oper requestBinder := NewUntypedRequestBinder(parameters, d.spec.Spec(), d.api.Formats()) requestBinder.setDebugLogf(d.debugLogf) - record := denco.NewRecord(pathConverter.ReplaceAllString(path, ":$1"), &routeEntry{ + record := denco.NewRecord(pathConverter.ReplaceAllString(escapeLiteralColons(path), ":$1"), &routeEntry{ BasePath: bp, PathPattern: path, Operation: operation, diff --git a/middleware/router_test.go b/middleware/router_test.go index 2abcb1ec..cfcb12df 100644 --- a/middleware/router_test.go +++ b/middleware/router_test.go @@ -14,6 +14,7 @@ import ( "github.com/go-openapi/analysis" "github.com/go-openapi/loads" "github.com/go-openapi/runtime/internal/testing/petstore" + "github.com/go-openapi/runtime/middleware/denco" "github.com/go-openapi/runtime/middleware/untyped" "github.com/go-openapi/testify/v2/assert" "github.com/go-openapi/testify/v2/require" @@ -234,6 +235,96 @@ func TestPathConverter(t *testing.T) { } } +func TestEscapeLiteralColons(t *testing.T) { + cases := []struct { + input string + expected string + }{ + {"/", "/"}, + {"/something", "/something"}, + {"/allow/{serverName}/tokenlist:add", "/allow/{serverName}/tokenlist%3Aadd"}, + {"/path:with:colons", "/path%3Awith%3Acolons"}, + {"/{id}:{name}", "/{id}%3A{name}"}, + {"/action:do/{id}", "/action%3Ado/{id}"}, + {"/normal/path", "/normal/path"}, + } + + for _, tc := range cases { + actual := escapeLiteralColons(tc.input) + assert.EqualT(t, tc.expected, actual, "expected escapeLiteralColons(%s) to be %s but got %s", tc.input, tc.expected, actual) + } +} + +func TestPathConverterWithLiteralColons(t *testing.T) { + // Verify the full pipeline: escapeLiteralColons then pathConverter + cases := []struct { + swagger string + denco string + }{ + // The main use case from issue #352 + {"/allow/{serverName}/tokenlist:add", "/allow/:serverName/tokenlist%3Aadd"}, + // Literal colons in static segments + {"/action:do/{id}", "/action%3Ado/:id"}, + {"/path:with:colons", "/path%3Awith%3Acolons"}, + // Multiple colons in different segments + {"/api:v1/items/{id}", "/api%3Av1/items/:id"}, + } + + for _, tc := range cases { + actual := pathConverter.ReplaceAllString(escapeLiteralColons(tc.swagger), ":$1") + assert.EqualT(t, tc.denco, actual, "expected swagger path %s to produce denco path %s but got %s", tc.swagger, tc.denco, actual) + } +} + +func TestDencoRouterWithLiteralColons(t *testing.T) { + // Test that the denco router correctly handles paths with literal colons + // when the colons are URL-encoded as %3A. + t.Run("static path with encoded colons", func(t *testing.T) { + router := denco.New() + err := router.Build([]denco.Record{ + denco.NewRecord("/path%3Awith%3Acolons", "static-colon-route"), + }) + require.NoError(t, err) + + data, _, ok := router.Lookup("/path%3Awith%3Acolons") + assert.TrueT(t, ok) + assert.EqualT(t, "static-colon-route", data) + + // Should not match the unescaped version + _, _, ok = router.Lookup("/path:with:colons") + assert.FalseT(t, ok) + }) + + t.Run("parametric path with encoded colons in suffix", func(t *testing.T) { + router := denco.New() + err := router.Build([]denco.Record{ + denco.NewRecord("/allow/:serverName/tokenlist%3Aadd", "param-colon-route"), + }) + require.NoError(t, err) + + data, params, ok := router.Lookup("/allow/myserver/tokenlist%3Aadd") + assert.TrueT(t, ok) + assert.EqualT(t, "param-colon-route", data) + require.Len(t, params, 1) + assert.EqualT(t, "myserver", params[0].Value) + }) + + t.Run("parametric path with encoded colons between params", func(t *testing.T) { + router := denco.New() + err := router.Build([]denco.Record{ + denco.NewRecord("/:id/items%3Acheck/:name", "between-params-route"), + }) + require.NoError(t, err) + + data, params, ok := router.Lookup("/foo/items%3Acheck/bar") + assert.TrueT(t, ok) + assert.EqualT(t, "between-params-route", data) + require.Len(t, params, 2) + assert.EqualT(t, "foo", params[0].Value) + assert.EqualT(t, "bar", params[1].Value) + }) +} + func TestExtractCompositParameters(t *testing.T) { // name is the composite parameter's name, value is the value of this compost parameter, pattern is the pattern to be matched cases := []struct {