Align protobuf message equality and hashing with IEEE 754 spec

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

@@ -16,6 +16,8 @@
 
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import com.google.errorprone.annotations.CheckReturnValue;
 import com.google.errorprone.annotations.Immutable;
 import com.google.protobuf.Any;
@@ -26,6 +28,7 @@
 import dev.cel.common.annotations.Internal;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 
 /**
@@ -35,9 +38,11 @@
  * <p>These are the key differences between CEL's proto equality and Java's proto equality:
  *
  * <ul>
- *   <li/>NaN is not equal to itself.
- *   <li/>Any values are unpacked before comparison.
- *   <li/>If two Any values cannot be unpacked, they are compared by bytes.
+ *   <li>NaN is not equal to itself.
+ *   <li>Positive and negative zero (+0.0 and -0.0) compare as equal when both fields are present
+ *       (under proto3 implicit presence, -0.0 is serialized as present while +0.0 is unset).
+ *   <li>Any values are unpacked before comparison.
+ *   <li>If two Any values cannot be unpacked, they are compared by bytes.
  * </ul>
  *
  * <p>CEL Library Internals. Do Not Use.
@@ -56,9 +61,6 @@ public ProtoEquality(DynamicProto dynamicProto) {
   }
 
   public boolean equals(Message message1, Message message2) {
-    if (message1 == message2) {
-      return true;
-    }
     if (!message1.getDescriptorForType().equals(message2.getDescriptorForType())) {
       return false;
     }
@@ -124,22 +126,51 @@ public boolean equals(Message message1, Message message2) {
     return true;
   }
 
+  /** Returns a hash code consistent with {@link #equals(Message, Message)}. */
+  public int hashCode(Message message) {
+    if (isAny(message)) {
+      // Constant hash: unpacked Any equality ignores wire bytes, while fallback ignores type_url.
+      return ANY_FULL_NAME.hashCode();
+    }
+    return Objects.hash(
+        message.getDescriptorForType(),
+        message.getUnknownFields(),
+        Maps.transformEntries(message.getAllFields(), this::hashFieldEntry));
+  }
+
+  private int hashFieldEntry(FieldDescriptor field, Object value) {
+    if (field.isMapField()) {
+      ProtoMap protoMap = protoMap((List<?>) value);
+      return Maps.transformValues(protoMap.map(), v -> hashFieldValue(protoMap.valueField(), v))
+          .hashCode();
+    }
+    if (field.isRepeated()) {
+      return Lists.transform((List<?>) value, elem -> hashFieldValue(field, elem)).hashCode();
+    }
+    return hashFieldValue(field, value);
+  }
+
+  private int hashFieldValue(FieldDescriptor field, Object value) {
+    switch (field.getJavaType()) {
+      case MESSAGE:
+        return hashCode((Message) value);
+      case DOUBLE:
+      case FLOAT:
+        double v = ((Number) value).doubleValue();
+        // Normalize -0.0 to 0.0 so equal zeros hash alike.
+        return Double.hashCode(v == 0.0d ? 0.0d : v);
+      default:
+        return value.hashCode();
+    }
+  }
+
   private boolean equalsFieldValues(FieldDescriptor field, Object value1, Object value2) {
     switch (field.getJavaType()) {
       case MESSAGE:
         return equals((Message) value1, (Message) value2);
       case DOUBLE:
-        // fallthrough
       case FLOAT:
-        Number num1 = (Number) value1;
-        Number num2 = (Number) value2;
-        boolean num1IsNaN = Double.isNaN(num1.doubleValue());
-        boolean num2IsNaN = Double.isNaN(num2.doubleValue());
-        // handle the NaN case
-        if (num1IsNaN && num2IsNaN) {
-          return false;
-        }
-        return num1.equals(num2);
+        return ((Number) value1).doubleValue() == ((Number) value2).doubleValue();
       default:
         return value1.equals(value2);
     }

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

@@ -19,17 +19,20 @@
 import com.google.protobuf.Any;
 import com.google.protobuf.DynamicMessage;
 import com.google.protobuf.InvalidProtocolBufferException;
+import com.google.protobuf.Message;
 import com.google.protobuf.Struct;
+import com.google.protobuf.UnknownFieldSet;
 import com.google.protobuf.Value;
+import com.google.testing.junit.testparameterinjector.TestParameter;
+import com.google.testing.junit.testparameterinjector.TestParameterInjector;
 import dev.cel.expr.conformance.proto3.TestAllTypes;
 import dev.cel.expr.conformance.proto3.TestAllTypes.NestedEnum;
 import dev.cel.expr.conformance.proto3.TestAllTypes.NestedMessage;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
 
-@RunWith(JUnit4.class)
+@RunWith(TestParameterInjector.class)
 public final class ProtoEqualityTest {
 
   private DynamicProto dynamicProto;
@@ -59,13 +62,22 @@ public void notEqualsFloatFields() {
         .isFalse();
   }
 
+  @Test
+  public void notEqualsNegativeZeroFields_proto3ImplicitPresence() {
+    // Proto3 implicit presence treats -0.0 as wire-present and +0.0 as unset, matching C++/Go.
+    TestAllTypes negativeZero = TestAllTypes.newBuilder().setSingleDouble(-0.0).build();
+    TestAllTypes positiveZero = TestAllTypes.newBuilder().setSingleDouble(0.0).build();
+
+    assertThat(protoEquality.equals(negativeZero, positiveZero)).isFalse();
+  }
+
   @Test
   public void notEqualsNaNFields() {
-    assertThat(
-            protoEquality.equals(
-                TestAllTypes.newBuilder().setSingleDouble(Double.NaN).build(),
-                TestAllTypes.newBuilder().setSingleDouble(Double.NaN).build()))
-        .isFalse();
+    TestAllTypes nanDouble = TestAllTypes.newBuilder().setSingleDouble(Double.NaN).build();
+    TestAllTypes nanFloat = TestAllTypes.newBuilder().setSingleFloat(Float.NaN).build();
+
+    assertThat(protoEquality.equals(nanDouble, nanDouble)).isFalse();
+    assertThat(protoEquality.equals(nanFloat, nanFloat)).isFalse();
   }
 
   @Test
@@ -239,7 +251,10 @@ public void equalsMessageAnyFields() {
             .build();
     Any packedStruct = Any.pack(struct);
     Any packedStruct2 = Any.pack(struct);
+
     assertThat(protoEquality.equals(packedStruct, packedStruct2)).isTrue();
+    assertThat(protoEquality.hashCode(packedStruct))
+        .isEqualTo(protoEquality.hashCode(packedStruct2));
   }
 
   @Test
@@ -252,7 +267,10 @@ public void equalsMessageUnknownAnyFields() {
     Any packedStruct = Any.pack(struct);
     Any packedStruct2 = Any.pack(struct);
     packedStruct = packedStruct.toBuilder().setTypeUrl("type.googleapis.com/Unknown").build();
+
     assertThat(protoEquality.equals(packedStruct, packedStruct2)).isTrue();
+    assertThat(protoEquality.hashCode(packedStruct))
+        .isEqualTo(protoEquality.hashCode(packedStruct2));
   }
 
   @Test
@@ -274,6 +292,84 @@ public void equalsMessageDynamicAnyFields() throws InvalidProtocolBufferExceptio
             Any.getDescriptor(),
             doublePackedStruct.getValue(),
             DefaultDescriptorPool.INSTANCE.getExtensionRegistry());
+
     assertThat(protoEquality.equals(dynAny, dynAny2)).isTrue();
+    assertThat(protoEquality.hashCode(dynAny)).isEqualTo(protoEquality.hashCode(dynAny2));
+  }
+
+  @SuppressWarnings("ImmutableEnumChecker") // Test only
+  private enum EqualMessagePairTestCase {
+    REPEATED_SIGNED_ZERO(
+        TestAllTypes.newBuilder().addRepeatedDouble(-0.0).addRepeatedFloat(-0.0f).build(),
+        TestAllTypes.newBuilder().addRepeatedDouble(0.0).addRepeatedFloat(0.0f).build()),
+    MAP_KEY_ORDER(
+        TestAllTypes.newBuilder()
+            .putMapStringString("a", "1")
+            .putMapStringString("b", "10")
+            .build(),
+        TestAllTypes.newBuilder()
+            .putMapStringString("b", "10")
+            .putMapStringString("a", "1")
+            .build()),
+    SINGLE_VALUE_SIGNED_ZERO(
+        TestAllTypes.newBuilder().setSingleValue(Value.newBuilder().setNumberValue(-0.0)).build(),
+        TestAllTypes.newBuilder().setSingleValue(Value.newBuilder().setNumberValue(0.0)).build()),
+    MAP_DOUBLE_VALUE_SIGNED_ZERO(
+        TestAllTypes.newBuilder().putMapInt64Double(1L, -0.0).build(),
+        TestAllTypes.newBuilder().putMapInt64Double(1L, 0.0).build());
+
+    private final Message x;
+    private final Message y;
+
+    EqualMessagePairTestCase(Message x, Message y) {
+      this.x = x;
+      this.y = y;
+    }
+  }
+
+  @SuppressWarnings("ImmutableEnumChecker") // Test only
+  private enum DistinctMessagePairTestCase {
+    DIFFERENT_UNKNOWN_FIELDS(
+        TestAllTypes.getDefaultInstance(),
+        TestAllTypes.newBuilder()
+            .setUnknownFields(
+                UnknownFieldSet.newBuilder()
+                    .addField(999, UnknownFieldSet.Field.newBuilder().addVarint(1L).build())
+                    .build())
+            .build()),
+    DIFFERENT_REPEATED_DOUBLES(
+        TestAllTypes.newBuilder().addRepeatedDouble(1.0).build(),
+        TestAllTypes.newBuilder().addRepeatedDouble(2.0).build()),
+    DIFFERENT_MAP_VALUES(
+        TestAllTypes.newBuilder().putMapStringString("a", "1").build(),
+        TestAllTypes.newBuilder().putMapStringString("a", "2").build()),
+    DIFFERENT_NESTED_MESSAGES(
+        TestAllTypes.newBuilder()
+            .setSingleNestedMessage(NestedMessage.newBuilder().setBb(1))
+            .build(),
+        TestAllTypes.newBuilder()
+            .setSingleNestedMessage(NestedMessage.newBuilder().setBb(2))
+            .build());
+
+    private final Message x;
+    private final Message y;
+
+    DistinctMessagePairTestCase(Message x, Message y) {
+      this.x = x;
+      this.y = y;
+    }
+  }
+
+  @Test
+  public void hashCode_equalMessagesHaveEqualHash(
+      @TestParameter EqualMessagePairTestCase testCase) {
+    assertThat(protoEquality.equals(testCase.x, testCase.y)).isTrue();
+    assertThat(protoEquality.hashCode(testCase.x)).isEqualTo(protoEquality.hashCode(testCase.y));
+  }
+
+  @Test
+  public void hashCode_distinctMessagesHaveDifferentHashes(
+      @TestParameter DistinctMessagePairTestCase testCase) {
+    assertThat(protoEquality.hashCode(testCase.x)).isNotEqualTo(protoEquality.hashCode(testCase.y));
   }
 }

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

