diff --git a/apis/base.go b/apis/base.go index 3e53f0bc..7a89623c 100644 --- a/apis/base.go +++ b/apis/base.go @@ -28,6 +28,7 @@ const trailedAdminPath = "/_/" func InitApi(app core.App) (*echo.Echo, error) { e := echo.New() e.Debug = false + e.Binder = &rest.MultiBinder{} e.JSONSerializer = &rest.Serializer{ FieldsParam: fieldsQueryParam, } diff --git a/apis/base_test.go b/apis/base_test.go index aa61e7ae..ea4a3ace 100644 --- a/apis/base_test.go +++ b/apis/base_test.go @@ -11,6 +11,7 @@ import ( "github.com/labstack/echo/v5" "github.com/pocketbase/pocketbase/apis" "github.com/pocketbase/pocketbase/tests" + "github.com/pocketbase/pocketbase/tools/rest" "github.com/spf13/cast" ) @@ -220,15 +221,24 @@ func TestRemoveTrailingSlashMiddleware(t *testing.T) { } } -func TestEagerRequestInfoCache(t *testing.T) { +func TestMultiBinder(t *testing.T) { t.Parallel() + rawJson := `{"name":"test123"}` + + formData, mp, err := tests.MockMultipartData(map[string]string{ + rest.MultipartJsonKey: rawJson, + }) + if err != nil { + t.Fatal(err) + } + scenarios := []tests.ApiScenario{ { - Name: "custom non-api group route", + Name: "non-api group route", Method: "POST", Url: "/custom", - Body: strings.NewReader(`{"name":"test123"}`), + Body: strings.NewReader(rawJson), BeforeTestFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { e.AddRoute(echo.Route{ Method: "POST", @@ -242,11 +252,10 @@ func TestEagerRequestInfoCache(t *testing.T) { return err } - // since the unknown method is not eager cache support - // it should fail reading the json body twice + // try to read the body again r := apis.RequestInfo(c) - if v := cast.ToString(r.Data["name"]); v != "" { - t.Fatalf("Expected empty request data body, got, %v", r.Data) + if v := cast.ToString(r.Data["name"]); v != "test123" { + t.Fatalf("Expected request data with name %q, got, %q", "test123", v) } return c.NoContent(200) @@ -256,41 +265,10 @@ func TestEagerRequestInfoCache(t *testing.T) { ExpectedStatus: 200, }, { - Name: "api group route with unsupported eager cache request method", + Name: "api group route", Method: "GET", Url: "/api/admins", - Body: strings.NewReader(`{"name":"test123"}`), - BeforeTestFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { - e.Use(func(next echo.HandlerFunc) echo.HandlerFunc { - return func(c echo.Context) error { - // it is not important whether the route handler return an error since - // we just need to ensure that the eagerRequestInfoCache was registered - next(c) - - // ensure that the body was read at least once - data := &struct { - Name string `json:"name"` - }{} - c.Bind(data) - - // since the unknown method is not eager cache support - // it should fail reading the json body twice - r := apis.RequestInfo(c) - if v := cast.ToString(r.Data["name"]); v != "" { - t.Fatalf("Expected empty request data body, got, %v", r.Data) - } - - return nil - } - }) - }, - ExpectedStatus: 200, - }, - { - Name: "api group route with supported eager cache request method", - Method: "POST", - Url: "/api/admins", - Body: strings.NewReader(`{"name":"test123"}`), + Body: strings.NewReader(rawJson), BeforeTestFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { e.Use(func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { @@ -316,6 +294,39 @@ func TestEagerRequestInfoCache(t *testing.T) { }, ExpectedStatus: 200, }, + { + Name: "custom route with @jsonPayload as multipart body", + Method: "POST", + Url: "/custom", + Body: formData, + RequestHeaders: map[string]string{ + "Content-Type": mp.FormDataContentType(), + }, + BeforeTestFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { + e.AddRoute(echo.Route{ + Method: "POST", + Path: "/custom", + Handler: func(c echo.Context) error { + data := &struct { + Name string `json:"name"` + }{} + + if err := c.Bind(data); err != nil { + return err + } + + // try to read the body again + r := apis.RequestInfo(c) + if v := cast.ToString(r.Data["name"]); v != "test123" { + t.Fatalf("Expected request data with name %q, got, %q", "test123", v) + } + + return c.NoContent(200) + }, + }) + }, + ExpectedStatus: 200, + }, } for _, scenario := range scenarios { diff --git a/apis/middlewares.go b/apis/middlewares.go index 0bf935a1..c50b7595 100644 --- a/apis/middlewares.go +++ b/apis/middlewares.go @@ -402,6 +402,8 @@ func realUserIp(r *http.Request, fallbackIp string) string { return fallbackIp } +// @todo consider removing as this may no longer be needed due to the custom rest.MultiBinder. +// // eagerRequestInfoCache ensures that the request data is cached in the request // context to allow reading for example the json request body data more than once. func eagerRequestInfoCache(app core.App) echo.MiddlewareFunc { diff --git a/tools/rest/json_serializer.go b/tools/rest/json_serializer.go index 57aad502..6c2dfd01 100644 --- a/tools/rest/json_serializer.go +++ b/tools/rest/json_serializer.go @@ -68,8 +68,8 @@ func (s *Serializer) Serialize(c echo.Context, i any, indent string) error { // // Example: // -// data := map[string]any{"a": 1, "b": 2, "c": map[string]any{"c1": 11, "c2": 22}} -// PickFields(data, "a,c.c1") // map[string]any{"a": 1, "c": map[string]any{"c1": 11}} +// data := map[string]any{"a": 1, "b": 2, "c": map[string]any{"c1": 11, "c2": 22}} +// PickFields(data, "a,c.c1") // map[string]any{"a": 1, "c": map[string]any{"c1": 11}} func PickFields(data any, rawFields string) (any, error) { parsedFields, err := parseFields(rawFields) if err != nil { diff --git a/tools/rest/multi_binder.go b/tools/rest/multi_binder.go index afaf6541..6fcc2e29 100644 --- a/tools/rest/multi_binder.go +++ b/tools/rest/multi_binder.go @@ -13,9 +13,35 @@ import ( ) // MultipartJsonKey is the key for the special multipart/form-data -// handling allowing reading serialized json payload without normalizations +// handling allowing reading serialized json payload without normalization. const MultipartJsonKey string = "@jsonPayload" +// MultiBinder is similar to [echo.DefaultBinder] but uses slightly different +// application/json and multipart/form-data bind methods to accommodate better +// the PocketBase router needs. +type MultiBinder struct{} + +// Bind implements the [Binder.Bind] method. +// +// Bind is almost identical to [echo.DefaultBinder.Bind] but uses the +// [rest.BindBody] function for binding the request body. +func (b *MultiBinder) Bind(c echo.Context, i interface{}) (err error) { + if err := echo.BindPathParams(c, i); err != nil { + return err + } + + // Only bind query parameters for GET/DELETE/HEAD to avoid unexpected behavior with destination struct binding from body. + // For example a request URL `&id=1&lang=en` with body `{"id":100,"lang":"de"}` would lead to precedence issues. + method := c.Request().Method + if method == http.MethodGet || method == http.MethodDelete || method == http.MethodHead { + if err = echo.BindQueryParams(c, i); err != nil { + return err + } + } + + return BindBody(c, i) +} + // BindBody binds request body content to i. // // This is similar to `echo.BindBody()`, but for JSON requests uses @@ -47,8 +73,8 @@ func BindBody(c echo.Context, i any) error { func CopyJsonBody(r *http.Request, i any) error { body := r.Body - // this usually shouldn't be needed because the Server calls close for us - // but we are changing the request body with a new reader + // this usually shouldn't be needed because the Server calls close + // for us but we are changing the request body with a new reader defer body.Close() limitReader := io.LimitReader(body, DefaultMaxMemory) @@ -82,7 +108,7 @@ func bindFormData(c echo.Context, i any) error { return nil } - // special case to allow submitting json without normalizations + // special case to allow submitting json without normalization // alongside the other multipart/form-data values jsonPayloadValues := values[MultipartJsonKey] for _, payload := range jsonPayloadValues { diff --git a/tools/rest/multi_binder_test.go b/tools/rest/multi_binder_test.go index 402c9d94..24732eca 100644 --- a/tools/rest/multi_binder_test.go +++ b/tools/rest/multi_binder_test.go @@ -13,6 +13,46 @@ import ( "github.com/pocketbase/pocketbase/tools/rest" ) +func TestMultiBinderBind(t *testing.T) { + binder := rest.MultiBinder{} + + req := httptest.NewRequest(http.MethodGet, "/test?query=123", strings.NewReader(`{"body":"456"}`)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + + rec := httptest.NewRecorder() + + e := echo.New() + e.Any("/:name", func(c echo.Context) error { + // bind twice to ensure that the json body reader copy is invoked + for i := 0; i < 2; i++ { + data := struct { + Name string `param:"name"` + Query string `query:"query"` + Body string `form:"body"` + }{} + + if err := binder.Bind(c, &data); err != nil { + t.Fatal(err) + } + + if data.Name != "test" { + t.Fatalf("Expected Name %q, got %q", "test", data.Name) + } + + if data.Query != "123" { + t.Fatalf("Expected Query %q, got %q", "123", data.Query) + } + + if data.Body != "456" { + t.Fatalf("Expected Body %q, got %q", "456", data.Body) + } + } + + return nil + }) + e.ServeHTTP(rec, req) +} + func TestBindBody(t *testing.T) { scenarios := []struct { body io.Reader