Fixes reading unsigned integers and setting oneof fields in the planner runtime

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

@@ -279,7 +279,8 @@ private Message adaptValueToUint32(Object value) {
     if (value instanceof Integer) {
       return UInt32Value.of((Integer) value);
     }
-    if (value instanceof Long) {
+    // Per the spec, an int isn't a uint, unless unsigned longs are disabled.
+    if (!celOptions.enableUnsignedLongs() && value instanceof Long) {
       try {
         return UInt32Value.of(unsignedIntCheckedCast((Long) value));
       } catch (IllegalArgumentException e) {
@@ -301,7 +302,7 @@ private Message adaptValueToUint64(Object value) {
     if (value instanceof Integer) {
       return UInt64Value.of(UnsignedInts.toLong((Integer) value));
     }
-    if (value instanceof Long) {
+    if (!celOptions.enableUnsignedLongs() && value instanceof Long) {
       return UInt64Value.of((Long) value);
     }
     if (value instanceof UnsignedLong) {

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

@@ -95,7 +95,8 @@ protected Object fromWellKnownProto(MessageLiteOrBuilder message, WellKnownProto
       case STRING_VALUE:
         return normalizePrimitive(((StringValue) message).getValue());
       case UINT32_VALUE:
-        return UnsignedLong.valueOf(((UInt32Value) message).getValue());
+        return UnsignedLong.fromLongBits(
+            Integer.toUnsignedLong(((UInt32Value) message).getValue()));
       case UINT64_VALUE:
         return UnsignedLong.fromLongBits(((UInt64Value) message).getValue());
       case EMPTY:

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

@@ -15,6 +15,7 @@
 package dev.cel.common.values;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
 import com.google.common.primitives.UnsignedLong;
 import com.google.errorprone.annotations.Immutable;
 import com.google.protobuf.Any;
@@ -130,6 +131,10 @@ public Object fromProtoMessageFieldToCelValue(Message message, FieldDescriptor f
         }
 
         if (fieldDescriptor.isMapField()) {
+          FieldDescriptor keyFieldDescriptor =
+              fieldDescriptor.getMessageType().findFieldByNumber(1);
+          FieldDescriptor valueFieldDescriptor =
+              fieldDescriptor.getMessageType().findFieldByNumber(2);
           Map<Object, Object> map = new HashMap<>();
           Object mapKey;
           Object mapValue;
@@ -140,34 +145,28 @@ public Object fromProtoMessageFieldToCelValue(Message message, FieldDescriptor f
               mapValue = mapEntry.getValue();
             } else if (entry instanceof DynamicMessage) {
               DynamicMessage dynamicMessage = (DynamicMessage) entry;
-              FieldDescriptor keyFieldDescriptor =
-                  fieldDescriptor.getMessageType().findFieldByNumber(1);
-              FieldDescriptor valueFieldDescriptor =
-                  fieldDescriptor.getMessageType().findFieldByNumber(2);
               mapKey = dynamicMessage.getField(keyFieldDescriptor);
               mapValue = dynamicMessage.getField(valueFieldDescriptor);
             } else {
               throw new IllegalStateException("Unexpected map field type: " + entry);
             }
 
-            map.put(mapKey, mapValue);
+            map.put(
+                toUnsigned(keyFieldDescriptor, mapKey), toUnsigned(valueFieldDescriptor, mapValue));
           }
           return toRuntimeValue(map);
         }
 
         return toRuntimeValue(result);
       case UINT32:
       case FIXED32:
-        if (!fieldDescriptor.isRepeated()) {
-          return UnsignedLong.valueOf((int) result);
-        }
-        break;
       case UINT64:
       case FIXED64:
-        if (!fieldDescriptor.isRepeated()) {
-          return UnsignedLong.fromLongBits((long) result);
-        }
-        break;
+        // Converts elements as they're read, so selecting one doesn't copy the whole list.
+        return fieldDescriptor.isRepeated()
+            ? CelPreAdaptedList.wrap(
+                Lists.transform((List<?>) result, element -> toUnsigned(fieldDescriptor, element)))
+            : toUnsigned(fieldDescriptor, result);
       default:
         break;
     }
@@ -187,6 +186,20 @@ public Object fromProtoMessageFieldToCelValue(Message message, FieldDescriptor f
     return toRuntimeValue(result);
   }
 
+  // Protobuf returns uint32 and uint64 values as signed ints and longs.
+  private static Object toUnsigned(FieldDescriptor fieldDescriptor, Object value) {
+    switch (fieldDescriptor.getType()) {
+      case UINT32:
+      case FIXED32:
+        return UnsignedLong.fromLongBits(Integer.toUnsignedLong((Integer) value));
+      case UINT64:
+      case FIXED64:
+        return UnsignedLong.fromLongBits((Long) value);
+      default:
+        return value;
+    }
+  }
+
   private ProtoCelValueConverter(
       CelDescriptorPool celDescriptorPool, DynamicProto dynamicProto, CelOptions celOptions) {
     Preconditions.checkNotNull(celDescriptorPool);

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

@@ -36,6 +36,7 @@
 import com.google.protobuf.StringValue;
 import com.google.protobuf.Struct;
 import com.google.protobuf.Timestamp;
+import com.google.protobuf.UInt32Value;
 import com.google.protobuf.UInt64Value;
 import com.google.protobuf.Value;
 import com.google.type.Expr;
@@ -213,6 +214,31 @@ public void adaptValueToProto_asymmetricFloatDoubleConversion() {
           .isEqualTo(DoubleValue.of(1.5D));
     }
 
+    @Test
+    public void adaptValueToProto_signedUintWrappers_throws() {
+      ProtoAdapter protoAdapter = new ProtoAdapter(DYNAMIC_PROTO, CelOptions.DEFAULT);
+      String uint32TypeName = UInt32Value.getDescriptor().getFullName();
+      String uint64TypeName = UInt64Value.getDescriptor().getFullName();
+
+      assertThrows(
+          IllegalArgumentException.class, () -> protoAdapter.adaptValueToProto(1L, uint32TypeName));
+      assertThrows(
+          IllegalArgumentException.class,
+          () -> protoAdapter.adaptValueToProto(-1L, uint64TypeName));
+    }
+
+    @Test
+    @SuppressWarnings("deprecation") // Signed uints are only enabled through a deprecated option.
+    public void adaptValueToProto_unsignedLongsDisabled_acceptsSignedUintWrappers() {
+      ProtoAdapter protoAdapter =
+          new ProtoAdapter(DYNAMIC_PROTO, CelOptions.current().enableUnsignedLongs(false).build());
+
+      assertThat(protoAdapter.adaptValueToProto(1L, UInt32Value.getDescriptor().getFullName()))
+          .isEqualTo(UInt32Value.of(1));
+      assertThat(protoAdapter.adaptValueToProto(1L, UInt64Value.getDescriptor().getFullName()))
+          .isEqualTo(UInt64Value.of(1));
+    }
+
     @Test
     public void adaptValueToProto_asymmetricJsonConversion() {
       ProtoAdapter protoAdapter = new ProtoAdapter(DYNAMIC_PROTO, CelOptions.DEFAULT);

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

@@ -191,7 +191,7 @@ private enum SelectFieldTestCase {
     BOOL("single_bool", true),
     INT32("single_int32", 4L),
     INT64("single_int64", 5L),
-    UINT32("single_uint32", UnsignedLong.valueOf(1L)),
+    UINT32("single_uint32", UnsignedLong.valueOf(0xFFFFFFFFL)),
     UINT64("single_uint64", UnsignedLong.MAX_VALUE),
     FLOAT("single_float", 1.5d),
     DOUBLE("single_double", 2.5d),
@@ -203,14 +203,18 @@ private enum SelectFieldTestCase {
     TIMESTAMP("single_timestamp", Instant.ofEpochSecond(100)),
     INT32_WRAPPER("single_int32_wrapper", 5L),
     INT64_WRAPPER("single_int64_wrapper", 10L),
-    UINT32_WRAPPER("single_uint32_wrapper", UnsignedLong.valueOf(1L)),
+    UINT32_WRAPPER("single_uint32_wrapper", UnsignedLong.valueOf(0xFFFFFFFFL)),
     UINT64_WRAPPER("single_uint64_wrapper", UnsignedLong.MAX_VALUE),
     FLOAT_WRAPPER("single_float_wrapper", 7.5d),
     DOUBLE_WRAPPER("single_double_wrapper", 8.5d),
     STRING_WRAPPER("single_string_wrapper", "hello"),
     BYTES_WRAPPER("single_bytes_wrapper", CelByteString.of(new byte[] {0x02})),
     REPEATED_INT64("repeated_int64", ImmutableList.of(5L)),
+    REPEATED_UINT64("repeated_uint64", ImmutableList.of(UnsignedLong.MAX_VALUE)),
     MAP_STRING_STRING("map_string_string", ImmutableMap.of("a", "b")),
+    MAP_UINT32_UINT64(
+        "map_uint32_uint64",
+        ImmutableMap.of(UnsignedLong.valueOf(0xFFFFFFFFL), UnsignedLong.MAX_VALUE)),
     NESTED_MESSAGE(
         "standalone_message",
         ProtoMessageValue.create(
@@ -653,7 +657,7 @@ private static TestAllTypes createPopulatedTestAllTypes() {
         .setSingleBool(true)
         .setSingleInt32(4)
         .setSingleInt64(5L)
-        .setSingleUint32(1)
+        .setSingleUint32(0xFFFFFFFF)
         .setSingleUint64(UnsignedLong.MAX_VALUE.longValue())
         .setSingleFloat(1.5f)
         .setSingleDouble(2.5d)
@@ -664,14 +668,16 @@ private static TestAllTypes createPopulatedTestAllTypes() {
         .setSingleTimestamp(Timestamp.newBuilder().setSeconds(100))
         .setSingleInt32Wrapper(Int32Value.of(5))
         .setSingleInt64Wrapper(Int64Value.of(10L))
-        .setSingleUint32Wrapper(UInt32Value.of(1))
+        .setSingleUint32Wrapper(UInt32Value.of(0xFFFFFFFF))
         .setSingleUint64Wrapper(UInt64Value.of(UnsignedLong.MAX_VALUE.longValue()))
         .setSingleStringWrapper(StringValue.of("hello"))
         .setSingleFloatWrapper(FloatValue.of(7.5f))
         .setSingleDoubleWrapper(DoubleValue.of(8.5d))
         .setSingleBytesWrapper(BytesValue.of(ByteString.copyFrom(new byte[] {0x02})))
         .addRepeatedInt64(5L)
+        .addRepeatedUint64(UnsignedLong.MAX_VALUE.longValue())
         .putMapStringString("a", "b")
+        .putMapUint32Uint64(0xFFFFFFFF, UnsignedLong.MAX_VALUE.longValue())
         .setStandaloneMessage(NestedMessage.getDefaultInstance())
         .setStandaloneEnum(NestedEnum.BAR)
         .build();

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

@@ -46,7 +46,8 @@ final class EvalCreateStruct extends PlannedInterpretable {
 
   @Override
   Object evalInternal(GlobalResolver resolver, ExecutionFrame frame) {
-    Map<String, Object> fieldValues = Maps.newHashMapWithExpectedSize(keys.length);
+    // In source order, so that the last of several fields set in one oneof wins, as in protobuf.
+    Map<String, Object> fieldValues = Maps.newLinkedHashMapWithExpectedSize(keys.length);
     AccumulatedUnknowns unknowns = null;
     for (int i = 0; i < keys.length; i++) {
       Object value = EvalHelpers.evalStrictly(values[i], resolver, frame);

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

@@ -692,6 +692,23 @@ public void plan_createStruct_withContainer() throws Exception {
     assertThat(result).isEqualTo(TestAllTypes.getDefaultInstance());
   }
 
+  // With fewer fields, the oneof fields would share a HashMap bucket and keep their order.
+  @Test
+  @TestParameters({
+    "{expression: 'TestAllTypes{oneof_bool: true, single_int32: 1, single_int64: 2,"
+        + " oneof_msg: TestAllTypes.NestedMessage{}}', expectedCase: ONEOF_MSG}",
+    "{expression: 'TestAllTypes{oneof_msg: TestAllTypes.NestedMessage{}, single_int32: 1,"
+        + " single_int64: 2, oneof_bool: true}', expectedCase: ONEOF_BOOL}"
+  })
+  public void plan_createStruct_oneofSetTwice_lastFieldWins(
+      String expression, TestAllTypes.KindCase expectedCase) throws Exception {
+    CelAbstractSyntaxTree ast = compile(expression);
+
+    TestAllTypes result = (TestAllTypes) PLANNER.plan(ast).eval();
+
+    assertThat(result.getKindCase()).isEqualTo(expectedCase);
+  }
+
   @Test
   public void plan_call_zeroArgs() throws Exception {
     CelAbstractSyntaxTree ast = compile("zero()");