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

[BUG] CEL-Java is not fully compatible with Protobuf V4

Open bugs84 opened this issue 1 year ago • 4 comments

Describe the bug ConstantFoldingOptimizer doesn't work for some expressions. e.g. "my_var in ['H', 'O']"

To Reproduce Clone this repository, that reproduce the bug. https://github.com/bugs84/CelOptimizerIssue

Current issue Program ends with following exception:

Exception in thread "main" java.lang.NoSuchMethodError: 'com.google.protobuf.Internal$LongList dev.cel.expr.UnknownSet.mutableCopy(com.google.protobuf.Internal$LongList)'
	at dev.cel.expr.UnknownSet.access$600(UnknownSet.java:14)
	at dev.cel.expr.UnknownSet$Builder.ensureExprsIsMutable(UnknownSet.java:456)
	at dev.cel.expr.UnknownSet$Builder.addAllExprs(UnknownSet.java:539)
	at dev.cel.runtime.InterpreterUtil.createUnknownExprValue(InterpreterUtil.java:95)
	at dev.cel.runtime.InterpreterUtil.createUnknownExprValue(InterpreterUtil.java:84)
	at dev.cel.runtime.InterpreterUtil.valueOrUnknown(InterpreterUtil.java:146)
	at dev.cel.runtime.RuntimeUnknownResolver.resolveSimpleName(RuntimeUnknownResolver.java:114)
	at dev.cel.runtime.DefaultInterpreter$ExecutionFrame.resolveSimpleName(DefaultInterpreter.java:975)
	at dev.cel.runtime.DefaultInterpreter$ExecutionFrame.access$200(DefaultInterpreter.java:936)
	at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.resolveIdent(DefaultInterpreter.java:274)
	at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.evalIdent(DefaultInterpreter.java:262)
	at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.evalInternal(DefaultInterpreter.java:193)
	at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.evalCall(DefaultInterpreter.java:387)
	at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.evalInternal(DefaultInterpreter.java:199)
	at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.evalTrackingUnknowns(DefaultInterpreter.java:179)
	at dev.cel.runtime.DefaultInterpreter$DefaultInterpretable.eval(DefaultInterpreter.java:170)
	at dev.cel.runtime.CelRuntime$Program.evalInternal(CelRuntime.java:146)
	at dev.cel.runtime.CelRuntime$Program.evalInternal(CelRuntime.java:121)
	at dev.cel.runtime.CelRuntime$Program.evalInternal(CelRuntime.java:116)
	at dev.cel.runtime.CelRuntime$Program.eval(CelRuntime.java:49)
	at dev.cel.common.ast.CelExprUtil.evaluateExpr(CelExprUtil.java:42)
	at dev.cel.optimizer.optimizers.ConstantFoldingOptimizer.maybeFold(ConstantFoldingOptimizer.java:251)
	at dev.cel.optimizer.optimizers.ConstantFoldingOptimizer.optimize(ConstantFoldingOptimizer.java:113)
	at dev.cel.optimizer.CelOptimizerImpl.optimize(CelOptimizerImpl.java:45)
	at HelloWorld.run(HelloWorld.java:62)
	at HelloWorld.main(HelloWorld.java:73)

bugs84 avatar Oct 03 '24 13:10 bugs84

I was able to reproduce this. My suspicion is that something about the protobuf upgrade to 4.28.0 is causing this issue, and that the binary compatibility may not have been fully restored as stated in https://github.com/protocolbuffers/protobuf/issues/17247. Either that, or the dependent protos (ExprValue from proto-google-common-protos, used to represent unknowns in CEL) is not fully protobuf v4 compatible.

The workaround is to downgrade CEL to 0.6.0 as of now. I'll look into this further.

l46kok avatar Oct 03 '24 21:10 l46kok

This is an issue on any code path in CEL that references the Builder's methods on protobuf messages. The earlier release of Protobuf v4 have removed GeneratedMessageV3, and shims were added afterwards to restore ABI compatibility in 4.27.x, but not fully. Particularly, any generated messages produced before protoc version 3.25.x are no longer compatible with Protobuf V4. Bazel's built-in proto toolchain (@com_google_protobuf//:protoc) happens to use 3.21. This unfortunately led to publishing a JAR with incompatible generated messages.

In short, CEL is not compatible with protobuf v4. The published JAR for 0.7.1 forces protobuf v4 as a dependency so it should not be used. I'll try to release a fix soon.

l46kok avatar Oct 04 '24 07:10 l46kok

Thank you.

Just note: Downgrade to CEL 0.6.0 - has different issue NoClassDefFoundError: com/google/rpc/StatusProto But it doesn't matter. For us is now enough to comment out ConstantFoldingOptimizer. And wait for proper fix. Thank you again!

bugs84 avatar Oct 04 '24 07:10 bugs84

Oh for that one, you just need to bring in proto-google-common-protos from maven. Not sure why that's not being resolved automatically in gradle, but that's a separate issue.

Btw, the issue in 0.7.1 is not just specific to constant folding. You'll run into issues in other places where an unknown value needs to be referenced at all internally.

l46kok avatar Oct 04 '24 07:10 l46kok

@bugs84 The latest release 0.8.0 should work now. Thank you for reporting.

l46kok avatar Oct 10 '24 18:10 l46kok

Thank you. I confirm, that sample project with cel 0.8.0 and com.google.api.grpc:proto-google-common-protos:2.46.0 works.

bugs84 avatar Oct 14 '24 07:10 bugs84