square / wire

gRPC and protocol buffers for Android, Kotlin, Swift and Java.
https://square.github.io/wire/
Apache License 2.0
4.24k stars 570 forks source link

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

Open DoumanAsh opened 2 days ago

DoumanAsh commented 2 days ago

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 commented 2 days ago

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 commented 2 days ago

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 commented 2 days ago

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 commented 1 day ago

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 commented 1 day ago

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