protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

Java/C++/Python implementation inconsistent with proto json spec when marshaling empty proto in any proto

Open lfolger opened this issue 1 year ago • 16 comments

See https://github.com/golang/protobuf/issues/1620 for full context.

The protobuf Go implementation gerates a different output and expects a different input when converting a google.protobuf.Empty proto nested in and google.protobuf.Any proto to and from JSON compared to C++, Java and Python.

According to the discussion on the issue, the Go protobuf implementation seems to be consistent with the spec but inconsistent with other languages.

Copying the relevant parts from the other issue:

(not using quotes here to keep the original formatting): start quote from dsnet:

The documentation for google.protobuf.Any.value says:

[The value field] must be a valid serialized protocol buffer of the above specified type.

Elsewhere, the documentation for google.protobuf.Any says:

If the embedded message type is well-known and has a custom JSON representation, that representation will be embedded adding a field value which holds the custom JSON in addition to the @type field.

Then, the documentation for google.protobuf.Empty says:

The JSON representation for Empty is empty JSON object {}.

Thus, it seems that the current behavior is correct.

end quote

Is our interpretation/understanding of the documentation incorrect? Would it be reasonable to adjust the documentation to reflect the behavior of the C++/Java/Python? Alternatively, could the behavior of C++/Java be adjusted to align with the documentation? I assume this is cost prohibitive and even aligning the Go implementation with C++ and Java might not be easy.

lfolger avatar Jun 11 '24 14:06 lfolger

Maybe worth updating docs, depending on your bandwidth, of course @Logofile

ref: https://github.com/protocolbuffers/protobuf/issues/5390

honglooker avatar Jun 11 '24 22:06 honglooker

Given the inconsistencies and the fact that Java/C++ don't accept the currently documented formatting, would it be reasonable to make all parsers accept both formattings?

C++ seems to fail if I try to feed it {"@type":"type.googleapis.com/google.protobuf.Empty","value":{}}.

lfolger avatar Jun 12 '24 09:06 lfolger

I think the only rational answer is that all parsers should accept both formats—for historical reasons. The only question is what should we align on the “canonical” preferred form.

puellanivis avatar Jun 12 '24 19:06 puellanivis

Hi,

Is there any plan to address the inconsistency with how empty proto messages are handled across different languages (C++, Java, Python)? Consistency here is important for reliable cross-language usage.

Thanks!

orloffv avatar Jul 16 '24 08:07 orloffv

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Oct 14 '24 10:10 github-actions[bot]

It seems like Java/C++/Python might be following the otherwise statement in the JSON mapping?

Otherwise, the value will be converted into a JSON object, and the "@type" field will be inserted to indicate the actual data type.

That is, it renders the Empty to an empty JSON object, and then inserts the "@type" field.

puellanivis avatar Oct 14 '24 11:10 puellanivis

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Jan 13 '25 10:01 github-actions[bot]

The “accept” of the Golang sides has been released for go protobufs since late November.

Is there any change we need to be making for the “produce” interest? I checked the thread for the greater protobuf project, and I’m not sure I see anything that says that we’ve settled on what the expected output should be.

I suppose at least for now, we should be just leaving producers the way they are now that I think we at least have compatibility? That is, since dropping the value field could still break clients connecting to a newer protobuf version if we change the format. We would have to come at least to a point where any non-accepting acceptor would be end-of-maintenance, and perhaps even for a bit of a buffer after as well.

puellanivis avatar Jan 13 '25 19:01 puellanivis

@puellanivis , thanks for digging into things to check the status, and for keeping this issue open by replying. I'll see if I can work on updating the docs to accurately capture the differing behavior between languages.

Logofile avatar Jan 13 '25 19:01 Logofile

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Apr 14 '25 10:04 github-actions[bot]

I believe there has still not been a clarification on this issue in the standards?

puellanivis avatar Apr 14 '25 11:04 puellanivis

Recently ran into this issue - Java and Go JSON parsing behaves differently in the case of an Any proto which contains an Empty, meaning there is no single format for this case that is successfully parsed for both parsers/unmarshalers. Commenting to ensure this issue remains active.

Are there plans to bring the Java json parser into alignment with the spec?

ajbradberry96 avatar Jul 11 '25 19:07 ajbradberry96

¿hm? The Go implementation was changed to accept both the JSON produced by C++/Java, and Go. So, the C++/Java version should be a “single format for this case that is successfully parsed for both parsers/unmarshalers”. It is just that the Go implementation does not produce this format, since it needs to keep backwards compat with its own users.

PS: It was merely our opinions that Go was the correct implementation of the spec. No such actual determination had been made by the wider community (which would have happened here in this thread).

puellanivis avatar Jul 12 '25 11:07 puellanivis

the C++/Java version should be a “single format for this case that is successfully parsed for both parsers/unmarshalers”.

To the best of my understanding, there isn't a single format accepted by all parsers -

Take:

{
    "anyField1": {
        "@type": "type.googleapis.com/google.protobuf.Empty",
        "value": {}
    }
}

Which is accepted happily by the Go unmarshaller, but fails in Java with com.google.protobuf.InvalidProtocolBufferException: Cannot find field: value in message google.protobuf.Empty. If we remove the value field,

{
    "anyField1": {
        "@type": "type.googleapis.com/google.protobuf.Empty"
    }
}

the Java parser is happy, but it fails in Go with code = InvalidArgument desc = Any JSON doesn't have 'value'

I've tried several different variations of throwing in nulls and empty objects all over, but can't seem to get a request to work for both. Am I missing something? I would love to hear that I am hahaha.

No such actual determination had been made by the wider community

Apologies for the overstatement, hopefully just being a squeaky wheel will get some movement on that front :)

ajbradberry96 avatar Jul 16 '25 16:07 ajbradberry96

the Java parser is happy, but it fails in Go with code = InvalidArgument desc = Any JSON doesn't have 'value'

https://go.dev/play/p/gnuMdrqLt6y

Please ensure that you are using google.golang.org/protobuf v1.35.2 or later.

puellanivis avatar Jul 17 '25 15:07 puellanivis

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Oct 16 '25 10:10 github-actions[bot]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please reopen it.

This issue was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

github-actions[bot] avatar Oct 31 '25 10:10 github-actions[bot]