From 62c1f111d0b9d02f44185dd9a720bab768528916 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Thu, 12 Mar 2026 11:08:12 -0700 Subject: [PATCH] Null assignability fix for repeated and map fields PiperOrigin-RevId: 882683464 --- .../dev/cel/common/internal/ProtoAdapter.java | 51 +++++++++++++++++-- .../test/resources/nullAssignability.baseline | 29 +++++++++++ .../dev/cel/testing/BaseInterpreterTest.java | 27 ++++++++++ 3 files changed, 102 insertions(+), 5 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 b6648a5b8..7e3910433 100644 --- a/common/src/main/java/dev/cel/common/internal/ProtoAdapter.java +++ b/common/src/main/java/dev/cel/common/internal/ProtoAdapter.java @@ -222,9 +222,7 @@ public Optional adaptValueToFieldType( throw new IllegalArgumentException("Unsupported field type"); } - String typeFullName = fieldDescriptor.getMessageType().getFullName(); - if (!WellKnownProto.ANY_VALUE.typeName().equals(typeFullName) - && !WellKnownProto.JSON_VALUE.typeName().equals(typeFullName)) { + if (!isFieldAnyOrJson(fieldDescriptor)) { return Optional.empty(); } } @@ -242,7 +240,11 @@ public Optional adaptValueToFieldType( getDefaultValueForMaybeMessage(keyDescriptor), valueDescriptor.getLiteType(), getDefaultValueForMaybeMessage(valueDescriptor)); + boolean isValueAnyOrJson = isFieldAnyOrJson(valueDescriptor); for (Map.Entry entry : ((Map) fieldValue).entrySet()) { + if (!isValueAnyOrJson && entry.getValue() instanceof NullValue) { + continue; + } mapEntries.add( protoMapEntry.toBuilder() .setKey(keyConverter.backwardConverter().convert(entry.getKey())) @@ -252,15 +254,54 @@ public Optional adaptValueToFieldType( return Optional.of(mapEntries); } if (fieldDescriptor.isRepeated()) { + List listValue = (List) fieldValue; + + if (!isFieldAnyOrJson(fieldDescriptor)) { + listValue = filterOutNullValues(listValue); + } + return Optional.of( - AdaptingTypes.adaptingList( - (List) fieldValue, fieldToValueConverter(fieldDescriptor).reverse())); + AdaptingTypes.adaptingList(listValue, fieldToValueConverter(fieldDescriptor).reverse())); } return Optional.of( fieldToValueConverter(fieldDescriptor).backwardConverter().convert(fieldValue)); } + private static List filterOutNullValues(List originalList) { + List filteredList = null; + + for (int i = 0; i < originalList.size(); i++) { + Object elem = originalList.get(i); + + if (elem instanceof NullValue) { + if (filteredList == null) { + filteredList = new ArrayList<>(originalList.size() - 1); + if (i > 0) { + filteredList.addAll(originalList.subList(0, i)); + } + } + } else if (filteredList != null) { + filteredList.add(elem); + } + } + + // Return the original list if no nulls were found to avoid unnecessary allocations + return filteredList != null ? filteredList : originalList; + } + + private static boolean isFieldAnyOrJson(FieldDescriptor fieldDescriptor) { + if (!fieldDescriptor.getType().equals(FieldDescriptor.Type.MESSAGE)) { + return false; + } + + String typeFullName = fieldDescriptor.getMessageType().getFullName(); + + return WellKnownProto.getByTypeName(typeFullName) + .map(wkp -> wkp.equals(WellKnownProto.ANY_VALUE) || wkp.equals(WellKnownProto.JSON_VALUE)) + .orElse(false); + } + @SuppressWarnings("rawtypes") private BidiConverter fieldToValueConverter(FieldDescriptor fieldDescriptor) { switch (fieldDescriptor.getType()) { diff --git a/runtime/src/test/resources/nullAssignability.baseline b/runtime/src/test/resources/nullAssignability.baseline index 47b9c7a0d..b60f434ea 100644 --- a/runtime/src/test/resources/nullAssignability.baseline +++ b/runtime/src/test/resources/nullAssignability.baseline @@ -33,3 +33,32 @@ Source: has(TestAllTypes{single_timestamp: null}.single_timestamp) bindings: {} result: false +Source: TestAllTypes{repeated_timestamp: [timestamp(1), null]}.repeated_timestamp == [timestamp(1)] +=====> +bindings: {} +result: true + +Source: TestAllTypes{map_bool_timestamp: {true: null, false: timestamp(1)}}.map_bool_timestamp == {false: timestamp(1)} +=====> +bindings: {} +result: true + +Source: TestAllTypes{repeated_any: [1, null]}.repeated_any == [1, null] +=====> +bindings: {} +result: true + +Source: TestAllTypes{map_bool_any: {true: null, false: 1}}.map_bool_any == {true: null, false: 1} +=====> +bindings: {} +result: true + +Source: TestAllTypes{repeated_value: [google.protobuf.Value{bool_value: true}, null]}.repeated_value == [true, null] +=====> +bindings: {} +result: true + +Source: TestAllTypes{map_bool_value: {true: null, false: google.protobuf.Value{bool_value: true}}}.map_bool_value == {true: null, false: true} +=====> +bindings: {} +result: true \ No newline at end of file diff --git a/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java b/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java index b3c9af423..f3c1cf398 100644 --- a/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java +++ b/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java @@ -2162,6 +2162,33 @@ public void nullAssignability() throws Exception { source = "has(TestAllTypes{single_timestamp: null}.single_timestamp)"; runTest(); + + source = + "TestAllTypes{repeated_timestamp: [timestamp(1), null]}.repeated_timestamp ==" + + " [timestamp(1)]"; + runTest(); + + source = + "TestAllTypes{map_bool_timestamp: {true: null, false: timestamp(1)}}.map_bool_timestamp ==" + + " {false: timestamp(1)}"; + runTest(); + + source = "TestAllTypes{repeated_any: [1, null]}.repeated_any == [1, null]"; + runTest(); + + source = + "TestAllTypes{map_bool_any: {true: null, false: 1}}.map_bool_any == {true: null, false: 1}"; + runTest(); + + source = + "TestAllTypes{repeated_value: [google.protobuf.Value{bool_value: true}," + + " null]}.repeated_value == [true, null]"; + runTest(); + + source = + "TestAllTypes{map_bool_value: {true: null, false: google.protobuf.Value{bool_value:" + + " true}}}.map_bool_value == {true: null, false: true}"; + runTest(); } @Test