Skip to content

Commit 8d73112

Browse files
l46kokcopybara-github
authored andcommitted
Fix conformance test case around receiver function names containing reserved keywords for parsed-only case
PiperOrigin-RevId: 955083744
1 parent 0bd9173 commit 8d73112

6 files changed

Lines changed: 65 additions & 48 deletions

File tree

common/src/main/java/dev/cel/common/internal/ProtoAdapter.java

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -325,13 +325,6 @@ private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) {
325325
value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value)));
326326
case FLOAT:
327327
return unwrapAndConvert(DOUBLE_CONVERTER);
328-
case DOUBLE:
329-
case SFIXED64:
330-
case SINT64:
331-
case INT64:
332-
return BidiConverter.of(
333-
BidiConverter.IDENTITY.forwardConverter(),
334-
value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value)));
335328
case BYTES:
336329
if (celOptions.evaluateCanonicalTypesToNativeValues()) {
337330
return BidiConverter.<Object, Object>of(
@@ -342,21 +335,26 @@ private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) {
342335
return BidiConverter.of(
343336
BidiConverter.IDENTITY.forwardConverter(),
344337
value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value)));
338+
case DOUBLE:
339+
case SFIXED64:
340+
case SINT64:
341+
case INT64:
345342
case STRING:
346-
return BidiConverter.of(
347-
BidiConverter.IDENTITY.forwardConverter(),
348-
value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value)));
349343
case BOOL:
350344
return BidiConverter.of(
351345
BidiConverter.IDENTITY.forwardConverter(),
352346
value -> BidiConverter.IDENTITY.backwardConverter().convert(maybeUnwrap(value)));
353347
case ENUM:
354348
return BidiConverter.<Object, Long>of(
355349
value -> (long) ((EnumValueDescriptor) value).getNumber(),
356-
number ->
357-
fieldDescriptor
358-
.getEnumType()
359-
.findValueByNumberCreatingIfUnknown(number.intValue()));
350+
number -> {
351+
if (number > Integer.MAX_VALUE || number < Integer.MIN_VALUE) {
352+
throw new IllegalArgumentException("Enum value out of int32 range: " + number);
353+
}
354+
return fieldDescriptor
355+
.getEnumType()
356+
.findValueByNumberCreatingIfUnknown(number.intValue());
357+
});
360358
case MESSAGE:
361359
return BidiConverter.<MessageOrBuilder, Object>of(
362360
this::adaptProtoToValue,

common/src/main/java/dev/cel/common/internal/ProtoTimeUtils.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,10 @@ public static Duration between(Timestamp from, Timestamp to) {
406406
Instant javaTo = ProtoTimeUtils.toJavaInstant(checkValid(to));
407407

408408
java.time.Duration between = java.time.Duration.between(javaFrom, javaTo);
409+
// Call toNanos() to validate 64-bit nanosecond overflow (throws ArithmeticException).
410+
// Suppress unused variable warning as the duration object itself is returned.
411+
@SuppressWarnings("unused")
412+
long unused = between.toNanos();
409413

410414
return ProtoTimeUtils.toProtoDuration(between);
411415
}

conformance/src/test/java/dev/cel/conformance/BUILD.bazel

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,7 @@ _TESTS_TO_SKIP_LEGACY = [
108108
# TODO: Support setting / getting enum values out of the defined enum value range.
109109
"enums/legacy_proto2/select_big,select_neg",
110110
"enums/legacy_proto2/assign_standalone_int_big,assign_standalone_int_neg",
111-
# TODO: Generate errors on enum value assignment overflows for proto3.
112-
"enums/legacy_proto3/assign_standalone_int_too_big,assign_standalone_int_too_neg",
113-
# TODO: Ensure overflow occurs on conversions of double values which might not work properly on all platforms.
114-
"conversions/int/double_int_min_range",
115-
# TODO: Duration and timestamp operations should error on overflow.
116-
"timestamps/timestamp_range/sub_time_duration_over,sub_time_duration_under",
111+
117112
# TODO: Ensure adding negative duration values is appropriately supported.
118113
"timestamps/timestamp_arithmetic/add_time_to_duration_nanos_negative",
119114

@@ -151,21 +146,10 @@ _TESTS_TO_SKIP_PLANNER = [
151146
"string_ext/format",
152147
"string_ext/format_errors",
153148

154-
# TODO: Check behavior for go/cpp
149+
# TODO: This is actually a user experience degradation.
150+
# Not worth fixing until we see a concrete need.
155151
"basic/functions/unbound_is_runtime_error",
156152

157-
# TODO: Ensure overflow occurs on conversions of double values which might not work properly on all platforms.
158-
"conversions/int/double_int_min_range",
159-
"enums/legacy_proto3/assign_standalone_int_too_big",
160-
"enums/legacy_proto3/assign_standalone_int_too_neg",
161-
162-
# TODO: Duration and timestamp operations should error on overflow.
163-
"timestamps/timestamp_range/sub_time_duration_over",
164-
"timestamps/timestamp_range/sub_time_duration_under",
165-
166-
# Skip until fixed.
167-
"parse/receiver_function_names",
168-
169153
# Type inference edgecases around null(able) assignability.
170154
# These type check, but resolve to a different type.
171155
# list(int), want list(wrapper(int))

runtime/src/main/java/dev/cel/runtime/RuntimeHelpers.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ public static Optional<UnsignedLong> doubleToUnsignedChecked(double v) {
391391
public static Optional<Long> doubleToLongChecked(double v) {
392392
// getExponent of NaN or Infinite values will return a Double.MAX_EXPONENT + 1 (or 128)
393393
int exp = Math.getExponent(v);
394-
if (exp >= 63 && v != Math.scalb(-1.0, 63)) {
394+
if (exp >= 63) {
395395
return Optional.empty();
396396
}
397397
return Optional.of((long) v);

runtime/src/main/java/dev/cel/runtime/planner/ProgramPlanner.java

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
package dev.cel.runtime.planner;
1616

17+
import static com.google.common.base.Preconditions.checkNotNull;
18+
1719
import com.google.auto.value.AutoValue;
1820
import com.google.common.base.Strings;
1921
import com.google.common.collect.ImmutableList;
@@ -293,17 +295,24 @@ private PlannedInterpretable planCall(CelExpr expr, PlannerContext ctx) {
293295
}
294296

295297
if (resolvedOverload == null) {
296-
if (!lateBoundFunctionNames.contains(functionName)) {
298+
boolean isLateBound = lateBoundFunctionNames.contains(functionName);
299+
// For type-checked ASTs, functions that are not explicitly registered as late-bound
300+
// must be resolved at plan time.
301+
// For parsed-only ASTs or late-bound functions, defer overload resolution to runtime.
302+
if (ctx.isChecked() && !isLateBound) {
297303
CelReference reference = ctx.referenceMap().get(expr.id());
298-
if (reference != null) {
304+
if (reference != null && !reference.overloadIds().isEmpty()) {
299305
throw new CelOverloadNotFoundException(functionName, reference.overloadIds());
300306
} else {
301307
throw new CelOverloadNotFoundException(functionName);
302308
}
303309
}
304310

305311
ImmutableList<String> overloadIds = ImmutableList.of();
306-
if (resolvedFunction.overloadId().isPresent()) {
312+
CelReference reference = ctx.referenceMap().get(expr.id());
313+
if (reference != null && !reference.overloadIds().isEmpty()) {
314+
overloadIds = reference.overloadIds();
315+
} else if (resolvedFunction.overloadId().isPresent()) {
307316
overloadIds = ImmutableList.of(resolvedFunction.overloadId().get());
308317
}
309318

@@ -628,16 +637,23 @@ private static Builder newBuilder() {
628637
}
629638

630639
static final class PlannerContext {
631-
private final ImmutableMap<Long, CelReference> referenceMap;
632-
private final ImmutableMap<Long, CelType> typeMap;
640+
private final CelAbstractSyntaxTree ast;
633641
private final HashMap<String, Integer> localVars = new HashMap<>();
634642

643+
CelAbstractSyntaxTree ast() {
644+
return ast;
645+
}
646+
635647
ImmutableMap<Long, CelReference> referenceMap() {
636-
return referenceMap;
648+
return ast.getReferenceMap();
637649
}
638650

639651
ImmutableMap<Long, CelType> typeMap() {
640-
return typeMap;
652+
return ast.getTypeMap();
653+
}
654+
655+
boolean isChecked() {
656+
return ast.isChecked();
641657
}
642658

643659
private void pushLocalVars(String... names) {
@@ -670,14 +686,12 @@ private boolean isLocalVar(String name) {
670686
return localVars.containsKey(name);
671687
}
672688

673-
private PlannerContext(
674-
ImmutableMap<Long, CelReference> referenceMap, ImmutableMap<Long, CelType> typeMap) {
675-
this.referenceMap = referenceMap;
676-
this.typeMap = typeMap;
689+
private PlannerContext(CelAbstractSyntaxTree ast) {
690+
this.ast = checkNotNull(ast);
677691
}
678692

679693
static PlannerContext create(CelAbstractSyntaxTree ast) {
680-
return new PlannerContext(ast.getReferenceMap(), ast.getTypeMap());
694+
return new PlannerContext(ast);
681695
}
682696
}
683697

runtime/src/main/java/dev/cel/runtime/standard/SubtractOperator.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,30 @@ public enum SubtractOverload implements CelStandardOverload {
6868
"subtract_timestamp_timestamp",
6969
Instant.class,
7070
Instant.class,
71-
(Instant i1, Instant i2) -> java.time.Duration.between(i2, i1));
71+
(Instant i1, Instant i2) -> {
72+
java.time.Duration between = java.time.Duration.between(i2, i1);
73+
try {
74+
// Call toNanos() to validate 64-bit nanosecond overflow (throws
75+
// ArithmeticException).
76+
@SuppressWarnings("unused")
77+
long unused = between.toNanos();
78+
} catch (ArithmeticException e) {
79+
throw new CelNumericOverflowException(e);
80+
}
81+
return between;
82+
});
7283
} else {
7384
return CelFunctionBinding.from(
7485
"subtract_timestamp_timestamp",
7586
Timestamp.class,
7687
Timestamp.class,
77-
(Timestamp t1, Timestamp t2) -> ProtoTimeUtils.between(t2, t1));
88+
(Timestamp t1, Timestamp t2) -> {
89+
try {
90+
return ProtoTimeUtils.between(t2, t1);
91+
} catch (ArithmeticException e) {
92+
throw new CelNumericOverflowException(e);
93+
}
94+
});
7895
}
7996
}),
8097
SUBTRACT_TIMESTAMP_DURATION(

0 commit comments

Comments
 (0)