New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unable to decode proto3 message from Retrofit's response error body #1056
Comments
I've looked into it and I can indeed repro the issue, what I'm getting is Note that I had to add |
Yeap it is indeed proto3 as I mentioned on the title, and the exception you get is the correct one if you are building from Let me know if I can help in any other way! |
Okay, so I tried decoding the hex using
which outputs the following:
If I'm reading this correctly, tag 5 contains an embedded message that in turn has what seems like boolean values for tags 5, 6, 9 and an int value for tag 7 - this looks consistent with the proto message above. That explains why decoding fails: when we're decoding bytes against the proto above we're expecting tag 5 to resolve to a boolean value, and the data suggests that it's something different. |
Hmm this should not be the case! The value in tag 5 is a boolean just like the rest of the |
Could you please recheck two things:
|
|
Re-checked with this online proto decoder: https://protogen.marcgravell.com/decode |
How is it possible that is decoded from the Google protobuf library though? 😕 If this is the case it should fail to decode in both libraries, right? Also this decoder is showing 2 different entries for |
@Egorand it seems that the following:
is the proper message payload that I would expect to be correctly decoded. So I am wondering where the following hex data |
That looks like a message prefix. 09 is the length. 2a will be the tag and
type.
…On Tue, Jul 9, 2019 at 2:17 PM Pavlos-Petros Tournaris < ***@***.***> wrote:
@Egorand <https://github.com/Egorand> it seems that the following:
$ echo "2801300138e8074801" | xxd -r -p | protoc --decode_raw
5: 1
6: 1
7: 1000
9: 1
is the proper message payload that I would expect to be correctly decoded.
So I am wondering where the following hex data 2a09 are added in the
stack.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1056?email_source=notifications&email_token=AAAQIEJPXJTVZUAAPRBFHX3P6TI4PA5CNFSM4H7EPC7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZRC3FI#issuecomment-509750677>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAQIEMPYUP3LZ77Y3BHEGDP6TI4PANCNFSM4H7EPC7A>
.
|
@JakeWharton indeed this is the case suggested by the online proto decoder mentioned by @Egorand : https://protogen.marcgravell.com/decode. I am not sure why the message is prefixed though. Is that some kind of protobuf message semantic? |
The following code seems to parse the message correctly, when removing the length prefix. fun String.hexStringToByteArray() = ByteArray(this.length / 2) { this.substring(it * 2, it * 2 + 2).toInt(16).toByte() }
val newBytes = "2801300138e8074801".hexStringToByteArray()
Subscription.ADAPTER.decode(newBytes) I suspect that the reason it does work with Google's protobuf library, is the usage of: https://developers.google.com/protocol-buffers/docs/reference/java/com/google/protobuf/Parser#parseDelimitedFrom-java.io.InputStream-
|
@swankjesse and I looked at it in more detail, and it turns out there's difference in how protoc and Wire parse the example you provided:
Although protoc does indeed succeed to parse the payload, since your schema is not consistent with the structure of the payload, you end up with an instance where all fields are initialized with |
Without trying to sound offensive, I am pretty sure that this is not the case for the Google protobuf. Let me explain why is that. I am using the values returned in this message in order to take show some different feedback based on some business logic. For the sake of the example let's state the following cases:
If all of the fields had If I though now drop the first two elements from the incoming Totally agree that this is a very hard to identify bug...! |
The schema you’re attempting to decode with is this:
But the correct schema for this message has an enclosing object:
You’re attempting to decode as a Subscription, but you should be attempting to decode as Box. |
Ok! Thank you all for your time and valuable help! Will take a look at this again with the backend engineers. Probably there is a change that we were not aware of and this value is now boxed like @swankjesse suggests! |
Yeah, the fact that protoc is so lenient is probably not a good thing, but in this case the only source of truth is code, and Wire's is different :) Please keep us posted on your findings! |
UpdateI did a more detailed investigation today 😄 It seems that you were totally right all along @Egorand & @swankjesse! The proto message is indeed coming in boxed in another proto message, since the backend packs the same response for both success and error cases (miscommunication issue right here). Let's assume that we have a message like:
this message includes the original Further down in the codepath, the value of one of Apparently this seems to be a testing issue after all, since the handling is not correct for the case of bad parsing, but we would have never been able to spot it if we did not move to Wire3 😄 I would like to thank you all once again! Precious investigation and taught me new things for Protobufs! |
Sweet! Glad that Wire helped you discover an issue. I'm gonna close this ticket as it seems like there's no action for us to take. |
Problem
Hey, I am trying to decode and create a new protobuf model, from the bytes located in Retrofit's
Response.errorBody()
.Reproducing
The way I am doing this, is get an
HttpException
with status code402
from our backend, containing a specific proto message in the error body.After accessing the
errorBody()
from theresponse
, I have the following:The above in hex has the following value:
2a092801300138e8074801
. Specifically I get a failure to decode the value in thebool e = 5;
of the following proto message. It resolves the value of40
when reading the next byte and fails to match that to a boolean inProtoAdapter.COMMON_BOOL
.The proto3 message
Dependencies versions
Wire-runtime:
3.0.0-alpha03
Wire Gradle plugin:
3.0.0-alpha03
Retrofit:
2.6.0
Okio:
2.3.0-SNAPSHOT
OkHttp:
4.0.0
The text was updated successfully, but these errors were encountered: