Reject Java nulls uniformly in map and list adaptation

PiperOrigin-RevId: 988453145

Author: l46kok

common/src/main/java/dev/cel/common/values/BUILD.bazel

@@ -167,6 +167,7 @@ java_library(
         ":preadapted_list",
         "//:auto_value",
         "//common/annotations",
+        "//common/exceptions:invalid_argument",
         "//common/types",
         "//common/types:type_providers",
         "@maven//:com_google_errorprone_error_prone_annotations",
@@ -218,6 +219,7 @@ cel_android_library(
         ":preadapted_list_android",
         "//:auto_value",
         "//common/annotations",
+        "//common/exceptions:invalid_argument",
         "//common/types:type_providers_android",
         "//common/types:types_android",
         "@maven//:com_google_errorprone_error_prone_annotations",

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

@@ -17,15 +17,18 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.errorprone.annotations.Immutable;
 import dev.cel.common.annotations.Internal;
+import dev.cel.common.exceptions.CelInvalidArgumentException;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.RandomAccess;
 import java.util.function.Function;
+import org.jspecify.annotations.Nullable;
 
 /**
  * {@code CelValueConverter} handles bidirectional conversion between native Java objects to {@link
@@ -74,7 +77,7 @@ protected Object mapContainer(Object value, Function<Object, Object> mapper) {
     if (value instanceof List && value instanceof RandomAccess) {
       List<Object> list = (List<Object>) value;
       for (int i = 0; i < list.size(); i++) {
-        Object element = list.get(i);
+        Object element = checkListElement(list.get(i), i);
         Object mapped = mapper.apply(element);
 
         if (mapped != element) {
@@ -85,7 +88,7 @@ protected Object mapContainer(Object value, Function<Object, Object> mapper) {
           }
           builder.add(mapped);
           for (int j = i + 1; j < list.size(); j++) {
-            builder.add(mapper.apply(list.get(j)));
+            builder.add(mapper.apply(checkListElement(list.get(j), j)));
           }
           return builder.build();
         }
@@ -100,8 +103,9 @@ protected Object mapContainer(Object value, Function<Object, Object> mapper) {
       Collection<Object> collection = (Collection<Object>) value;
       ImmutableList.Builder<Object> builder =
           ImmutableList.builderWithExpectedSize(collection.size());
+      int index = 0;
       for (Object element : collection) {
-        builder.add(mapper.apply(element));
+        builder.add(mapper.apply(checkListElement(element, index++)));
       }
       return builder.build();
     }
@@ -112,6 +116,7 @@ protected Object mapContainer(Object value, Function<Object, Object> mapper) {
 
       while (iterator.hasNext()) {
         Map.Entry<Object, Object> entry = iterator.next();
+        checkMapEntry(entry);
         Object mappedKey = mapper.apply(entry.getKey());
         Object mappedValue = mapper.apply(entry.getValue());
 
@@ -128,6 +133,7 @@ protected Object mapContainer(Object value, Function<Object, Object> mapper) {
           builder.put(mappedKey, mappedValue);
           while (iterator.hasNext()) {
             Map.Entry<Object, Object> nextEntry = iterator.next();
+            checkMapEntry(nextEntry);
             builder.put(mapper.apply(nextEntry.getKey()), mapper.apply(nextEntry.getValue()));
           }
           return builder.buildOrThrow();
@@ -162,6 +168,59 @@ public Object toRuntimeValue(Object value) {
     return normalizePrimitive(value);
   }
 
+  /**
+   * Adapts {@code value} for an intermediate field selection hop.
+   *
+   * <p>{@link Map} instances are returned as-is to avoid O(N) whole-map normalization per hop; the
+   * accessed entry is validated on lookup via {@link #findMapValue} or {@link #containsMapKey}.
+   * Callers materializing a final evaluation result must use {@link #toRuntimeValue} instead.
+   */
+  public final Object toTraversalTarget(Object value) {
+    if (value instanceof Map) {
+      return value;
+    }
+
+    return toRuntimeValue(value);
+  }
+
+  /**
+   * Returns the unadapted value bound to {@code key} in {@code map}, or {@link Optional#empty()} if
+   * absent.
+   *
+   * @throws CelInvalidArgumentException if {@code key} is bound to {@code null}.
+   */
+  public static Optional<Object> findMapValue(Map<?, ?> map, Object key) {
+    Object value = map.get(key);
+    if (value != null) {
+      return Optional.of(value);
+    }
+
+    if (map.containsKey(key)) {
+      throw new CelInvalidArgumentException(
+          String.format("Map value cannot be null for key: %s", key));
+    }
+
+    return Optional.empty();
+  }
+
+  /**
+   * Returns whether {@code key} is present in {@code map}.
+   *
+   * @throws CelInvalidArgumentException if {@code key} is bound to {@code null}.
+   */
+  public static boolean containsMapKey(Map<?, ?> map, Object key) {
+    if (map.get(key) != null) {
+      return true;
+    }
+
+    if (map.containsKey(key)) {
+      throw new CelInvalidArgumentException(
+          String.format("Map value cannot be null for key: %s", key));
+    }
+
+    return false;
+  }
+
   protected Object normalizePrimitive(Object value) {
     Preconditions.checkNotNull(value);
 
@@ -196,6 +255,28 @@ private Object unwrap(CelValue celValue) {
     return celValue.value();
   }
 
+  private static void checkMapEntry(Map.Entry<?, ?> entry) {
+    Object key = entry.getKey();
+    if (key == null) {
+      throw new CelInvalidArgumentException("Map key cannot be null.");
+    }
+
+    if (entry.getValue() == null) {
+      throw new CelInvalidArgumentException(
+          String.format("Map value cannot be null for key: %s", key));
+    }
+  }
+
+  @CanIgnoreReturnValue
+  private static Object checkListElement(@Nullable Object element, int index) {
+    if (element == null) {
+      throw new CelInvalidArgumentException(
+          String.format("List element cannot be null at index: %d", index));
+    }
+
+    return element;
+  }
+
   protected CelValueConverter() {
     this.maybeUnwrapFunction = this::maybeUnwrap;
     this.toRuntimeValueFunction = this::toRuntimeValue;

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

@@ -105,23 +105,13 @@ public Set<Entry<Object, Object>> entrySet() {
 
   @Override
   public Object select(Object field) {
-    Object val = internalMap.get(field);
-    if (val != null) {
-      return val;
-    }
-    if (!internalMap.containsKey(field)) {
-      throw CelAttributeNotFoundException.forMissingMapKey(field.toString());
-    }
-    throw CelAttributeNotFoundException.of(
-        String.format("Map value cannot be null for key: %s", field));
+    return CelValueConverter.findMapValue(internalMap, field)
+        .orElseThrow(() -> CelAttributeNotFoundException.forMissingMapKey(field.toString()));
   }
 
   @Override
   public Optional<?> find(Object field) {
-    if (internalMap.containsKey(field)) {
-      return Optional.ofNullable(internalMap.get(field));
-    }
-    return Optional.empty();
+    return CelValueConverter.findMapValue(internalMap, field);
   }
 
   @Override

common/src/test/java/dev/cel/common/values/BUILD.bazel

@@ -14,8 +14,10 @@ java_library(
         "//bundle:cel",
         "//common:cel_ast",
         "//common:cel_descriptor_util",
+        "//common:error_codes",
         "//common:options",
         "//common/exceptions:attribute_not_found",
+        "//common/exceptions:invalid_argument",
         "//common/internal:cel_descriptor_pools",
         "//common/internal:cel_lite_descriptor_pool",
         "//common/internal:default_lite_descriptor_pool",