diff --git a/CHANGES.md b/CHANGES.md index 352018e7..5edd5a7c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,11 +1,15 @@ # Change history of go-restful -## [v3.10.2] - 2023-03-09 +## [v3.11.0] - 2023-08-19 + +- restored behavior as <= v3.9.0 with option to change path strategy using TrimRightSlashEnabled. + +## [v3.10.2] - 2023-03-09 - DO NOT USE - introduced MergePathStrategy to be able to revert behaviour of path concatenation to 3.9.0 see comment in Readme how to customize this behaviour. -## [v3.10.1] - 2022-11-19 +## [v3.10.1] - 2022-11-19 - DO NOT USE - fix broken 3.10.0 by using path package for joining paths diff --git a/README.md b/README.md index 2451cf4f..265f089c 100644 --- a/README.md +++ b/README.md @@ -96,10 +96,7 @@ 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) +- Use the package variable `TrimRightSlashEnabled` (default true) to controls the behavior of matching routes that end with a slash `/` ## Resources diff --git a/curly_test.go b/curly_test.go index dfdd790d..d42eed37 100644 --- a/curly_test.go +++ b/curly_test.go @@ -106,7 +106,7 @@ var routeMatchers = []struct { {"/a", "/a", true, 0, 1, false}, {"/a", "/b", false, 0, 0, false}, {"/a", "/b", false, 0, 0, false}, - {"/a/{b}/c/", "/a/2/c", false, 0, 0, false}, + {"/a/{b}/c/", "/a/2/c", true, 1, 2, false}, {"/{a}/{b}/{c}/", "/a/b", false, 0, 0, false}, {"/{x:*}", "/", false, 0, 0, false}, {"/{x:*}", "/a", true, 1, 0, false}, diff --git a/examples/tests/restful-curly-router_test.go b/examples/tests/restful-curly-router_test.go deleted file mode 100644 index 87aed068..00000000 --- a/examples/tests/restful-curly-router_test.go +++ /dev/null @@ -1,149 +0,0 @@ -package main - -import ( - "bytes" - "errors" - "log" - "net/http" - "testing" - "time" - - "github.com/emicklei/go-restful" -) - -type User struct { - Id, Name string -} - -type UserResource struct { - users map[string]User -} - -func (u UserResource) Register(container *restful.Container) { - ws := new(restful.WebService) - ws. - Path("/users"). - Consumes(restful.MIME_XML, restful.MIME_JSON). - Produces(restful.MIME_JSON, restful.MIME_XML) - - ws.Route(ws.GET("/{user-id}").To(u.findUser)) - ws.Route(ws.POST("").To(u.updateUser)) - ws.Route(ws.PUT("/{user-id}").To(u.createUser)) - ws.Route(ws.DELETE("/{user-id}").To(u.removeUser)) - - container.Add(ws) -} - -// GET http://localhost:8090/users/1 -// -func (u UserResource) findUser(request *restful.Request, response *restful.Response) { - id := request.PathParameter("user-id") - usr := u.users[id] - if len(usr.Id) == 0 { - response.AddHeader("Content-Type", "text/plain") - response.WriteErrorString(http.StatusNotFound, "User could not be found.") - } else { - response.WriteEntity(usr) - } -} - -// POST http://localhost:8090/users -// 1Melissa Raspberry -// -func (u *UserResource) updateUser(request *restful.Request, response *restful.Response) { - usr := new(User) - err := request.ReadEntity(&usr) - if err == nil { - u.users[usr.Id] = *usr - response.WriteEntity(usr) - } else { - response.AddHeader("Content-Type", "text/plain") - response.WriteErrorString(http.StatusInternalServerError, err.Error()) - } -} - -// PUT http://localhost:8090/users/1 -// 1Melissa -// -func (u *UserResource) createUser(request *restful.Request, response *restful.Response) { - usr := User{Id: request.PathParameter("user-id")} - err := request.ReadEntity(&usr) - if err == nil { - u.users[usr.Id] = usr - response.WriteHeader(http.StatusCreated) - response.WriteEntity(usr) - } else { - response.AddHeader("Content-Type", "text/plain") - response.WriteErrorString(http.StatusInternalServerError, err.Error()) - } -} - -// DELETE http://localhost:8090/users/1 -// -func (u *UserResource) removeUser(request *restful.Request, response *restful.Response) { - id := request.PathParameter("user-id") - delete(u.users, id) -} - -func RunRestfulCurlyRouterServer() { - wsContainer := restful.NewContainer() - wsContainer.Router(restful.CurlyRouter{}) - u := UserResource{map[string]User{}} - u.Register(wsContainer) - - log.Print("start listening on localhost:8090") - server := &http.Server{Addr: ":8090", Handler: wsContainer} - log.Fatal(server.ListenAndServe()) -} - -func waitForServerUp(serverURL string) error { - for start := time.Now(); time.Since(start) < time.Minute; time.Sleep(5 * time.Second) { - _, err := http.Get(serverURL + "/") - if err == nil { - return nil - } - } - return errors.New("waiting for server timed out") -} - -func TestServer(t *testing.T) { - serverURL := "http://localhost:8090" - go func() { - RunRestfulCurlyRouterServer() - }() - if err := waitForServerUp(serverURL); err != nil { - t.Errorf("%v", err) - } - - // GET should give a 405 - resp, err := http.Get(serverURL + "/users/") - if err != nil { - t.Errorf("unexpected error in GET /users/: %v", err) - } - if resp.StatusCode != http.StatusMethodNotAllowed { - t.Errorf("unexpected response: %v, expected: %v", resp.StatusCode, http.StatusOK) - } - - // Send a POST request. - var jsonStr = []byte(`{"id":"1","name":"user1"}`) - req, err := http.NewRequest("POST", serverURL+"/users/", bytes.NewBuffer(jsonStr)) - req.Header.Set("Content-Type", restful.MIME_JSON) - - client := &http.Client{} - resp, err = client.Do(req) - if err != nil { - t.Errorf("unexpected error in sending req: %v", err) - } - if resp.StatusCode != http.StatusOK { - t.Errorf("unexpected response: %v, expected: %v", resp.StatusCode, http.StatusOK) - } - - // Test that GET works. - resp, err = http.Get(serverURL + "/users/1") - if err != nil { - t.Errorf("unexpected error in GET /users/1: %v", err) - } - if resp.StatusCode != http.StatusOK { - t.Errorf("unexpected response: %v, expected: %v", resp.StatusCode, http.StatusOK) - } -} diff --git a/examples/tests/restful-route_test.go b/examples/tests/restful-route_test.go deleted file mode 100644 index 20c366bf..00000000 --- a/examples/tests/restful-route_test.go +++ /dev/null @@ -1,39 +0,0 @@ -package main - -import ( - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/emicklei/go-restful" -) - -var ( - Result string -) - -func TestRouteExtractParameter(t *testing.T) { - // setup service - ws := new(restful.WebService) - ws.Consumes(restful.MIME_XML) - ws.Route(ws.GET("/test/{param}").To(DummyHandler)) - restful.Add(ws) - - // setup request + writer - bodyReader := strings.NewReader("42") - httpRequest, _ := http.NewRequest("GET", "/test/THIS", bodyReader) - httpRequest.Header.Set("Content-Type", restful.MIME_XML) - httpWriter := httptest.NewRecorder() - - // run - restful.DefaultContainer.ServeHTTP(httpWriter, httpRequest) - - if Result != "THIS" { - t.Fatalf("Result is actually: %s", Result) - } -} - -func DummyHandler(rq *restful.Request, rp *restful.Response) { - Result = rq.PathParameter("param") -} diff --git a/examples/tests/restful-routefunction_test.go b/examples/tests/restful-routefunction_test.go deleted file mode 100644 index 957c0555..00000000 --- a/examples/tests/restful-routefunction_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package main - -import ( - "net/http" - "net/http/httptest" - "testing" - - "github.com/emicklei/go-restful" -) - -// This example show how to test one particular RouteFunction (getIt) -// It uses the httptest.ResponseRecorder to capture output - -func getIt(req *restful.Request, resp *restful.Response) { - resp.WriteHeader(204) -} - -func TestCallFunction(t *testing.T) { - httpReq, _ := http.NewRequest("GET", "/", nil) - req := restful.NewRequest(httpReq) - - recorder := new(httptest.ResponseRecorder) - resp := restful.NewResponse(recorder) - - getIt(req, resp) - if recorder.Code != 204 { - t.Fatalf("Missing or wrong status code:%d", recorder.Code) - } -} diff --git a/examples/user-resource/go.mod b/examples/user-resource/go.mod index a1a99290..481659bd 100644 --- a/examples/user-resource/go.mod +++ b/examples/user-resource/go.mod @@ -2,6 +2,8 @@ module github.com/emicklei/go-restful/examples/user-resource go 1.14 +replace github.com/emicklei/go-restful/v3 => ../../. + require ( github.com/emicklei/go-restful-openapi/v2 v2.8.0 github.com/emicklei/go-restful/v3 v3.8.0 diff --git a/examples/user-resource/restful-user-resource.go b/examples/user-resource/restful-user-resource.go index 550cf065..1fee5634 100644 --- a/examples/user-resource/restful-user-resource.go +++ b/examples/user-resource/restful-user-resource.go @@ -1,8 +1,10 @@ package main import ( + "fmt" "log" "net/http" + "time" restfulspec "github.com/emicklei/go-restful-openapi/v2" restful "github.com/emicklei/go-restful/v3" @@ -41,14 +43,14 @@ func (u UserResource) WebService() *restful.WebService { Returns(200, "OK", User{}). Returns(404, "Not Found", nil)) - ws.Route(ws.PUT("/{user-id}").To(u.updateUser). + ws.Route(ws.PUT("/{user-id}").To(u.upsertUser). // docs Doc("update a user"). Param(ws.PathParameter("user-id", "identifier of the user").DataType("string")). Metadata(restfulspec.KeyOpenAPITags, tags). Reads(User{})) // from the request - ws.Route(ws.PUT("").To(u.createUser). + ws.Route(ws.POST("").To(u.createUser). // docs Doc("create a user"). Metadata(restfulspec.KeyOpenAPITags, tags). @@ -64,8 +66,8 @@ func (u UserResource) WebService() *restful.WebService { } // GET http://localhost:8080/users -// func (u UserResource) findAllUsers(request *restful.Request, response *restful.Response) { + log.Println("findAllUsers") list := []User{} for _, each := range u.users { list = append(list, each) @@ -74,8 +76,8 @@ func (u UserResource) findAllUsers(request *restful.Request, response *restful.R } // GET http://localhost:8080/users/1 -// func (u UserResource) findUser(request *restful.Request, response *restful.Response) { + log.Println("findUser") id := request.PathParameter("user-id") usr := u.users[id] if len(usr.ID) == 0 { @@ -87,23 +89,23 @@ func (u UserResource) findUser(request *restful.Request, response *restful.Respo // PUT http://localhost:8080/users/1 // 1Melissa Raspberry -// -func (u *UserResource) updateUser(request *restful.Request, response *restful.Response) { - usr := new(User) +func (u *UserResource) upsertUser(request *restful.Request, response *restful.Response) { + log.Println("upsertUser") + usr := User{ID: request.PathParameter("user-id")} err := request.ReadEntity(&usr) if err == nil { - u.users[usr.ID] = *usr + u.users[usr.ID] = usr response.WriteEntity(usr) } else { response.WriteError(http.StatusInternalServerError, err) } } -// PUT http://localhost:8080/users/1 +// POST http://localhost:8080/users // 1Melissa -// func (u *UserResource) createUser(request *restful.Request, response *restful.Response) { - usr := User{ID: request.PathParameter("user-id")} + log.Println("createUser") + usr := User{ID: fmt.Sprintf("%d", time.Now().Unix())} err := request.ReadEntity(&usr) if err == nil { u.users[usr.ID] = usr @@ -114,8 +116,8 @@ func (u *UserResource) createUser(request *restful.Request, response *restful.Re } // DELETE http://localhost:8080/users/1 -// func (u *UserResource) removeUser(request *restful.Request, response *restful.Response) { + log.Println("removeUser") id := request.PathParameter("user-id") delete(u.users, id) } @@ -167,7 +169,7 @@ func enrichSwaggerObject(swo *spec.Swagger) { // User is just a sample type type User struct { - ID string `json:"id" description:"identifier of the user"` - Name string `json:"name" description:"name of the user" default:"john"` - Age int `json:"age" description:"age of the user" default:"21"` + ID string `xml:"id" json:"id" description:"identifier of the user"` + Name string `xml:"name" json:"name" description:"name of the user" default:"john"` + Age int `xml:"age" json:"age" description:"age of the user" default:"21"` } diff --git a/filter_test.go b/filter_test.go index df6fa4bc..fadfb570 100644 --- a/filter_test.go +++ b/filter_test.go @@ -18,7 +18,6 @@ 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 { diff --git a/path_processor_test.go b/path_processor_test.go index 2fc1540a..d269a8af 100644 --- a/path_processor_test.go +++ b/path_processor_test.go @@ -33,7 +33,15 @@ func TestMatchesPath_TwoVars(t *testing.T) { } func TestMatchesPath_VarOnFront(t *testing.T) { - params := doExtractParams("{what}/from/{source}/", 4, "who/from/SOS/", t) // slash is not removed + params := doExtractParams("{what}/from/{source}/", 3, "who/from/SOS/", t) + if params["source"] != "SOS" { + t.Errorf("parameter mismatch SOS") + } +} + +func TestMatchesPath_VarOnFront_KeepSlash(t *testing.T) { + TrimRightSlashEnabled = false + params := doExtractParams("{what}/from/{source}/", 4, "who/from/SOS/", t) if params["source"] != "SOS" { t.Errorf("parameter mismatch SOS") } diff --git a/route.go b/route.go index 66aadd09..306c44be 100644 --- a/route.go +++ b/route.go @@ -165,7 +165,13 @@ func tokenizePath(path string) []string { if "/" == path { return nil } - return strings.Split(strings.TrimLeft(path, "/"), "/") + if TrimRightSlashEnabled { + // 3.9.0 + return strings.Split(strings.Trim(path, "/"), "/") + } else { + // 3.10.2 + return strings.Split(strings.TrimLeft(path, "/"), "/") + } } // for debugging @@ -178,4 +184,8 @@ func (r *Route) EnableContentEncoding(enabled bool) { r.contentEncodingEnabled = &enabled } -var TrimRightSlashEnabled = false +// TrimRightSlashEnabled controls whether +// - path on route building is using path.Join +// - the path of the incoming request is trimmed of its slash suffux. +// Value of true matches the behavior of <= 3.9.0 +var TrimRightSlashEnabled = true diff --git a/route_builder.go b/route_builder.go index d0513c03..75168c12 100644 --- a/route_builder.go +++ b/route_builder.go @@ -358,28 +358,14 @@ func (b *RouteBuilder) Build() Route { return route } -type MergePathStrategyFunc func(rootPath, routePath string) string - -var ( - // behavior >= 3.10 - PathJoinStrategy = func(rootPath, routePath string) string { - return path.Join(rootPath, routePath) - } +// merge two paths using the current (package global) merge path strategy. +func concatPath(rootPath, routePath string) string { - // behavior <= 3.9 - TrimSlashStrategy = func(rootPath, routePath string) string { + if TrimRightSlashEnabled { return strings.TrimRight(rootPath, "/") + "/" + strings.TrimLeft(routePath, "/") + } else { + return path.Join(rootPath, 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 diff --git a/web_service_test.go b/web_service_test.go index c5a1ac10..f316d26d 100644 --- a/web_service_test.go +++ b/web_service_test.go @@ -327,69 +327,6 @@ 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") - 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 newPanicingService() *WebService { ws := new(WebService).Path("") ws.Route(ws.GET("/fire").To(doPanic))