From 8d73112c3c1d3f9bf033c667dcb68affa140d71a Mon Sep 17 00:00:00 2001 From: Sean Huh Date: Tue, 28 Jul 2026 00:38:18 -0700 Subject: [PATCH] Fix conformance test case around receiver function names containing reserved keywords for parsed-only case PiperOrigin-RevId: 955083744 --- .../dev/cel/common/internal/ProtoAdapter.java | 26 ++++++------- .../cel/common/internal/ProtoTimeUtils.java | 4 ++ .../test/java/dev/cel/conformance/BUILD.bazel | 22 ++--------- .../java/dev/cel/runtime/RuntimeHelpers.java | 2 +- .../cel/runtime/planner/ProgramPlanner.java | 38 +++++++++++++------ .../runtime/standard/SubtractOperator.java | 21 +++++++++- 6 files changed, 65 insertions(+), 48 deletions(-) diff --git a/common/src/main/java/dev/cel/common/internal/ProtoAdapter.java b/common/src/main/java/dev/cel/common/internal/ProtoAdapter.java index 7e3910433..b1b56afe1 100644 --- a/common/src/main/java/dev/cel/common/internal/ProtoAdapter.java +++ b/common/src/main/java/dev/cel/common/internal/ProtoAdapter.java @@ -325,13 +325,6 @@ private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) { value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value))); case FLOAT: return unwrapAndConvert(DOUBLE_CONVERTER); - case DOUBLE: - case SFIXED64: - case SINT64: - case INT64: - return BidiConverter.of( - BidiConverter.IDENTITY.forwardConverter(), - value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value))); case BYTES: if (celOptions.evaluateCanonicalTypesToNativeValues()) { return BidiConverter.of( @@ -342,10 +335,11 @@ private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) { return BidiConverter.of( BidiConverter.IDENTITY.forwardConverter(), value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value))); + case DOUBLE: + case SFIXED64: + case SINT64: + case INT64: case STRING: - return BidiConverter.of( - BidiConverter.IDENTITY.forwardConverter(), - value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value))); case BOOL: return BidiConverter.of( BidiConverter.IDENTITY.forwardConverter(), @@ -353,10 +347,14 @@ private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) { case ENUM: return BidiConverter.of( value -> (long) ((EnumValueDescriptor) value).getNumber(), - number -> - fieldDescriptor - .getEnumType() - .findValueByNumberCreatingIfUnknown(number.intValue())); + number -> { + if (number > Integer.MAX_VALUE || number < Integer.MIN_VALUE) { + throw new IllegalArgumentException("Enum value out of int32 range: " + number); + } + return fieldDescriptor + .getEnumType() + .findValueByNumberCreatingIfUnknown(number.intValue()); + }); case MESSAGE: return BidiConverter.of( this::adaptProtoToValue, diff --git a/common/src/main/java/dev/cel/common/internal/ProtoTimeUtils.java b/common/src/main/java/dev/cel/common/internal/ProtoTimeUtils.java index 36671842d..2e4dd9185 100644 --- a/common/src/main/java/dev/cel/common/internal/ProtoTimeUtils.java +++ b/common/src/main/java/dev/cel/common/internal/ProtoTimeUtils.java @@ -406,6 +406,10 @@ public static Duration between(Timestamp from, Timestamp to) { Instant javaTo = ProtoTimeUtils.toJavaInstant(checkValid(to)); java.time.Duration between = java.time.Duration.between(javaFrom, javaTo); + // Call toNanos() to validate 64-bit nanosecond overflow (throws ArithmeticException). + // Suppress unused variable warning as the duration object itself is returned. + @SuppressWarnings("unused") + long unused = between.toNanos(); return ProtoTimeUtils.toProtoDuration(between); } diff --git a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel index e9ed58642..920f226c2 100644 --- a/conformance/src/test/java/dev/cel/conformance/BUILD.bazel +++ b/conformance/src/test/java/dev/cel/conformance/BUILD.bazel @@ -108,12 +108,7 @@ _TESTS_TO_SKIP_LEGACY = [ # TODO: Support setting / getting enum values out of the defined enum value range. "enums/legacy_proto2/select_big,select_neg", "enums/legacy_proto2/assign_standalone_int_big,assign_standalone_int_neg", - # TODO: Generate errors on enum value assignment overflows for proto3. - "enums/legacy_proto3/assign_standalone_int_too_big,assign_standalone_int_too_neg", - # TODO: Ensure overflow occurs on conversions of double values which might not work properly on all platforms. - "conversions/int/double_int_min_range", - # TODO: Duration and timestamp operations should error on overflow. - "timestamps/timestamp_range/sub_time_duration_over,sub_time_duration_under", + # TODO: Ensure adding negative duration values is appropriately supported. "timestamps/timestamp_arithmetic/add_time_to_duration_nanos_negative", @@ -151,21 +146,10 @@ _TESTS_TO_SKIP_PLANNER = [ "string_ext/format", "string_ext/format_errors", - # TODO: Check behavior for go/cpp + # TODO: This is actually a user experience degradation. + # Not worth fixing until we see a concrete need. "basic/functions/unbound_is_runtime_error", - # TODO: Ensure overflow occurs on conversions of double values which might not work properly on all platforms. - "conversions/int/double_int_min_range", - "enums/legacy_proto3/assign_standalone_int_too_big", - "enums/legacy_proto3/assign_standalone_int_too_neg", - - # TODO: Duration and timestamp operations should error on overflow. - "timestamps/timestamp_range/sub_time_duration_over", - "timestamps/timestamp_range/sub_time_duration_under", - - # Skip until fixed. - "parse/receiver_function_names", - # Type inference edgecases around null(able) assignability. # These type check, but resolve to a different type. # list(int), want list(wrapper(int)) diff --git a/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java b/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java index 0ee7824b7..fb512541f 100644 --- a/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java +++ b/runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java @@ -391,7 +391,7 @@ public static Optional doubleToUnsignedChecked(double v) { public static Optional doubleToLongChecked(double v) { // getExponent of NaN or Infinite values will return a Double.MAX_EXPONENT + 1 (or 128) int exp = Math.getExponent(v); - if (exp >= 63 && v != Math.scalb(-1.0, 63)) { + if (exp >= 63) { return Optional.empty(); } return Optional.of((long) v); diff --git a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java index 6bb3d1e22..77f605efc 100644 --- a/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java +++ b/runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java @@ -14,6 +14,8 @@ package dev.cel.runtime.planner; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.auto.value.AutoValue; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; @@ -293,9 +295,13 @@ private PlannedInterpretable planCall(CelExpr expr, PlannerContext ctx) { } if (resolvedOverload == null) { - if (!lateBoundFunctionNames.contains(functionName)) { + boolean isLateBound = lateBoundFunctionNames.contains(functionName); + // For type-checked ASTs, functions that are not explicitly registered as late-bound + // must be resolved at plan time. + // For parsed-only ASTs or late-bound functions, defer overload resolution to runtime. + if (ctx.isChecked() && !isLateBound) { CelReference reference = ctx.referenceMap().get(expr.id()); - if (reference != null) { + if (reference != null && !reference.overloadIds().isEmpty()) { throw new CelOverloadNotFoundException(functionName, reference.overloadIds()); } else { throw new CelOverloadNotFoundException(functionName); @@ -303,7 +309,10 @@ private PlannedInterpretable planCall(CelExpr expr, PlannerContext ctx) { } ImmutableList overloadIds = ImmutableList.of(); - if (resolvedFunction.overloadId().isPresent()) { + CelReference reference = ctx.referenceMap().get(expr.id()); + if (reference != null && !reference.overloadIds().isEmpty()) { + overloadIds = reference.overloadIds(); + } else if (resolvedFunction.overloadId().isPresent()) { overloadIds = ImmutableList.of(resolvedFunction.overloadId().get()); } @@ -628,16 +637,23 @@ private static Builder newBuilder() { } static final class PlannerContext { - private final ImmutableMap referenceMap; - private final ImmutableMap typeMap; + private final CelAbstractSyntaxTree ast; private final HashMap localVars = new HashMap<>(); + CelAbstractSyntaxTree ast() { + return ast; + } + ImmutableMap referenceMap() { - return referenceMap; + return ast.getReferenceMap(); } ImmutableMap typeMap() { - return typeMap; + return ast.getTypeMap(); + } + + boolean isChecked() { + return ast.isChecked(); } private void pushLocalVars(String... names) { @@ -670,14 +686,12 @@ private boolean isLocalVar(String name) { return localVars.containsKey(name); } - private PlannerContext( - ImmutableMap referenceMap, ImmutableMap typeMap) { - this.referenceMap = referenceMap; - this.typeMap = typeMap; + private PlannerContext(CelAbstractSyntaxTree ast) { + this.ast = checkNotNull(ast); } static PlannerContext create(CelAbstractSyntaxTree ast) { - return new PlannerContext(ast.getReferenceMap(), ast.getTypeMap()); + return new PlannerContext(ast); } } diff --git a/runtime/src/main/java/dev/cel/runtime/standard/SubtractOperator.java b/runtime/src/main/java/dev/cel/runtime/standard/SubtractOperator.java index 784c46825..cf836ab70 100644 --- a/runtime/src/main/java/dev/cel/runtime/standard/SubtractOperator.java +++ b/runtime/src/main/java/dev/cel/runtime/standard/SubtractOperator.java @@ -68,13 +68,30 @@ public enum SubtractOverload implements CelStandardOverload { "subtract_timestamp_timestamp", Instant.class, Instant.class, - (Instant i1, Instant i2) -> java.time.Duration.between(i2, i1)); + (Instant i1, Instant i2) -> { + java.time.Duration between = java.time.Duration.between(i2, i1); + try { + // Call toNanos() to validate 64-bit nanosecond overflow (throws + // ArithmeticException). + @SuppressWarnings("unused") + long unused = between.toNanos(); + } catch (ArithmeticException e) { + throw new CelNumericOverflowException(e); + } + return between; + }); } else { return CelFunctionBinding.from( "subtract_timestamp_timestamp", Timestamp.class, Timestamp.class, - (Timestamp t1, Timestamp t2) -> ProtoTimeUtils.between(t2, t1)); + (Timestamp t1, Timestamp t2) -> { + try { + return ProtoTimeUtils.between(t2, t1); + } catch (ArithmeticException e) { + throw new CelNumericOverflowException(e); + } + }); } }), SUBTRACT_TIMESTAMP_DURATION(