Skip to content

Commit be018d0

Browse files
l46kokcopybara-github
authored andcommitted
Fix optionals to properly error on invalid qualification
PiperOrigin-RevId: 875419705
1 parent 0737691 commit be018d0

File tree

6 files changed

+69
-18
lines changed

6 files changed

+69
-18
lines changed

common/src/main/java/dev/cel/common/exceptions/CelInvalidArgumentException.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,8 @@ public final class CelInvalidArgumentException extends CelRuntimeException {
2424
public CelInvalidArgumentException(Throwable cause) {
2525
super(cause, CelErrorCode.INVALID_ARGUMENT);
2626
}
27+
28+
public CelInvalidArgumentException(String message) {
29+
super(message, CelErrorCode.INVALID_ARGUMENT);
30+
}
2731
}

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,6 @@ _TESTS_TO_SKIP_PLANNER = [
175175
"timestamps/timestamp_range/sub_time_duration_under",
176176

177177
# Skip until fixed.
178-
"fields/qualified_identifier_resolution/map_key_float",
179-
"fields/qualified_identifier_resolution/map_key_null",
180-
"fields/qualified_identifier_resolution/map_value_repeat_key_heterogeneous",
181-
"optionals/optionals/map_null_entry_no_such_key",
182-
"optionals/optionals/map_present_key_invalid_field",
183178
"parse/receiver_function_names",
184179
"proto2/extensions_get/package_scoped_test_all_types_ext",
185180
"proto2/extensions_get/package_scoped_repeated_test_all_types",

runtime/src/main/java/dev/cel/runtime/planner/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ java_library(
334334
":localized_evaluation_exception",
335335
":planned_interpretable",
336336
"//common/exceptions:duplicate_key",
337+
"//common/exceptions:invalid_argument",
337338
"//runtime:evaluation_exception",
338339
"//runtime:interpretable",
339340
"@maven//:com_google_errorprone_error_prone_annotations",

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

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
import com.google.common.base.Preconditions;
1818
import com.google.common.collect.ImmutableMap;
1919
import com.google.common.collect.Sets;
20+
import com.google.common.primitives.UnsignedLong;
2021
import com.google.errorprone.annotations.Immutable;
2122
import dev.cel.common.exceptions.CelDuplicateKeyException;
23+
import dev.cel.common.exceptions.CelInvalidArgumentException;
2224
import dev.cel.runtime.CelEvaluationException;
2325
import dev.cel.runtime.GlobalResolver;
2426
import java.util.HashSet;
@@ -46,13 +48,37 @@ public Object eval(GlobalResolver resolver, ExecutionFrame frame) throws CelEval
4648
HashSet<Object> keysSeen = Sets.newHashSetWithExpectedSize(keys.length);
4749

4850
for (int i = 0; i < keys.length; i++) {
49-
Object key = keys[i].eval(resolver, frame);
50-
Object val = values[i].eval(resolver, frame);
51+
PlannedInterpretable keyInterpretable = keys[i];
52+
Object key = keyInterpretable.eval(resolver, frame);
53+
if (!(key instanceof String
54+
|| key instanceof Long
55+
|| key instanceof UnsignedLong
56+
|| key instanceof Boolean)) {
57+
throw new LocalizedEvaluationException(
58+
new CelInvalidArgumentException("Unsupported key type: " + key),
59+
keyInterpretable.exprId());
60+
}
5161

52-
if (!keysSeen.add(key)) {
53-
throw new LocalizedEvaluationException(CelDuplicateKeyException.of(key), keys[i].exprId());
62+
boolean isDuplicate = !keysSeen.add(key);
63+
if (!isDuplicate) {
64+
if (key instanceof Long) {
65+
long longVal = (Long) key;
66+
if (longVal >= 0) {
67+
isDuplicate = keysSeen.contains(UnsignedLong.valueOf(longVal));
68+
}
69+
} else if (key instanceof UnsignedLong) {
70+
UnsignedLong ulongVal = (UnsignedLong) key;
71+
isDuplicate = keysSeen.contains(ulongVal.longValue());
72+
}
5473
}
5574

75+
if (isDuplicate) {
76+
throw new LocalizedEvaluationException(
77+
CelDuplicateKeyException.of(key), keyInterpretable.exprId());
78+
}
79+
80+
Object val = values[i].eval(resolver, frame);
81+
5682
if (isOptional[i]) {
5783
if (!(val instanceof Optional)) {
5884
throw new IllegalArgumentException(

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

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package dev.cel.runtime.planner;
1616

1717
import dev.cel.common.exceptions.CelAttributeNotFoundException;
18+
import dev.cel.common.values.OptionalValue;
1819
import dev.cel.common.values.SelectableValue;
1920
import java.util.Map;
2021

@@ -31,21 +32,34 @@ public String value() {
3132
@Override
3233
@SuppressWarnings("unchecked") // Qualifications on maps/structs must be a string
3334
public Object qualify(Object obj) {
35+
if (obj instanceof OptionalValue) {
36+
OptionalValue<?, ?> opt = (OptionalValue<?, ?>) obj;
37+
if (!opt.isZeroValue()) {
38+
Object inner = opt.value();
39+
if (!(inner instanceof SelectableValue) && !(inner instanceof Map)) {
40+
throw CelAttributeNotFoundException.forFieldResolution(value);
41+
}
42+
}
43+
}
44+
3445
if (obj instanceof SelectableValue) {
3546
return ((SelectableValue<String>) obj).select(value);
36-
} else if (obj instanceof Map) {
37-
Map<String, Object> map = (Map<String, Object>) obj;
38-
if (!map.containsKey(value)) {
39-
throw CelAttributeNotFoundException.forMissingMapKey(value);
40-
}
47+
}
4148

49+
if (obj instanceof Map) {
50+
Map<?, ?> map = (Map<?, ?>) obj;
4251
Object mapVal = map.get(value);
4352

44-
if (mapVal == null) {
45-
throw CelAttributeNotFoundException.of(
46-
String.format("Map value cannot be null for key: %s", value));
53+
if (mapVal != null) {
54+
return mapVal;
55+
}
56+
57+
if (!map.containsKey(value)) {
58+
throw CelAttributeNotFoundException.forMissingMapKey(value);
4759
}
48-
return map.get(value);
60+
61+
throw CelAttributeNotFoundException.of(
62+
String.format("Map value cannot be null for key: %s", value));
4963
}
5064

5165
throw CelAttributeNotFoundException.forFieldResolution(value);

runtime/src/test/java/dev/cel/runtime/planner/ProgramPlannerTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,17 @@ public void plan_createMap_containsDuplicateKey_throws() throws Exception {
390390
.contains("evaluation error at <input>:20: duplicate map key [true]");
391391
}
392392

393+
@Test
394+
public void plan_createMap_unsupportedKeyType_throws() throws Exception {
395+
CelAbstractSyntaxTree ast = compile("{1.0: 'foo'}");
396+
Program program = PLANNER.plan(ast);
397+
398+
CelEvaluationException e = assertThrows(CelEvaluationException.class, program::eval);
399+
assertThat(e)
400+
.hasMessageThat()
401+
.contains("evaluation error at <input>:1: Unsupported key type: 1.0");
402+
}
403+
393404
@Test
394405
public void plan_createStruct() throws Exception {
395406
CelAbstractSyntaxTree ast = compile("cel.expr.conformance.proto3.TestAllTypes{}");

0 commit comments

Comments
 (0)