Add data_base64 support and simplify CE JSON marshaling#39
Add data_base64 support and simplify CE JSON marshaling#39
Conversation
- Replace headerToMap with sjson for CE JSON marshaling - Return unmarshal errors instead of silently discarding them - Guard payloadFunc call when no data field is present - Use value receivers for RawEvent MarshalJSON/BytesForSignature - Reject events with both "data" and "data_base64" per CE spec - Add negative test cases for error paths Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the CloudEvents JSON handling to support the CloudEvents JSON spec’s data_base64 field and standardizes marshaling/unmarshaling behavior across CloudEventHeader, CloudEvent[A], and the new standalone RawEvent.
Changes:
- Introduces a standalone
RawEventstruct with custom JSON marshal/unmarshal supportingdataanddata_base64, including mutual exclusivity checks. - Refactors CloudEvent unmarshaling into a single-pass
unmarshalCloudEventWithPayloadand stops swallowing per-field unmarshal errors. - Replaces header serialization helpers with
sjson-based field injection and adds new negative test cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cloudevent.go | Adds RawEvent struct, data_base64 support, content-type based wire-form selection, and helper methods. |
| cloudevent_json.go | Refactors JSON unmarshaling flow (single-pass map decode + per-field parsing) and standardizes marshaling via sjson. |
| cloudevent_test.go | Adds negative tests for invalid base64, mutually exclusive fields, invalid time type, and missing data field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var dataRaw json.RawMessage | ||
| var dataBase64 string | ||
| if raw, ok := rawFields["data_base64"]; ok && len(raw) > 0 { | ||
| if err := json.Unmarshal(raw, &dataBase64); err != nil { | ||
| return c, err | ||
| } | ||
| } | ||
| if raw, ok := rawFields["data"]; ok { | ||
| dataRaw = raw | ||
| } | ||
| if dataRaw != nil || dataBase64 != "" { | ||
| if err := payloadFunc(dataRaw, dataBase64); err != nil { | ||
| return c, err | ||
| } | ||
| } |
There was a problem hiding this comment.
unmarshalCloudEventWithPayload treats data_base64 as present only when the decoded string is non-empty. If the JSON contains an explicit empty string ("data_base64":"") this will be ignored: payloadFunc won’t run, RawEvent won’t see the field, and the mutual-exclusion check can be bypassed. Track presence separately (e.g., hasDataBase64 := ok), and call payloadFunc / enforce exclusivity based on key presence rather than dataBase64 != "".
| if dataRaw != nil && dataBase64 != "" { | ||
| return fmt.Errorf("cloudevent: both \"data\" and \"data_base64\" present; only one allowed") | ||
| } | ||
| if dataBase64 != "" { | ||
| decoded, err := base64.StdEncoding.DecodeString(dataBase64) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| r.Data = decoded | ||
| r.DataBase64 = dataBase64 | ||
| } else { | ||
| r.Data = dataRaw | ||
| r.DataBase64 = "" | ||
| } |
There was a problem hiding this comment.
RawEvent.UnmarshalJSON uses dataBase64 != "" to decide whether data_base64 was provided. This loses information (and skips base64 decoding) when the input contains an explicit empty base64 payload ("data_base64":""), and also won’t error if both data and an empty data_base64 are present. Consider propagating a separate “field present” boolean from unmarshalCloudEventWithPayload and basing the branch + mutual-exclusion check on presence rather than non-empty string.
| if len(r.Data) > 0 || r.DataBase64 != "" { | ||
| if r.DataBase64 != "" { | ||
| data, err = sjson.SetBytes(data, "data_base64", r.DataBase64) | ||
| } else if IsJSONDataContentType(r.DataContentType) || (r.DataContentType == "" && json.Valid(r.Data)) { |
There was a problem hiding this comment.
RawEvent.MarshalJSON will emit data as raw JSON whenever DataContentType indicates JSON, without verifying that r.Data is valid JSON. If DataContentType is set to application/json but r.Data contains non-JSON bytes (e.g., decoded from data_base64), this will produce invalid JSON output. Guard the SetRawBytes("data", ...) path with json.Valid(r.Data) (and otherwise fall back to data_base64 or return an error).
| } else if IsJSONDataContentType(r.DataContentType) || (r.DataContentType == "" && json.Valid(r.Data)) { | |
| } else if json.Valid(r.Data) && (IsJSONDataContentType(r.DataContentType) || r.DataContentType == "") { |
| func TestRawEvent_UnmarshalJSON_InvalidBase64(t *testing.T) { | ||
| t.Parallel() | ||
| jsonStr := `{"id":"1","source":"s","type":"t","data_base64":"$$not-base64$$"}` | ||
| var ev cloudevent.RawEvent | ||
| err := json.Unmarshal([]byte(jsonStr), &ev) | ||
| require.Error(t, err, "expected error for invalid base64 in data_base64") | ||
| } | ||
|
|
||
| func TestRawEvent_UnmarshalJSON_BothDataAndDataBase64(t *testing.T) { | ||
| t.Parallel() | ||
| jsonStr := `{"id":"1","source":"s","type":"t","data":{"x":1},"data_base64":"Zm9v"}` | ||
| var ev cloudevent.RawEvent | ||
| err := json.Unmarshal([]byte(jsonStr), &ev) | ||
| require.Error(t, err, "expected error when both data and data_base64 are present") | ||
| assert.Contains(t, err.Error(), "both") | ||
| } |
There was a problem hiding this comment.
The new RawEvent behavior is only covered by negative tests. There are no tests exercising successful unmarshal/marshal round-trips for data_base64 (including the MIME-based selection logic in RawEvent.MarshalJSON). Adding a positive test that unmarshals an event with data_base64, then marshals it back (and asserts the field is preserved and mutually-exclusive behavior holds) would help prevent regressions.
1 participant
Summary
data_base64support toRawEvent:RawEventis now a standalone struct (no longer a type alias) with customMarshalJSON/UnmarshalJSONthat handles both"data"and"data_base64"per the CloudEvents JSON spec, including round-trip preservation and MIME-based wire form selection.headerToMaphelper withsjsonfor consistent header serialization acrossCloudEventHeader,CloudEvent[A], andRawEvent.json.Unmarshalcalls inunmarshalCloudEventWithPayloadnow return errors instead of discarding them.payloadFuncis only called when"data"or"data_base64"is present, soCloudEvent[A]no longer rejects valid events without a data field.RawEvent.MarshalJSONandBytesForSignaturesojson.Marshal(value)works correctly without requiring a pointer.RawEvent.UnmarshalJSONreturns an error if both"data"and"data_base64"are present.Breaking changes
RawEventchanged fromtype RawEvent = CloudEvent[json.RawMessage]to a standalone struct. Callers constructingRawEventviaCloudEvent[json.RawMessage]{...}will need to update.Test plan
Made with Cursor