From 28fc186f5c178a13d7ab7fbe10384ba758bba05f Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Sun, 14 Jan 2024 22:20:46 +0200 Subject: [PATCH] added support for loading a serialized json payload as part of multipart/form-data request --- CHANGELOG.md | 3 + apis/record_crud_test.go | 134 ++++++++++++++++++++++++++++++++ forms/record_upsert.go | 14 +++- forms/record_upsert_test.go | 21 ++++- tools/rest/multi_binder.go | 21 ++++- tools/rest/multi_binder_test.go | 15 ++-- 6 files changed, 192 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ee7569e..f2ce3107 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ - Mark user as verified on confirm password reset ([#4066](https://github.com/pocketbase/pocketbase/issues/4066)). _If the user email has changed after issuing the reset token (eg. updated by an admin), then the `verified` user state remains unchanged._ +- Added support for loading a serialized json payload for `multipart/form-data` requests using the special `@jsonPayload` key. + _This is intended to be used primarily by the SDKs to resolve [js-sdk#274](https://github.com/pocketbase/js-sdk/issues/274)._ + - Added `TestMailer.SentMessages` field that holds all sent test app emails until cleanup. - Minor Admin UI improvements (reduced the min table row height, added new TinyMCE codesample plugin languages, etc.) diff --git a/apis/record_crud_test.go b/apis/record_crud_test.go index 7617e86e..145a63b9 100644 --- a/apis/record_crud_test.go +++ b/apis/record_crud_test.go @@ -14,6 +14,8 @@ import ( "github.com/pocketbase/pocketbase/core" "github.com/pocketbase/pocketbase/models" "github.com/pocketbase/pocketbase/tests" + "github.com/pocketbase/pocketbase/tools/rest" + "github.com/pocketbase/pocketbase/tools/types" ) func TestRecordCrudList(t *testing.T) { @@ -1030,6 +1032,20 @@ func TestRecordCrudCreate(t *testing.T) { t.Fatal(err) } + formData2, mp2, err2 := tests.MockMultipartData(map[string]string{ + rest.MultipartJsonKey: `{"title": "title_test2", "testPayload": 123}`, + }, "files") + if err2 != nil { + t.Fatal(err2) + } + + formData3, mp3, err3 := tests.MockMultipartData(map[string]string{ + rest.MultipartJsonKey: `{"title": "title_test3", "testPayload": 123}`, + }, "files") + if err3 != nil { + t.Fatal(err3) + } + scenarios := []tests.ApiScenario{ { Name: "missing collection", @@ -1237,6 +1253,58 @@ func TestRecordCrudCreate(t *testing.T) { "OnModelAfterCreate": 1, }, }, + { + Name: "submit via multipart form data with @jsonPayload key and unsatisfied @request.data rule", + Method: http.MethodPost, + Url: "/api/collections/demo3/records", + Body: formData2, + RequestHeaders: map[string]string{ + "Content-Type": mp2.FormDataContentType(), + }, + BeforeTestFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { + collection, err := app.Dao().FindCollectionByNameOrId("demo3") + if err != nil { + t.Fatalf("failed to find demo3 collection: %v", err) + } + collection.CreateRule = types.Pointer("@request.data.testPayload != 123") + if err := app.Dao().WithoutHooks().SaveCollection(collection); err != nil { + t.Fatalf("failed to update demo3 collection create rule: %v", err) + } + }, + ExpectedStatus: 400, + ExpectedContent: []string{`"data":{}`}, + }, + { + Name: "submit via multipart form data with @jsonPayload key and satisfied @request.data rule", + Method: http.MethodPost, + Url: "/api/collections/demo3/records", + Body: formData3, + RequestHeaders: map[string]string{ + "Content-Type": mp3.FormDataContentType(), + }, + BeforeTestFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { + collection, err := app.Dao().FindCollectionByNameOrId("demo3") + if err != nil { + t.Fatalf("failed to find demo3 collection: %v", err) + } + collection.CreateRule = types.Pointer("@request.data.testPayload = 123") + if err := app.Dao().WithoutHooks().SaveCollection(collection); err != nil { + t.Fatalf("failed to update demo3 collection create rule: %v", err) + } + }, + ExpectedStatus: 200, + ExpectedContent: []string{ + `"id":"`, + `"title":"title_test3"`, + `"files":["`, + }, + ExpectedEvents: map[string]int{ + "OnRecordBeforeCreateRequest": 1, + "OnRecordAfterCreateRequest": 1, + "OnModelBeforeCreate": 1, + "OnModelAfterCreate": 1, + }, + }, { Name: "unique field error check", Method: http.MethodPost, @@ -1608,6 +1676,20 @@ func TestRecordCrudUpdate(t *testing.T) { t.Fatal(err) } + formData2, mp2, err2 := tests.MockMultipartData(map[string]string{ + rest.MultipartJsonKey: `{"title": "title_test2", "testPayload": 123}`, + }, "files") + if err2 != nil { + t.Fatal(err2) + } + + formData3, mp3, err3 := tests.MockMultipartData(map[string]string{ + rest.MultipartJsonKey: `{"title": "title_test3", "testPayload": 123}`, + }, "files") + if err3 != nil { + t.Fatal(err3) + } + scenarios := []tests.ApiScenario{ { Name: "missing collection", @@ -1830,6 +1912,58 @@ func TestRecordCrudUpdate(t *testing.T) { "OnModelAfterUpdate": 1, }, }, + { + Name: "submit via multipart form data with @jsonPayload key and unsatisfied @request.data rule", + Method: http.MethodPatch, + Url: "/api/collections/demo3/records/mk5fmymtx4wsprk", + Body: formData2, + RequestHeaders: map[string]string{ + "Content-Type": mp2.FormDataContentType(), + }, + BeforeTestFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { + collection, err := app.Dao().FindCollectionByNameOrId("demo3") + if err != nil { + t.Fatalf("failed to find demo3 collection: %v", err) + } + collection.UpdateRule = types.Pointer("@request.data.testPayload != 123") + if err := app.Dao().WithoutHooks().SaveCollection(collection); err != nil { + t.Fatalf("failed to update demo3 collection update rule: %v", err) + } + }, + ExpectedStatus: 404, + ExpectedContent: []string{`"data":{}`}, + }, + { + Name: "submit via multipart form data with @jsonPayload key and satisfied @request.data rule", + Method: http.MethodPatch, + Url: "/api/collections/demo3/records/mk5fmymtx4wsprk", + Body: formData3, + RequestHeaders: map[string]string{ + "Content-Type": mp3.FormDataContentType(), + }, + BeforeTestFunc: func(t *testing.T, app *tests.TestApp, e *echo.Echo) { + collection, err := app.Dao().FindCollectionByNameOrId("demo3") + if err != nil { + t.Fatalf("failed to find demo3 collection: %v", err) + } + collection.UpdateRule = types.Pointer("@request.data.testPayload = 123") + if err := app.Dao().WithoutHooks().SaveCollection(collection); err != nil { + t.Fatalf("failed to update demo3 collection update rule: %v", err) + } + }, + ExpectedStatus: 200, + ExpectedContent: []string{ + `"id":"mk5fmymtx4wsprk"`, + `"title":"title_test3"`, + `"files":["`, + }, + ExpectedEvents: map[string]int{ + "OnRecordBeforeUpdateRequest": 1, + "OnRecordAfterUpdateRequest": 1, + "OnModelBeforeUpdate": 1, + "OnModelAfterUpdate": 1, + }, + }, { Name: "OnRecordAfterUpdateRequest error response", Method: http.MethodPatch, diff --git a/forms/record_upsert.go b/forms/record_upsert.go index 587ff1ca..d4a86b5b 100644 --- a/forms/record_upsert.go +++ b/forms/record_upsert.go @@ -165,7 +165,7 @@ func (form *RecordUpsert) extractMultipartFormData( data := map[string]any{} filesToUpload := map[string][]*filesystem.File{} - arrayValueSupportTypes := schema.ArraybleFieldTypes() + arraybleFieldTypes := schema.ArraybleFieldTypes() for fullKey, values := range r.PostForm { key := fullKey @@ -178,8 +178,18 @@ func (form *RecordUpsert) extractMultipartFormData( continue } + // special case for multipart json encoded fields + if key == rest.MultipartJsonKey { + for _, v := range values { + if err := json.Unmarshal([]byte(v), &data); err != nil { + form.app.Logger().Debug("Failed to decode @json value into the data map", "error", err, "value", v) + } + } + continue + } + field := form.record.Collection().Schema.GetFieldByName(key) - if field != nil && list.ExistInSlice(field.Type, arrayValueSupportTypes) { + if field != nil && list.ExistInSlice(field.Type, arraybleFieldTypes) { data[key] = values } else { data[key] = values[0] diff --git a/forms/record_upsert_test.go b/forms/record_upsert_test.go index 22bb86ef..6a730f81 100644 --- a/forms/record_upsert_test.go +++ b/forms/record_upsert_test.go @@ -22,6 +22,7 @@ import ( "github.com/pocketbase/pocketbase/tests" "github.com/pocketbase/pocketbase/tools/filesystem" "github.com/pocketbase/pocketbase/tools/list" + "github.com/pocketbase/pocketbase/tools/rest" "github.com/pocketbase/pocketbase/tools/types" ) @@ -150,9 +151,10 @@ func TestRecordUpsertLoadRequestMultipart(t *testing.T) { } formData, mp, err := tests.MockMultipartData(map[string]string{ - "a.b.id": "test_id", - "a.b.text": "test123", - "a.b.unknown": "test456", + "a.b.id": "test_id", + "a.b.text": "test123", + "a.b.unknown": "test456", + "a.b." + rest.MultipartJsonKey: `{"json":["a","b"],"email":"test3@example.com"}`, // file fields unset/delete "a.b.file_one-": "test_d61b33QdDU.txt", // delete with modifier "a.b.file_many.0": "", // delete by index @@ -184,6 +186,19 @@ func TestRecordUpsertLoadRequestMultipart(t *testing.T) { t.Fatalf("Didn't expect unknown field to be set, got %v", v) } + if v, ok := form.Data()["email"]; !ok || v != "test3@example.com" { + t.Fatalf("Expect email field to be %q, got %q", "test3@example.com", v) + } + + rawJsonValue, ok := form.Data()["json"].(types.JsonRaw) + if !ok { + t.Fatal("Expect json field to be set") + } + expectedJsonValue := `["a","b"]` + if rawJsonValue.String() != expectedJsonValue { + t.Fatalf("Expect json field %v, got %v", expectedJsonValue, rawJsonValue) + } + fileOne, ok := form.Data()["file_one"] if !ok { t.Fatal("Expect file_one field to be set") diff --git a/tools/rest/multi_binder.go b/tools/rest/multi_binder.go index 9f026354..afaf6541 100644 --- a/tools/rest/multi_binder.go +++ b/tools/rest/multi_binder.go @@ -12,6 +12,10 @@ import ( "github.com/spf13/cast" ) +// MultipartJsonKey is the key for the special multipart/form-data +// handling allowing reading serialized json payload without normalizations +const MultipartJsonKey string = "@jsonPayload" + // BindBody binds request body content to i. // // This is similar to `echo.BindBody()`, but for JSON requests uses @@ -62,10 +66,8 @@ func CopyJsonBody(r *http.Request, i any) error { return err } -// This is temp hotfix for properly binding multipart/form-data array values -// when a map destination is used. -// -// It should be replaced with echo.BindBody(c, i) once the issue is fixed in echo. +// Custom multipart/form-data binder that implements an additional handling like +// loading a serialized json payload or properly scan array values when a map destination is used. func bindFormData(c echo.Context, i any) error { if i == nil { return nil @@ -80,6 +82,13 @@ func bindFormData(c echo.Context, i any) error { return nil } + // special case to allow submitting json without normalizations + // alongside the other multipart/form-data values + jsonPayloadValues := values[MultipartJsonKey] + for _, payload := range jsonPayloadValues { + json.Unmarshal([]byte(payload), i) + } + rt := reflect.TypeOf(i).Elem() // map @@ -87,6 +96,10 @@ func bindFormData(c echo.Context, i any) error { rv := reflect.ValueOf(i).Elem() for k, v := range values { + if k == MultipartJsonKey { + continue + } + if total := len(v); total == 1 { rv.SetMapIndex(reflect.ValueOf(k), reflect.ValueOf(normalizeMultipartValue(v[0]))) } else { diff --git a/tools/rest/multi_binder_test.go b/tools/rest/multi_binder_test.go index 48d63519..402c9d94 100644 --- a/tools/rest/multi_binder_test.go +++ b/tools/rest/multi_binder_test.go @@ -41,16 +41,17 @@ func TestBindBody(t *testing.T) { { strings.NewReader( url.Values{ - "string": []string{"str"}, - "stings": []string{"str1", "str2", ""}, - "number": []string{"-123"}, - "numbers": []string{"123", "456.789"}, - "bool": []string{"true"}, - "bools": []string{"true", "false"}, + "string": []string{"str"}, + "stings": []string{"str1", "str2", ""}, + "number": []string{"-123"}, + "numbers": []string{"123", "456.789"}, + "bool": []string{"true"}, + "bools": []string{"true", "false"}, + rest.MultipartJsonKey: []string{`invalid`, `{"a":123}`, `{"b":456}`}, }.Encode(), ), echo.MIMEApplicationForm, - `{"bool":true,"bools":[true,false],"number":-123,"numbers":[123,456.789],"stings":["str1","str2",""],"string":"str"}`, + `{"a":123,"b":456,"bool":true,"bools":[true,false],"number":-123,"numbers":[123,456.789],"stings":["str1","str2",""],"string":"str"}`, false, }, }