Skip to content
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

Map Adapter cannot handle Value whose representation can be zero bytes #3153

Open
DoumanAsh opened this issue Oct 23, 2024 · 5 comments
Open

Comments

@DoumanAsh
Copy link

DoumanAsh commented Oct 23, 2024

Subj

If you use message like:

message ValidZeroPayload {
    float distance_meters = 2;
    reserved 1;
}

Wire will choke on parsing map with value initialized as:

ValidZeroPayload {
    distance_meters: 0.0
}

That's because it will be encoded as zero bytes due to zero being treated as non-presence in case of integers and floats

Therefore it triggers this check to fail:
https://github.com/square/wire/blob/master/wire-runtime/src/commonMain/kotlin/com/squareup/wire/ProtoAdapter.kt#L812
Even though message itself is legit in protobuf 3

@oldergod
Copy link
Member

Thanks for reporting. Would you be able to write a repro situation in https://github.com/square/wire/tree/master/wire-gradle-plugin-playground/src/main for instance? You would add the proto file in the main/proto/ folder, ./gradlew generateProtos and write code in main/java/ which would throw as your pointed?
That'd help us very much in fixing it.

@DoumanAsh
Copy link
Author

DoumanAsh commented Oct 23, 2024

Sure, I will take a look this weekend, but looking at main it seems there is none of such tests exists
Is it more of setup to simulate legit server?

@oldergod
Copy link
Member

No need to set up a test, you can write a main method and simply reproduce the crash. You can then copy/paste here the .proto and the .kt or .java file.

Something like

fun main() {
  val bytes = // the payload you mentioned in bytes or in JSON string if that's the problem
  MyGeneratedClass.ADAPTER.decode(bytes) // this would throw
}

To get the bytes you could also do something like MyGeneratedClass(distance_meters=0.0).encodeByteString() and then pass this.

@DoumanAsh
Copy link
Author

Ok I tried to make example in pure kotlin with a very simple message


message ValidZeroMessage {
  float meters = 2;
  reserved 1;
}

message MessageWithZeroValueMap {
  map<string, ValidZeroMessage> values = 1;
}

But it seems wire correctly serializes it omitting message and then deserializes it using following code:

import human.MessageWithZeroValueMap
import human.ValidZeroMessage

fun main() {
  println("Show that message itself is valid!")
  val zero_message = ValidZeroMessage.Builder().meters(0.0f).build()
  val zero_message_encoded = zero_message.encodeByteString()

  println("Null message=$zero_message_encoded")

  val decoded_zero_message = ValidZeroMessage.ADAPTER.decode(zero_message_encoded)
  println("Decoded null message=$decoded_zero_message")

  println("Now try to put it into map")
  val values = mapOf("1" to decoded_zero_message)
  val msg_map = MessageWithZeroValueMap.Builder().values(values).build()

  println("Map with null message=$msg_map")

  val msg_map_encoded = msg_map.encodeByteString()
  println("Encoded map with null message=$msg_map_encoded")

  val decoded_msg_map = MessageWithZeroValueMap.ADAPTER.decode(msg_map_encoded)
  println("Decoded map message=$decoded_msg_map")
}

I need to check out my example from work once again to see what could be missing
But it seems to be working correctly in this simple example, I'm not sure why a more complex message would fail but oh well
Give me some time to ponder it

@oldergod
Copy link
Member

@DoumanAsh all good. Thanks a lot for looking into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants