scalapb / ScalaPB

Protocol buffer compiler for Scala.
https://scalapb.github.io/
Apache License 2.0
1.3k stars 278 forks source link

ArrayIndexOutOfBoundsException on getFieldByNumber #1669

Closed honzastrnads1 closed 5 months ago

honzastrnads1 commented 6 months ago

An ArrayIndexOutOfBoundsException is thrown when getFieldByNumber is invoked on a field that represents unrecognized enum value.

The stack trace is:

Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 4
    at java.base/java.util.Arrays$ArrayList.get(Arrays.java:4165)
    at java.base/java.util.Collections$UnmodifiableList.get(Collections.java:1347)
    at scalapb.GeneratedEnum.javaValueDescriptor(GeneratedMessageCompanion.scala:29)
    at scalapb.GeneratedEnum.javaValueDescriptor$(GeneratedMessageCompanion.scala:28)
    at Msg$Status.javaValueDescriptor(Tmp.scala:35)
    at Msg.getFieldByNumber(Tmp.scala:18)
    at Test$.delayedEndpoint$Test$1(Tmp.scala:80)
    at Test$delayedInit$body.apply(Tmp.scala:78)
    at scala.Function0.apply$mcV$sp(Function0.scala:42)
    at scala.Function0.apply$mcV$sp$(Function0.scala:42)
    at scala.runtime.AbstractFunction0.apply$mcV$sp(AbstractFunction0.scala:17)
    at scala.App.$anonfun$main$1(App.scala:98)
    at scala.App.$anonfun$main$1$adapted(App.scala:98)
    at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:575)
    at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:573)
    at scala.collection.AbstractIterable.foreach(Iterable.scala:933)
    at scala.App.main(App.scala:98)
    at scala.App.main$(App.scala:96)
    at Test$.main(Tmp.scala:78)
    at Test.main(Tmp.scala)

Following protobuf definiton should be enough to reproduce:

message Msg {
  enum Status {
    UNKNOWN = 0;
    OK = 1;
  }
}

... and the code that triggers the exception:

object Test extends App {
  val msg = Msg(Msg.Status.Unrecognized(42))
  msg.getFieldByNumber(7)
}

Version used: 0.11.12.

thesamet commented 6 months ago

Assuming the message also contains Status status = 7 above.

Confirming the behavior you are seeing. There isn't a straightforward fix because as far as I can tell there's no public method in the Java protobuf API to instantiate an EnumValueDescriptor that represents an unknown enum. Is it possible for your code to use getField instead of getFieldByNumber at least for enums?

honzastrnads1 commented 6 months ago

Assuming the message also contains Status status = 7 above.

Yes, sorry for that.

Confirming the behavior you are seeing. There isn't a straightforward fix because as far as I can tell there's no public method in the Java protobuf API to instantiate an EnumValueDescriptor that represents an unknown enum. Is it possible for your code to use getField instead of getFieldByNumber at least for enums?

No, it's not easily possible to substitute and I don't think it would help either. We would race into the same issue, just on a different place, specifically here.

thesamet commented 6 months ago

Unrecognized enums override scalaValueDescriptor to give a descriptor that represents the unknown value, see here

thesamet commented 6 months ago

Since the original issue (on getFieldByNumber) doesn't have a reasonable fix there isn't much that can be done to address it (I consider deprecating the method in a future release as it predates the PValue API used by getField). I wanted to check in if getField provides a reasonable alternative or you have any other suggestion.

honzastrnads1 commented 5 months ago

Unfortunately migrating to getField would require quite a few changes on our side, so it's a no go for us at the moment.