Skip to content

Commit

Permalink
Discard unknown JSON fields by default (#518)
Browse files Browse the repository at this point in the history
The current JSON unmarshaling logic tightly couples clients and servers: clients can't unmarshal messages containing new fields, and servers can't unmarshal messages containing ancient fields that have since been deleted. This is counter to the typical protobuf forward- and backward-compatibility story, and is far more restrictive than the binary codec. This PR loosens the default behavior to discard unknown fields.

Fixes #477.
  • Loading branch information
akshayjshah authored Jun 5, 2023
1 parent aaa215c commit c99bd5a
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 9 deletions.
4 changes: 3 additions & 1 deletion codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ func (c *protoJSONCodec) Unmarshal(binary []byte, message any) error {
if len(binary) == 0 {
return errors.New("zero-length payload is not a valid JSON object")
}
var options protojson.UnmarshalOptions
// Discard unknown fields so clients and servers aren't forced to always use
// exactly the same version of the schema.
options := protojson.UnmarshalOptions{DiscardUnknown: true}
return options.Unmarshal(binary, protoMessage)
}

Expand Down
31 changes: 23 additions & 8 deletions codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,28 @@ func TestStableCodec(t *testing.T) {
func TestJSONCodec(t *testing.T) {
t.Parallel()

var empty emptypb.Empty
codec := &protoJSONCodec{name: "json"}
err := codec.Unmarshal([]byte{}, &empty)
assert.NotNil(t, err)
assert.True(
t,
strings.Contains(err.Error(), "valid JSON"),
assert.Sprintf(`error message should explain that "" is not a valid JSON object`),
)

t.Run("success", func(t *testing.T) {
t.Parallel()
err := codec.Unmarshal([]byte("{}"), &emptypb.Empty{})
assert.Nil(t, err)
})

t.Run("unknown fields", func(t *testing.T) {
t.Parallel()
err := codec.Unmarshal([]byte(`{"foo": "bar"}`), &emptypb.Empty{})
assert.Nil(t, err)
})

t.Run("empty string", func(t *testing.T) {
t.Parallel()
err := codec.Unmarshal([]byte{}, &emptypb.Empty{})
assert.NotNil(t, err)
assert.True(
t,
strings.Contains(err.Error(), "valid JSON"),
assert.Sprintf(`error message should explain that "" is not a valid JSON object`),
)
})
}

0 comments on commit c99bd5a

Please sign in to comment.