cel-java icon indicating copy to clipboard operation
cel-java copied to clipboard

`IllegalArgumentException: FieldDescriptor does not match message type` when using protobuf.Any

Open SanjayVas opened this issue 2 years ago • 3 comments

I get the following error when my var is a protobuf.Any message:

java.lang.IllegalArgumentException: FieldDescriptor does not match message type.

Stack trace:

com.google.protobuf.DynamicMessage.verifyContainingType(DynamicMessage.java:306)
	at com.google.protobuf.DynamicMessage.getField(DynamicMessage.java:195)
	at org.projectnessie.cel.common.types.pb.FieldDescription.getValueFromField(FieldDescription.java:447)
	at org.projectnessie.cel.common.types.pb.FieldDescription.getField(FieldDescription.java:438)
	at org.projectnessie.cel.common.types.pb.PbObjectT.get(PbObjectT.java:77)
	at org.projectnessie.cel.interpreter.AttributeFactory.refResolve(AttributeFactory.java:1222)
	at org.projectnessie.cel.interpreter.AttributeFactory$StringQualifier.qualify(AttributeFactory.java:840)
	at org.projectnessie.cel.interpreter.AttributeFactory$AbsoluteAttribute.tryResolve(AttributeFactory.java:363)
	at org.projectnessie.cel.interpreter.AttributeFactory$AbsoluteAttribute.resolve(AttributeFactory.java:341)
	at org.projectnessie.cel.interpreter.Interpretable$EvalAttr.eval(Interpretable.java:1698)
	at org.projectnessie.cel.interpreter.Interpretable$EvalBinary.eval(Interpretable.java:647)
	at org.projectnessie.cel.Prog.eval(Prog.java:89)

cel-java version: 0.3.11

Using a debugger at FieldDescription.getValueFromField, it appears that message is an instance of DynamicMessage with type.fullName equal to google.protobuf.Any. This has the correct type URL set. The FieldDescriptor has containingType.fullName equal to the concrete message type that the type URL resolves to.

I imagine that something is going wrong in unpacking. I've specified the message type in EnvOptions using a DynamicMessage from a Descriptor which was built from a FileDecriptorSet. In my test scenario, that message type also happens to be compiled in.

SanjayVas avatar Dec 09 '22 22:12 SanjayVas

This TODO may be relevant: https://github.com/projectnessie/cel-java/blob/1495cab2b5340d04d58b4e5848e0d0e922f0cedb/core/src/main/java/org/projectnessie/cel/common/types/pb/Db.java#L114

SanjayVas avatar Dec 09 '22 23:12 SanjayVas

So it appears there's actually two issues:

  1. If the protobuf message type has a compiled-in descriptor, that one must be used when building the Env (the TODO linked above).
  2. Nested google.protobuf.Any messages are not properly unpacked as per the CEL spec

The workaround for the (1) is to manually check against a list of known types. I don't know of any way to reflectively determine these types at the moment. The workaround for (2) is to unpack any google.protobuf.Any message instances passed to Program.eval, as well as registering those as separate variable declarations.

Example:

message Container {
  int64 foo = 1;

  message Contained {
    google.protobuf.Any metadata = 1;
  }
  Contained contained = 2;
}
message TestMetadata {
  uint32 age = 1;
}
val actualMessage: Container = // Initialized with contained.metadata set to a TestMetadata.
val typeRegistry: TypeRegistry = // Initialized with known Descriptors

val env =
  Env.newEnv(
    EnvOption.container("Container"),
    EnvOption.customTypeProvider(celTypeRegistry),
    EnvOption.customTypeAdapter(celTypeRegistry),
    EnvOption.declarations(Decls.newVar("foo", Checked.checkedInt), Decls.newVar("contained.metadata", Checked.checkedAny))
  )
val ast = env.compile("contained.metadata.age > 21").ast
val program = env.program(ast)

val variables: Map<String, Any> = mutableMapOf<String, Any>().apply {
  for ((fieldDescriptor, value) in actualMessage.allFields) {
    put(fieldDescriptor.name, value)
  }
  put("contained.metadata", DynamicMessage.parseFrom(typeRegistry.getDescriptorForTypeUrl(actualMessage.contained.metadata.typeUrl), actualMessage.contained.metadata.value))
}

SanjayVas avatar Dec 12 '22 20:12 SanjayVas

@SanjayVas : Thanks for the detailed analysis! We'll see what can be done. Still, if you have a particular fix in mind please feel free to open a PR.

dimas-b avatar Dec 13 '22 14:12 dimas-b