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

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

Open
SanjayVas opened this issue Dec 9, 2022 · 3 comments

Comments

@SanjayVas
Copy link

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
Copy link
Author

This TODO may be relevant:

// TODO is there something like this in Java ??

@SanjayVas
Copy link
Author

SanjayVas commented Dec 12, 2022

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))
}

@dimas-b
Copy link
Member

dimas-b commented Dec 13, 2022

@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.

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