diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d17dd44..5231b70a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,9 @@ - Clamped arccosine to [-1,1] in the Harvesine formula for the `geoDistance()` filter function to workaround edge case related to float rounding errors for some coordinates. -- Prevent unnecessery body chunk read if we already known that we are beyond the allowed limit. +- Prevent unnecessary body chunk read if we already known that we are beyond the allowed limit. + +- Updated the `json` field validator to check the `encoding/json/v2` semantics and allow duplicated keys when serializing for compliance with old jsonv1 data. - Bumped `golang.org/x/*` dependencies to silence security scanners ([#7829](https://github.com/pocketbase/pocketbase/discussions/7829)). diff --git a/core/field_json.go b/core/field_json.go index 5dac9bc9..2d3588d5 100644 --- a/core/field_json.go +++ b/core/field_json.go @@ -2,6 +2,8 @@ package core import ( "context" + "encoding/json/jsontext" + "encoding/json/v2" "slices" "strconv" "strings" @@ -171,6 +173,18 @@ func (f *JSONField) ValidateValue(ctx context.Context, app App, record *Record) return validation.NewError("validation_invalid_json", "Must be a valid json value") } + // temp extra encoding/json/v2 check since the above validator is + // still using the v1 semantics + // + // @todo remove after updating the string validator + if len(raw) > 0 { + var dummy any + err := json.Unmarshal(raw, &dummy, jsontext.AllowInvalidUTF8(true)) + if err != nil { + return validation.NewError("validation_invalid_json", "Must be a valid json value") + } + } + rawStr := strings.TrimSpace(raw.String()) if f.Required && slices.Contains(emptyJSONValues, rawStr) { diff --git a/core/field_json_test.go b/core/field_json_test.go index 13eb1f21..52fa4be4 100644 --- a/core/field_json_test.go +++ b/core/field_json_test.go @@ -101,6 +101,16 @@ func TestJSONFieldValidateValue(t *testing.T) { }, true, }, + { + "invalid raw value (v2 semantics)", + &core.JSONField{Name: "test"}, + func() *core.Record { + record := core.NewRecord(collection) + record.SetRaw("test", types.JSONRaw(`{"a": 1, "a": 2}`)) + return record + }, + true, + }, { "zero field value (not required)", &core.JSONField{Name: "test"}, diff --git a/core/record_model.go b/core/record_model.go index b27b5203..a8c4199a 100644 --- a/core/record_model.go +++ b/core/record_model.go @@ -1333,7 +1333,9 @@ func (m Record) MarshalJSON() ([]byte, error) { return json.Marshal( m.PublicExport(), json.Deterministic(true), - // to preserve the old jsonv1 behavior in case of invalid data + // for compliance with old data (and slightly better performance) + jsontext.AllowDuplicateNames(true), + // preserve the old jsonv1 behavior in case of invalid data jsontext.AllowInvalidUTF8(true), ) } diff --git a/core/record_model_test.go b/core/record_model_test.go index 75e046e2..2bcf2338 100644 --- a/core/record_model_test.go +++ b/core/record_model_test.go @@ -1413,14 +1413,15 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { f3 := &core.SelectField{Name: "field3", MaxSelect: 2, Values: []string{"test1", "test2", "test3"}} f4 := &core.TextField{Name: "field4", Hidden: true} f5 := &core.TextField{Name: "field5", Hidden: true} + f6 := &core.JSONField{Name: "field6"} colBase := core.NewBaseCollection("test_base") colBase.Id = "_pbc_base_123" - colBase.Fields.Add(f1, f2, f3, f4, f5) + colBase.Fields.Add(f1, f2, f3, f4, f5, f6) colAuth := core.NewAuthCollection("test_auth") colAuth.Id = "_pbc_auth_123" - colAuth.Fields.Add(f1, f2, f3, f4, f5) + colAuth.Fields.Add(f1, f2, f3, f4, f5, f6) scenarios := []struct { name string @@ -1439,7 +1440,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { false, nil, nil, - `{"collectionId":"_pbc_base_123","collectionName":"test_base","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id"}`, + `{"collectionId":"_pbc_base_123","collectionName":"test_base","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field6":{"a":1,"a":2},"id":"test_id"}`, }, { "[base] with email visibility", @@ -1448,7 +1449,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { false, nil, nil, - `{"collectionId":"_pbc_base_123","collectionName":"test_base","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id"}`, + `{"collectionId":"_pbc_base_123","collectionName":"test_base","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field6":{"a":1,"a":2},"id":"test_id"}`, }, { "[base] with custom data", @@ -1457,7 +1458,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { true, nil, nil, - `{"collectionId":"_pbc_base_123","collectionName":"test_base","email":"test_email","emailVisibility":"test_invalid","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","password":"test_passwordHash","tokenKey":"test_tokenKey","unknown":"test_unknown","verified":true}`, + `{"collectionId":"_pbc_base_123","collectionName":"test_base","email":"test_email","emailVisibility":"test_invalid","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field6":{"a":1,"a":2},"id":"test_id","password":"test_passwordHash","tokenKey":"test_tokenKey","unknown":"test_unknown","verified":true}`, }, { "[base] with explicit hide and unhide fields", @@ -1466,7 +1467,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { true, []string{"field3", "field1", "expand", "collectionId", "collectionName", "email", "tokenKey", "unknown"}, []string{"field4", "@pbInternalAbc"}, - `{"emailVisibility":"test_invalid","field2":"field_2.png","field4":"field_4","id":"test_id","password":"test_passwordHash","verified":true}`, + `{"emailVisibility":"test_invalid","field2":"field_2.png","field4":"field_4","field6":{"a":1,"a":2},"id":"test_id","password":"test_passwordHash","verified":true}`, }, { "[base] trying to unhide custom fields without explicit WithCustomData", @@ -1475,7 +1476,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { true, nil, []string{"field5", "@pbInternalAbc", "email", "tokenKey", "unknown"}, - `{"collectionId":"_pbc_base_123","collectionName":"test_base","email":"test_email","emailVisibility":"test_invalid","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field5":"field_5","id":"test_id","password":"test_passwordHash","tokenKey":"test_tokenKey","unknown":"test_unknown","verified":true}`, + `{"collectionId":"_pbc_base_123","collectionName":"test_base","email":"test_email","emailVisibility":"test_invalid","expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field5":"field_5","field6":{"a":1,"a":2},"id":"test_id","password":"test_passwordHash","tokenKey":"test_tokenKey","unknown":"test_unknown","verified":true}`, }, // auth @@ -1486,7 +1487,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { false, nil, nil, - `{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","verified":true}`, + `{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field6":{"a":1,"a":2},"id":"test_id","verified":true}`, }, { "[auth] with email visibility", @@ -1495,7 +1496,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { false, nil, nil, - `{"collectionId":"_pbc_auth_123","collectionName":"test_auth","email":"test_email","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","verified":true}`, + `{"collectionId":"_pbc_auth_123","collectionName":"test_auth","email":"test_email","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field6":{"a":1,"a":2},"id":"test_id","verified":true}`, }, { "[auth] with custom data", @@ -1504,7 +1505,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { true, nil, nil, - `{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"id":"test_id","unknown":"test_unknown","verified":true}`, + `{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field6":{"a":1,"a":2},"id":"test_id","unknown":"test_unknown","verified":true}`, }, { "[auth] with explicit hide and unhide fields", @@ -1513,7 +1514,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { true, []string{"field3", "field1", "expand", "collectionId", "collectionName", "email", "unknown"}, []string{"field4", "@pbInternalAbc"}, - `{"emailVisibility":false,"field2":"field_2.png","field4":"field_4","id":"test_id","verified":true}`, + `{"emailVisibility":false,"field2":"field_2.png","field4":"field_4","field6":{"a":1,"a":2},"id":"test_id","verified":true}`, }, { "[auth] trying to unhide custom fields without explicit WithCustomData", @@ -1522,7 +1523,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { true, nil, []string{"field5", "@pbInternalAbc", "tokenKey", "unknown", "email"}, // emailVisibility:false has higher priority - `{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field5":"field_5","id":"test_id","unknown":"test_unknown","verified":true}`, + `{"collectionId":"_pbc_auth_123","collectionName":"test_auth","emailVisibility":false,"expand":{"test":123},"field1":"field_1�","field2":"field_2.png","field3":["test1","test2"],"field5":"field_5","field6":{"a":1,"a":2},"id":"test_id","unknown":"test_unknown","verified":true}`, }, } @@ -1533,6 +1534,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { "field3": []string{"test1", "test2"}, "field4": "field_4", "field5": "field_5", + "field6": types.JSONRaw(`{"a":1,"a":2}`), // intentionally duplicated to check serialization "expand": map[string]any{"test": 123}, "collectionId": "m_id", // should be always ignored "collectionName": "m_name", // should be always ignored @@ -1558,6 +1560,7 @@ func TestRecordPublicExportAndMarshalJSON(t *testing.T) { exportResult, err := json.Marshal( m.PublicExport(), json.Deterministic(true), + jsontext.AllowDuplicateNames(true), jsontext.AllowInvalidUTF8(true), ) if err != nil { diff --git a/tools/picker/pick.go b/tools/picker/pick.go index 84d584ab..887e151d 100644 --- a/tools/picker/pick.go +++ b/tools/picker/pick.go @@ -41,16 +41,19 @@ func Pick(data any, rawFields string) (any, error) { // Note that invalid UTF8 characters are mangled for compatibility // with earlier versions and to prevent unnecessary causing an error. // + // Duplicated keys are also enabled for just in case of old jsonv1 data + // (new db entries are usually validated against the jsonv2 semantics). + // // @todo research other approaches to avoid the double serialization // --- - encoded, err := json.Marshal(data, jsontext.AllowInvalidUTF8(true)) + encoded, err := json.Marshal(data, jsontext.AllowDuplicateNames(true), jsontext.AllowInvalidUTF8(true)) if err != nil { return nil, err } var decoded any - err = json.Unmarshal(encoded, &decoded) + err = json.Unmarshal(encoded, &decoded, jsontext.AllowDuplicateNames(true)) if err != nil { return nil, err } diff --git a/tools/picker/pick_test.go b/tools/picker/pick_test.go index 3d809d21..590c8415 100644 --- a/tools/picker/pick_test.go +++ b/tools/picker/pick_test.go @@ -1,6 +1,7 @@ package picker_test import ( + "encoding/json/jsontext" "encoding/json/v2" "errors" "testing" @@ -269,6 +270,13 @@ func TestPickFields(t *testing.T) { false, `{"id":"12","rel":{"title":"rel..."},"title":"lo"}`, }, + { + "jsonv1 values", + map[string]any{"a": "test\xc3", "b": jsontext.Value(`{"a":1,"a":2}`)}, + "a, b", + false, + `{"a":"test�","b":{"a":2}}`, + }, } for _, s := range scenarios { @@ -284,7 +292,12 @@ func TestPickFields(t *testing.T) { return } - serialized, err := json.Marshal(result, json.Deterministic(true)) + serialized, err := json.Marshal( + result, + json.Deterministic(true), + jsontext.AllowInvalidUTF8(true), + jsontext.AllowDuplicateNames(true), + ) if err != nil { t.Fatal(err) } diff --git a/tools/router/event.go b/tools/router/event.go index cc353965..5c00e4ca 100644 --- a/tools/router/event.go +++ b/tools/router/event.go @@ -206,7 +206,14 @@ func (e *Event) JSON(status int, data any) error { e.setResponseHeaderIfEmpty(headerContentType, "application/json") e.Response.WriteHeader(status) - return json.MarshalWrite(e.Response, data, jsontext.AllowInvalidUTF8(true)) + return json.MarshalWrite( + e.Response, + data, + // for compliance with old data (and slightly better performance) + jsontext.AllowDuplicateNames(true), + // preserve the old jsonv1 behavior in case of invalid data + jsontext.AllowInvalidUTF8(true), + ) } // XML writes an XML response. diff --git a/tools/router/event_test.go b/tools/router/event_test.go index 08ab716f..542ed98e 100644 --- a/tools/router/event_test.go +++ b/tools/router/event_test.go @@ -3,6 +3,7 @@ package router_test import ( "bytes" "crypto/tls" + "encoding/json/jsontext" "encoding/json/v2" "encoding/xml" "errors" @@ -21,6 +22,7 @@ import ( validation "github.com/pocketbase/ozzo-validation/v4" "github.com/pocketbase/pocketbase/tools/picker" "github.com/pocketbase/pocketbase/tools/router" + "github.com/pocketbase/pocketbase/tools/types" ) type unwrapTester struct { @@ -428,10 +430,11 @@ func TestEventJSON(t *testing.T) { "a": 123, "b": true, "c": "test", - "d": "\xc3", /* invalid utf8 char to test mangling */ + "d": "\xc3", // invalid utf8 char to test mangling + "e": types.JSONRaw(`{"a":1,"a":2}`), // duplicated keys to ensure compliance with old data } expectedPickedBody := `{"a":123,"c":"test"}` - expectedFullBody := `{"a":123,"b":true,"c":"test","d":"�"}` + expectedFullBody := `{"a":123,"b":true,"c":"test","d":"�","e":{"a":2}}` scenarios := []testResponseWriteScenario[any]{ { @@ -1074,9 +1077,9 @@ func testEventResponseWrite[T any]( // try to deserialize into a map and then serialize again in a // deterministic manner in case it is json var jsonBody any - err = json.Unmarshal(rawBody, &jsonBody) + err = json.Unmarshal(rawBody, &jsonBody, jsontext.AllowDuplicateNames(true)) if err == nil { - normalized, err := json.Marshal(jsonBody, json.Deterministic(true)) + normalized, err := json.Marshal(jsonBody, json.Deterministic(true), jsontext.AllowDuplicateNames(true)) if err != nil { t.Fatalf("Failed to deterministicly serialize json body\n%s\ngot\n%v", jsonBody, err) } diff --git a/tools/types/json_raw.go b/tools/types/json_raw.go index d228e41c..cb0ba093 100644 --- a/tools/types/json_raw.go +++ b/tools/types/json_raw.go @@ -7,6 +7,8 @@ import ( "errors" ) +// @todo consider deprecating in favour of jsontext.Value + // JSONRaw defines a json value type that is safe for db read/write. type JSONRaw []byte