From d8bd3028e3e94c8bb6c37ddb2a4d87abe7747c66 Mon Sep 17 00:00:00 2001
From: Michael Groover <mpgroover@google.com>
Date: Wed, 27 Jan 2021 21:24:09 -0800
Subject: [PATCH] Document Signature equals/hash should not include flags

Some of the methods in PackageParser.SigningDetails rely on the
Signature class not including the flags / capabilities when
computing the hash code or performing equality checks. This commit
adds comments to the Signature class to highlight this and adds
tests to detect if this behavior is changed.

Bug: 176814921
Test: atest SignatureTest
Change-Id: I28c591af559568d498449f1c6cbceb1c292d2ad5
---
 core/java/android/content/pm/Signature.java   |  4 +++
 .../src/android/content/pm/SignatureTest.java | 26 +++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/core/java/android/content/pm/Signature.java b/core/java/android/content/pm/Signature.java
index 02fb06b808a22..bce4b872b8a6d 100644
--- a/core/java/android/content/pm/Signature.java
+++ b/core/java/android/content/pm/Signature.java
@@ -256,6 +256,8 @@ public class Signature implements Parcelable {
         try {
             if (obj != null) {
                 Signature other = (Signature)obj;
+                // Note, some classes, such as PackageParser.SigningDetails, rely on equals
+                // only comparing the mSignature arrays without the flags.
                 return this == other || Arrays.equals(mSignature, other.mSignature);
             }
         } catch (ClassCastException e) {
@@ -268,6 +270,8 @@ public class Signature implements Parcelable {
         if (mHaveHashCode) {
             return mHashCode;
         }
+        // Note, similar to equals some classes rely on the hash code not including
+        // the flags for Set membership checks.
         mHashCode = Arrays.hashCode(mSignature);
         mHaveHashCode = true;
         return mHashCode;
diff --git a/core/tests/coretests/src/android/content/pm/SignatureTest.java b/core/tests/coretests/src/android/content/pm/SignatureTest.java
index f0b4af6fc44f9..19458da796e8f 100644
--- a/core/tests/coretests/src/android/content/pm/SignatureTest.java
+++ b/core/tests/coretests/src/android/content/pm/SignatureTest.java
@@ -54,6 +54,32 @@ public class SignatureTest extends TestCase {
         assertFalse(Signature.areEffectiveMatch(asArray(A, M), asArray(A, B)));
     }
 
+    public void testHashCode_doesNotIncludeFlags() throws Exception {
+        // Some classes rely on the hash code not including the flags / capabilities for the signer
+        // to verify Set membership. This test verifies two signers with the same signature but
+        // different flags have the same hash code.
+        Signature signatureAWithAllCaps = new Signature(A.toCharsString());
+        // There are currently 5 capabilities that can be assigned to a previous signer, although
+        // for the purposes of this test all that matters is that the two flag values are distinct.
+        signatureAWithAllCaps.setFlags(31);
+        Signature signatureAWithNoCaps = new Signature(A.toCharsString());
+        signatureAWithNoCaps.setFlags(0);
+
+        assertEquals(signatureAWithAllCaps.hashCode(), signatureAWithNoCaps.hashCode());
+    }
+
+    public void testEquals_doesNotIncludeFlags() throws Exception {
+        // Similar to above some classes rely on equals only comparing the signature arrays
+        // for equality without including the flags. This test verifies two signers with the
+        // same signature but different flags are still considered equal.
+        Signature signatureAWithAllCaps = new Signature(A.toCharsString());
+        signatureAWithAllCaps.setFlags(31);
+        Signature signatureAWithNoCaps = new Signature(A.toCharsString());
+        signatureAWithNoCaps.setFlags(0);
+
+        assertEquals(signatureAWithAllCaps, signatureAWithNoCaps);
+    }
+
     private static Signature[] asArray(Signature... s) {
         return s;
     }