@@ -60,6 +60,17 @@ public boolean objectEquals(Object x, Object y) {
     return super.objectEquals(x, y);
   }
 
+  @Override
+  public int hashCode(Object object) {
+    if (!celOptions.disableCelStandardEquality() && celOptions.enableProtoDifferencerEquality()) {
+      object = runtimeHelpers.adaptValue(object);
+      if (object instanceof Message) {
+        return protoEquality.hashCode((Message) object);
+      }
+    }
+    return super.hashCode(object);
+  }
+
   private ProtoMessageRuntimeEquality(DynamicProto dynamicProto, CelOptions celOptions) {
     super(ProtoMessageRuntimeHelpers.create(dynamicProto, celOptions), celOptions);
     this.protoEquality = new ProtoEquality(dynamicProto);

runtime/src/test/java/dev/cel/runtime/ProtoMessageRuntimeEqualityTest.java

@@ -172,6 +172,20 @@ public void indexMap() throws Exception {
         () -> RUNTIME_EQUALITY_DEFAULT_OPTIONS.indexMap(mixedKeyMap, 1.1));
   }
 
+  @Test
+  public void hashCode_delegatesBasedOnOptions() {
+    AttributeContext hostAttr =
+        AttributeContext.newBuilder().setRequest(Request.newBuilder().setHost("localhost")).build();
+    ProtoMessageRuntimeEquality legacyWithProtoDiff =
+        ProtoMessageRuntimeEquality.create(
+            DYNAMIC_PROTO, PROTO_EQUALITY.toBuilder().disableCelStandardEquality(true).build());
+
+    assertThat(RUNTIME_EQUALITY_PROTO_EQUALITY.hashCode(hostAttr))
+        .isNotEqualTo(hostAttr.hashCode());
+    assertThat(RUNTIME_EQUALITY_EMPTY_OPTIONS.hashCode(hostAttr)).isEqualTo(hostAttr.hashCode());
+    assertThat(legacyWithProtoDiff.hashCode(hostAttr)).isEqualTo(hostAttr.hashCode());
+  }
+
   @AutoValue
   abstract static class State {
     /**
@@ -652,6 +666,19 @@ public static List<Object[]> data() {
                 .build(),
             Result.proto(/* equalsOutcome= */ true, /* diffOutcome= */ false),
           },
+          {
+            Auth.newBuilder()
+                .setClaims(
+                    Struct.newBuilder()
+                        .putFields("custom", Value.newBuilder().setNumberValue(-0.0).build()))
+                .build(),
+            Auth.newBuilder()
+                .setClaims(
+                    Struct.newBuilder()
+                        .putFields("custom", Value.newBuilder().setNumberValue(0.0).build()))
+                .build(),
+            Result.proto(/* equalsOutcome= */ false, /* diffOutcome= */ true),
+          },
 
           // Note: this is the motivating use case for converting to heterogeneous equality in
           // the future.
@@ -695,6 +722,10 @@ public void objectEquals() throws Exception {
       }
       assertThat(state.runtimeEquality().objectEquals(lhs, rhs)).isEqualTo(state.outcome());
       assertThat(state.runtimeEquality().objectEquals(rhs, lhs)).isEqualTo(state.outcome());
+      if (state.outcome() && lhs != null && rhs != null) {
+        assertThat(state.runtimeEquality().hashCode(lhs))
+            .isEqualTo(state.runtimeEquality().hashCode(rhs));
+      }
     }
   }
 }