[grid][java] do not rely on .hashCode for .equals (#12361)

joerg1985 · diemol · web-flow · commit 6865f1cc9b8f · 2023-07-17T11:16:17.000+02:00
Co-authored-by: Diego Molina &lt;diemol@users.noreply.github.com&gt;
diff --git a/java/src/org/openqa/selenium/SharedCapabilitiesMethods.java b/java/src/org/openqa/selenium/SharedCapabilitiesMethods.java
@@ -17,31 +17,39 @@
 
 package org.openqa.selenium;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.IdentityHashMap;
 import java.util.Map;
-import java.util.Objects;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.openqa.selenium.logging.LogLevelMapping;
 import org.openqa.selenium.logging.LoggingPreferences;
 
 class SharedCapabilitiesMethods {
 
+  private static final String[] EMPTY_ARRAY = new String[0];
+
   private SharedCapabilitiesMethods() {
     // Utility class
   }
 
   static int hashCode(Capabilities caps) {
-    return caps.getCapabilityNames().stream()
-        .sorted()
-        .mapToInt(name -> Objects.hash(name, caps.getCapability(name)))
-        .reduce(0, (l, r) -> l ^ r);
+    String[] sortedNames = caps.getCapabilityNames().toArray(EMPTY_ARRAY);
+    Arrays.sort(sortedNames, String::compareTo);
+    // we only use the names to generate a hash code, this might result in hash collisions. thanks to the
+    // moz:firefoxOptions, goog:chromeOptions and ms:edgeOptions, these hash collisions should not happen too often.
+    return Arrays.hashCode(sortedNames);
   }
 
   static boolean equals(Capabilities left, Capabilities right) {
-    return left.hashCode() == right.hashCode();
+    if (left == right) {
+      return true;
+    }
+    // deeply compare the keys & values, usually only called when the hash codes of two objects are identical.
+    // note: there should be no arrays (directly or nested) inside the map, otherwise the equals will not work.
+    return left.asMap().equals(right.asMap());
   }
 
   static void setCapability(Map<String, Object> caps, String key, Object value) {
diff --git a/java/test/org/openqa/selenium/CapabilitiesTest.java b/java/test/org/openqa/selenium/CapabilitiesTest.java
@@ -129,4 +129,13 @@ void ensureHashCodesAreEqual() {
     assertThat(one.hashCode()).isEqualTo(three.hashCode());
     assertThat(two.hashCode()).isEqualTo(three.hashCode());
   }
+
+  @Test
+  void ensureEqualHashCodesMightBeNotEqual() {
+    Capabilities one = new ImmutableCapabilities("key", "DB");
+    Capabilities two = new ImmutableCapabilities("key", "Ca");
+
+    assertThat(one.hashCode()).isEqualTo(two.hashCode());
+    assertThat(one).isNotEqualTo(two);
+  }
 }

Original file line number	Diff line number	Diff line change
`@@ -129,4 +129,13 @@ void ensureHashCodesAreEqual() {`
`129`	`129`	`assertThat(one.hashCode()).isEqualTo(three.hashCode());`
`130`	`130`	`assertThat(two.hashCode()).isEqualTo(three.hashCode());`
`131`	`131`	`}`
	`132`	`+`
	`133`	`+ @Test`
	`134`	`+ void ensureEqualHashCodesMightBeNotEqual() {`
	`135`	`+ Capabilities one = new ImmutableCapabilities("key", "DB");`
	`136`	`+ Capabilities two = new ImmutableCapabilities("key", "Ca");`
	`137`	`+`
	`138`	`+ assertThat(one.hashCode()).isEqualTo(two.hashCode());`
	`139`	`+ assertThat(one).isNotEqualTo(two);`
	`140`	`+ }`
`132`	`141`	`}`