Skip to content

Support reserved names in ProtoJSON deserialisers #26123

@Grimeh

Description

@Grimeh

What language does this apply to?
My particular use case is in Go but it applies to all languages that implement ProtoJSON.


Describe the problem you are trying to solve.
Currently if a field is removed from a message, this is a wire-safe change for the binary serialiser (the field will be discarded by the deserialiser) but will trigger a parsing error for ProtoJSON.

That is, unless the "discard/ignore unknown" deserialiser option is enabled. However this will cause ALL unknown fields to be silently discarded. Which is a shame because the default behaviour is still desirable for truly unknown fields.

It's already possible to reserve field names with the reserve keyword, and the docs state that TextProto implementations may recognise this and discard unknown fields if it's a reserved name. The same docs also state "Runtime JSON parsing is not affected by reserved names.".


Describe the solution you'd like
It would be ideal if ProtoJSON implementations respected reserved names, by discarding unknown fields if they match a reserved name. This way it's possible to remove a field and have it safely deserialised by both binary and json implementations, as long as the field name is reserved. Truly unknown fields (not defined or reserved) still result in a parse error by default, catching actual errors ASAP.

NOTE: I am not requesting that unknown fields are preserved by ProtoJSON impls. The lack of preservation of reserved fields is completely acceptable (and even desirable imo).


Describe alternatives you've considered
The alternative (more of a workaround) is to enable the "discard unknown fields" ProtoJSON deserialiser option everywhere that could be affected by a removed field in the future (which is pretty much everywhere). As mentioned above this is bit of a sledgehammer fix and significantly weakens the safety checks of the deserialiser, which I'd like to avoid if at all possible.

Another alternative is to leave the field in the message definition and change the field doc comment to something like "OBSOLETE DO NOT USE", however this is confusing and easy to miss.


Additional context
Example:

Version 1

message MyMessage {
    string name = 1;
    string city = 2;
}

Version 2

message MyMessage {
    reserved 2;
    reserved city;

    string name = 1;
}

This is a wire-safe change for the binary deserialiser, but it will cause the ProtoJSON deserialiser to fail with a unknown field "city" error. Ideally the "city": "my city" field would be discarded since the name is reserved (and thus "known").

Previous discussions around this (eg. #3440) mostly centred around the discrepancy between the binary & json deserialisation with default settings. I couldn't find an issue specifically around accounting for reserved names in ProtoJSON deserialisation.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementuntriagedauto added to all issues by default when created.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions