Fix null assignment to fields

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

@@ -24,4 +24,8 @@ public final class CelInvalidArgumentException extends CelRuntimeException {
   public CelInvalidArgumentException(Throwable cause) {
     super(cause, CelErrorCode.INVALID_ARGUMENT);
   }
+
+  public CelInvalidArgumentException(String message) {
+    super(message, CelErrorCode.INVALID_ARGUMENT);
+  }
 }

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

@@ -204,8 +204,26 @@ public Optional<Object> adaptFieldToValue(FieldDescriptor fieldDescriptor, Objec
   @SuppressWarnings({"unchecked", "rawtypes"})
   public Optional<Object> adaptValueToFieldType(
       FieldDescriptor fieldDescriptor, Object fieldValue) {
-    if (isWrapperType(fieldDescriptor) && fieldValue.equals(NullValue.NULL_VALUE)) {
-      return Optional.empty();
+    if (fieldValue instanceof NullValue) {
+      // `null` literal can only be assigned to singular messages. Explicitly check
+      // for non-allowed cases (collections, maps)
+      if (fieldDescriptor.isMapField()
+          || fieldDescriptor.isRepeated()
+          || fieldDescriptor.getJavaType() != FieldDescriptor.JavaType.MESSAGE
+          || WellKnownProto.JSON_STRUCT_VALUE
+              .typeName()
+              .equals(fieldDescriptor.getMessageType().getFullName())
+          || WellKnownProto.JSON_LIST_VALUE
+              .typeName()
+              .equals(fieldDescriptor.getMessageType().getFullName())) {
+        throw new IllegalArgumentException("Unsupported field type");
+      }
+
+      String typeFullName = fieldDescriptor.getMessageType().getFullName();
+      if (!WellKnownProto.ANY_VALUE.typeName().equals(typeFullName)
+          && !WellKnownProto.JSON_VALUE.typeName().equals(typeFullName)) {
+        return Optional.empty();
+      }
     }
     if (fieldDescriptor.isMapField()) {
       Descriptor entryDescriptor = fieldDescriptor.getMessageType();
@@ -370,14 +388,6 @@ private static String typeName(Descriptor protoType) {
     return protoType.getFullName();
   }
 
-  private static boolean isWrapperType(FieldDescriptor fieldDescriptor) {
-    if (fieldDescriptor.getJavaType() != FieldDescriptor.JavaType.MESSAGE) {
-      return false;
-    }
-    String fieldTypeName = fieldDescriptor.getMessageType().getFullName();
-    return WellKnownProto.isWrapperType(fieldTypeName);
-  }
-
   private static int intCheckedCast(long value) {
     try {
       return Ints.checkedCast(value);

common/src/main/java/dev/cel/common/values/CelValueConverter.java

@@ -41,24 +41,39 @@ public static CelValueConverter getDefaultInstance() {
     return DEFAULT_INSTANCE;
   }
 
-  /** Adapts a {@link CelValue} to a plain old Java Object. */
-  public Object unwrap(CelValue celValue) {
-    Preconditions.checkNotNull(celValue);
+  /**
+   * Unwraps the {@code value} into its plain old Java Object representation.
+   *
+   * <p>The value may be a {@link CelValue}, a {@link Collection} or a {@link Map}.
+   */
+  public Object maybeUnwrap(Object value) {
+    if (value instanceof CelValue) {
+      return unwrap((CelValue) value);
+    }
 
-    if (celValue instanceof OptionalValue) {
-      OptionalValue<Object, Object> optionalValue = (OptionalValue<Object, Object>) celValue;
-      if (optionalValue.isZeroValue()) {
-        return Optional.empty();
+    if (value instanceof Collection) {
+      Collection<Object> collection = (Collection<Object>) value;
+      ImmutableList.Builder<Object> builder =
+          ImmutableList.builderWithExpectedSize(collection.size());
+      for (Object element : collection) {
+        builder.add(maybeUnwrap(element));
       }
 
-      return Optional.of(optionalValue.value());
+      return builder.build();
     }
 
-    if (celValue instanceof ErrorValue) {
-      return celValue;
+    if (value instanceof Map) {
+      Map<Object, Object> map = (Map<Object, Object>) value;
+      ImmutableMap.Builder<Object, Object> builder =
+          ImmutableMap.builderWithExpectedSize(map.size());
+      for (Map.Entry<Object, Object> entry : map.entrySet()) {
+        builder.put(maybeUnwrap(entry.getKey()), maybeUnwrap(entry.getValue()));
+      }
+
+      return builder.buildOrThrow();
     }
 
-    return celValue.value();
+    return value;
   }
 
   /**
@@ -101,6 +116,26 @@ protected Object normalizePrimitive(Object value) {
     return value;
   }
 
+  /** Adapts a {@link CelValue} to a plain old Java Object. */
+  private static Object unwrap(CelValue celValue) {
+    Preconditions.checkNotNull(celValue);
+
+    if (celValue instanceof OptionalValue) {
+      OptionalValue<Object, Object> optionalValue = (OptionalValue<Object, Object>) celValue;
+      if (optionalValue.isZeroValue()) {
+        return Optional.empty();
+      }
+
+      return Optional.of(optionalValue.value());
+    }
+
+    if (celValue instanceof ErrorValue) {
+      return celValue;
+    }
+
+    return celValue.value();
+  }
+
   private ImmutableList<Object> toListValue(Collection<Object> iterable) {
     Preconditions.checkNotNull(iterable);
 

common/src/main/java/dev/cel/common/values/ProtoCelValueConverter.java

@@ -67,7 +67,7 @@ protected Object fromWellKnownProto(MessageLiteOrBuilder msg, WellKnownProto wel
         try {
           unpackedMessage = dynamicProto.unpack((Any) message);
         } catch (InvalidProtocolBufferException e) {
-          throw new IllegalStateException(
+          throw new IllegalArgumentException(
               "Unpacking failed for message: " + message.getDescriptorForType().getFullName(), e);
         }
         return toRuntimeValue(unpackedMessage);

common/src/test/java/dev/cel/common/internal/ProtoAdapterTest.java

@@ -150,10 +150,7 @@ public static List<Object[]> data() {
     @Test
     public void adaptValueToProto_bidirectionalConversion() {
       DynamicProto dynamicProto = DynamicProto.create(DefaultMessageFactory.INSTANCE);
-      ProtoAdapter protoAdapter =
-          new ProtoAdapter(
-              dynamicProto,
-              CelOptions.current().build());
+      ProtoAdapter protoAdapter = new ProtoAdapter(dynamicProto, CelOptions.current().build());
       assertThat(protoAdapter.adaptValueToProto(value, proto.getDescriptorForType().getFullName()))
           .isEqualTo(proto);
       assertThat(protoAdapter.adaptProtoToValue(proto)).isEqualTo(value);
@@ -181,6 +178,18 @@ public void adaptAnyValue_hermeticTypes_bidirectionalConversion() {
 
   @RunWith(JUnit4.class)
   public static class AsymmetricConversionTest {
+
+    @Test
+    public void unpackAny_celNullValue() throws Exception {
+      ProtoAdapter protoAdapter = new ProtoAdapter(DYNAMIC_PROTO, CelOptions.DEFAULT);
+      Any any =
+          (Any)
+              protoAdapter.adaptValueToProto(
+                  dev.cel.common.values.NullValue.NULL_VALUE, "google.protobuf.Any");
+      Object unpacked = protoAdapter.adaptProtoToValue(any);
+      assertThat(unpacked).isEqualTo(dev.cel.common.values.NullValue.NULL_VALUE);
+    }
+
     @Test
     public void adaptValueToProto_asymmetricFloatConversion() {
       ProtoAdapter protoAdapter = new ProtoAdapter(DYNAMIC_PROTO, CelOptions.DEFAULT);

common/src/test/java/dev/cel/common/values/CelValueConverterTest.java

@@ -37,15 +37,16 @@ public void toRuntimeValue_optionalValue() {
   @Test
   @SuppressWarnings("unchecked") // Test only
   public void unwrap_optionalValue() {
-    Optional<Long> result = (Optional<Long>) CEL_VALUE_CONVERTER.unwrap(OptionalValue.create(2L));
+    Optional<Long> result =
+        (Optional<Long>) CEL_VALUE_CONVERTER.maybeUnwrap(OptionalValue.create(2L));
 
     assertThat(result).isEqualTo(Optional.of(2L));
   }
 
   @Test
   @SuppressWarnings("unchecked") // Test only
   public void unwrap_emptyOptionalValue() {
-    Optional<Long> result = (Optional<Long>) CEL_VALUE_CONVERTER.unwrap(OptionalValue.EMPTY);
+    Optional<Long> result = (Optional<Long>) CEL_VALUE_CONVERTER.maybeUnwrap(OptionalValue.EMPTY);
 
     assertThat(result).isEqualTo(Optional.empty());
   }

common/src/test/java/dev/cel/common/values/ProtoCelValueConverterTest.java

@@ -35,7 +35,7 @@ public class ProtoCelValueConverterTest {
 
   @Test
   public void unwrap_nullValue() {
-    NullValue nullValue = (NullValue) PROTO_CEL_VALUE_CONVERTER.unwrap(NullValue.NULL_VALUE);
+    NullValue nullValue = (NullValue) PROTO_CEL_VALUE_CONVERTER.maybeUnwrap(NullValue.NULL_VALUE);
 
     // Note: No conversion is attempted. We're using dev.cel.common.values.NullValue.NULL_VALUE as
     // the

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

@@ -124,14 +124,6 @@ _TESTS_TO_SKIP_LEGACY = [
     "string_ext/format",
     "string_ext/format_errors",
 
-    # TODO: Fix null assignment to a field
-    "proto2/set_null/single_message",
-    "proto2/set_null/single_duration",
-    "proto2/set_null/single_timestamp",
-    "proto3/set_null/single_message",
-    "proto3/set_null/single_duration",
-    "proto3/set_null/single_timestamp",
-
     # Future features for CEL 1.0
     # TODO: Strong typing support for enums, specified but not implemented.
     "enums/strong_proto2",
@@ -162,7 +154,6 @@ _TESTS_TO_SKIP_PLANNER = [
     "string_ext/format_errors",
 
     # TODO: Check behavior for go/cpp
-    "basic/functions/unbound",
     "basic/functions/unbound_is_runtime_error",
 
     # TODO: Ensure overflow occurs on conversions of double values which might not work properly on all platforms.
@@ -175,26 +166,7 @@ _TESTS_TO_SKIP_PLANNER = [
     "timestamps/timestamp_range/sub_time_duration_under",
 
     # Skip until fixed.
-    "fields/qualified_identifier_resolution/map_key_float",
-    "fields/qualified_identifier_resolution/map_key_null",
-    "fields/qualified_identifier_resolution/map_value_repeat_key_heterogeneous",
-    "optionals/optionals/map_null_entry_no_such_key",
-    "optionals/optionals/map_present_key_invalid_field",
     "parse/receiver_function_names",
-    "proto2/extensions_get/package_scoped_test_all_types_ext",
-    "proto2/extensions_get/package_scoped_repeated_test_all_types",
-    "proto2/extensions_get/message_scoped_nested_ext",
-    "proto2/extensions_get/message_scoped_repeated_test_all_types",
-    "proto2_ext/get_ext/package_scoped_repeated_test_all_types",
-    "proto2_ext/get_ext/message_scoped_repeated_test_all_types",
-
-    # TODO: Fix null assignment to a field
-    "proto2/set_null/single_message",
-    "proto2/set_null/single_duration",
-    "proto2/set_null/single_timestamp",
-    "proto3/set_null/single_message",
-    "proto3/set_null/single_duration",
-    "proto3/set_null/single_timestamp",
 
     # Type inference edgecases around null(able) assignability.
     # These type check, but resolve to a different type.

conformance/src/test/java/dev/cel/conformance/ConformanceTest.java

@@ -210,10 +210,10 @@ public void evaluate() throws Throwable {
     }
 
     CelRuntime runtime = getRuntime(test, usePlanner);
-    Program program = runtime.createProgram(response.getAst());
     ExprValue result = null;
     CelEvaluationException error = null;
     try {
+      Program program = runtime.createProgram(response.getAst());
       result = toExprValue(program.eval(getBindings(test)), response.getAst().getResultType());
     } catch (CelEvaluationException e) {
       error = e;

extensions/src/test/java/dev/cel/extensions/CelProtoExtensionsTest.java

@@ -46,6 +46,7 @@
 import dev.cel.runtime.CelFunctionBinding;
 import dev.cel.runtime.CelRuntime;
 import dev.cel.runtime.CelRuntimeFactory;
+import dev.cel.runtime.CelRuntimeImpl;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -60,9 +61,14 @@ public final class CelProtoExtensionsTest {
           .addVar("msg", StructTypeReference.create("cel.expr.conformance.proto2.TestAllTypes"))
           .setContainer(CelContainer.ofName("cel.expr.conformance.proto2"))
           .build();
-
   private static final CelRuntime CEL_RUNTIME =
-      CelRuntimeFactory.standardCelRuntimeBuilder()
+      CelRuntimeImpl.newBuilder()
+          .setOptions(
+              CelOptions.current()
+                  .enableTimestampEpoch(true)
+                  .enableHeterogeneousNumericComparisons(true)
+                  .build())
+          // CEL-Internal-2
           .addFileTypes(TestAllTypesExtensions.getDescriptor())
           .build();
 

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

@@ -22,7 +22,6 @@
 import dev.cel.common.exceptions.CelAttributeNotFoundException;
 import dev.cel.common.values.BaseProtoCelValueConverter;
 import dev.cel.common.values.BaseProtoMessageValueProvider;
-import dev.cel.common.values.CelValue;
 import dev.cel.common.values.CelValueProvider;
 import dev.cel.common.values.CombinedCelValueProvider;
 import dev.cel.common.values.SelectableValue;
@@ -64,7 +63,7 @@ static CelValueRuntimeTypeProvider newInstance(CelValueProvider valueProvider) {
 
   @Override
   public Object createMessage(String messageName, Map<String, Object> values) {
-    return maybeUnwrapCelValue(
+    return protoCelValueConverter.maybeUnwrap(
         valueProvider
             .newValue(messageName, values)
             .orElseThrow(
@@ -87,7 +86,7 @@ public Object selectField(Object message, String fieldName) {
     SelectableValue<String> selectableValue = getSelectableValueOrThrow(message, fieldName);
     Object value = selectableValue.select(fieldName);
 
-    return maybeUnwrapCelValue(value);
+    return protoCelValueConverter.maybeUnwrap(value);
   }
 
   @Override
@@ -120,24 +119,12 @@ public Object adapt(String messageName, Object message) {
     }
 
     if (message instanceof MessageLite) {
-      return maybeUnwrapCelValue(protoCelValueConverter.toRuntimeValue(message));
+      return protoCelValueConverter.maybeUnwrap(protoCelValueConverter.toRuntimeValue(message));
     }
 
     return message;
   }
 
-  /**
-   * DefaultInterpreter cannot handle CelValue and instead expects plain Java objects.
-   *
-   * <p>This will become unnecessary once we introduce a rewrite of a Cel runtime.
-   */
-  private Object maybeUnwrapCelValue(Object object) {
-    if (object instanceof CelValue) {
-      return protoCelValueConverter.unwrap((CelValue) object);
-    }
-    return object;
-  }
-
   private static void throwInvalidFieldSelection(String fieldName) {
     throw CelAttributeNotFoundException.forFieldResolution(fieldName);
   }

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

@@ -128,7 +128,6 @@ java_library(
         "//common/types",
         "//common/types:type_providers",
         "//common/values",
-        "//common/values:cel_value",
         "//runtime:interpretable",
         "@maven//:com_google_errorprone_error_prone_annotations",
         "@maven//:com_google_guava_guava",
@@ -334,6 +333,7 @@ java_library(
         ":localized_evaluation_exception",
         ":planned_interpretable",
         "//common/exceptions:duplicate_key",
+        "//common/exceptions:invalid_argument",
         "//runtime:evaluation_exception",
         "//runtime:interpretable",
         "@maven//:com_google_errorprone_error_prone_annotations",
@@ -379,7 +379,6 @@ java_library(
         "//common:error_codes",
         "//common/exceptions:runtime_exception",
         "//common/values",
-        "//common/values:cel_value",
         "//runtime:evaluation_exception",
         "//runtime:interpretable",
         "//runtime:resolved_overload",