Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce MergePathStrategy for #521 #519 #523

Merged
merged 3 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ There are several hooks to customize the behavior of the go-restful package.
- Compression
- Encoders for other serializers
- Use [jsoniter](https://github.com/json-iterator/go) by building this package using a build tag, e.g. `go build -tags=jsoniter .`
- Use the variable `MergePathStrategy` to change the behaviour of composing the Route path given a root path and a local route path
- versions >= 3.10.1 has set the value to `PathJoinStrategy` that fixes a reported [security issue](https://github.com/advisories/GHSA-r48q-9g5r-8q2h) but may cause your services not to work correctly anymore.
- versions <= 3.9 had the behaviour that can be restored in newer versions by setting the value to `TrimSlashStrategy`.
- you can set value to a custom implementation (must implement MergePathStrategyFunc)

## Resources

Expand Down
1 change: 1 addition & 0 deletions filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func tearDown() {
DefaultContainer.webServices = []*WebService{}
DefaultContainer.isRegisteredOnRoot = true // this allows for setupServices multiple times
DefaultContainer.containerFilters = []FilterFunction{}
MergePathStrategy = PathJoinStrategy
}

func newTestService(addServiceFilter bool, addRouteFilter bool) *WebService {
Expand Down
24 changes: 22 additions & 2 deletions route_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,28 @@ func (b *RouteBuilder) Build() Route {
return route
}

func concatPath(path1, path2 string) string {
return path.Join(path1, path2)
type MergePathStrategyFunc func(rootPath, routePath string) string

var (
// behavior >= 3.10
PathJoinStrategy = func(rootPath, routePath string) string {
return path.Join(rootPath, routePath)
}

// behavior <= 3.9
TrimSlashStrategy = func(rootPath, routePath string) string {
return strings.TrimRight(rootPath, "/") + "/" + strings.TrimLeft(routePath, "/")
}

// MergePathStrategy is the active strategy for merging a Route path when building the routing of all WebServices.
// The value is set to PathJoinStrategy
// PathJoinStrategy is a strategy that is more strict [Security - PRISMA-2022-0227]
MergePathStrategy = PathJoinStrategy
)

// merge two paths using the current (package global) merge path strategy.
func concatPath(rootPath, routePath string) string {
return MergePathStrategy(rootPath, routePath)
}

var anonymousFuncCount int32
Expand Down
40 changes: 36 additions & 4 deletions web_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,38 @@ func TestOptionsShortcut(t *testing.T) {
}
}

func TestClientWithAndWithoutTrailingSlashOld(t *testing.T) {
tearDown()
MergePathStrategy = TrimSlashStrategy
ws := new(WebService).Path("/test")
ws.Route(ws.PUT("/").To(return200))
Add(ws)

for _, tt := range []struct {
url string
wantCode int
}{
// TrimSlashStrategy
{url: "http://here.com/test", wantCode: 404},
{url: "http://here.com/test/", wantCode: 200},
// PathJoinStrategy
//{url: "http://here.com/test", wantCode: 200},
//{url: "http://here.com/test/", wantCode: 404},
} {
t.Run(tt.url, func(t *testing.T) {
httpRequest, _ := http.NewRequest("PUT", tt.url, nil)
httpRequest.Header.Set("Accept", "*/*")
httpWriter := httptest.NewRecorder()
// override the default here
DefaultContainer.DoNotRecover(false)
DefaultContainer.dispatch(httpWriter, httpRequest)
if tt.wantCode != httpWriter.Code {
t.Errorf("Expected %d, got %d", tt.wantCode, httpWriter.Code)
}
})
}
}

func TestClientWithAndWithoutTrailingSlash(t *testing.T) {
tearDown()
ws := new(WebService).Path("/test")
Expand All @@ -337,10 +369,10 @@ func TestClientWithAndWithoutTrailingSlash(t *testing.T) {
url string
wantCode int
}{
// behavior before #520
// {url: "http://here.com/test", wantCode: 404},
// {url: "http://here.com/test/", wantCode: 200},
// current behavior
// TrimSlashStrategy
//{url: "http://here.com/test", wantCode: 404},
//{url: "http://here.com/test/", wantCode: 200},
// PathJoinStrategy
{url: "http://here.com/test", wantCode: 200},
{url: "http://here.com/test/", wantCode: 404},
} {
Expand Down