[jdk18] RFR: 8279222: Incorrect legacyMap.get in java.security.Provider after JDK-8276660 [v2]

[Valerie Peng]

On Thu, 23 Dec 2021 18:51:22 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> src/java.base/share/classes/java/security/Provider.java line 1152:
>> 
>>> 1150:                 case ADD:
>>> 1151:                     // clean up old alias if present
>>> 1152:                     Service prevAliasService = legacyMap.get(aliasKey);
>> 
>> The change looks okay but I'm surprised this wasn't caught by tests. 
>> @valeriepeng - would it be feasible to have a test that removes an old alias so that this code is exercised by tests?
>
> Hmm, this "ADD" case should be covered by existing regression tests. I will take a look. Thanks.

Hmm, existing test does add an alias, the legacyMap.get() would returns null upon 'aliasAlg' (type: String). To test this particular scenario, I enhanced existing regression to re-assign an alias and then check if the returned impl is as expected. Here is the proposed test patch:

> diff --git a/test/jdk/java/security/Provider/CaseSensitiveServices.java b/test/jdk/java/security/Provider/CaseSensitiveServices.java
index 2a6676b4f58..5b0e0f1fb0c 100644
--- a/test/jdk/java/security/Provider/CaseSensitiveServices.java
+++ b/test/jdk/java/security/Provider/CaseSensitiveServices.java
@@ -36,9 +36,12 @@ public class CaseSensitiveServices extends Provider {
         super("Foo", "1.0", null);
         put("MessageDigest.Foo", "com.Foo");
         put("mESSAGEdIGEST.fOO xYz", "aBc");
-        put("ALg.aliaS.MESSAGEdigest.Fu", "FoO");
+        // first assign the DEF alias to algorithm Foo
+        put("ALg.aliaS.MESSAGEdigest.DEF", "FoO");
         put("messageDigest.Bar", "com.Bar");
         put("MESSAGEDIGEST.BAZ", "com.Baz");
+        // reassign the DEF alias to algorithm Bar
+        put("ALg.aliaS.MESSAGEdigest.DEF", "Bar");
     }

     public static void main(String[] args) throws Exception {
@@ -47,12 +50,18 @@ public class CaseSensitiveServices extends Provider {
         if (p.getServices().size() != 3) {
             throw new Exception("services.size() should be 3");
         }
+
         Service s = testService(p, "MessageDigest", "fOO");
         String val = s.getAttribute("Xyz");
         if ("aBc".equals(val) == false) {
             throw new Exception("Wrong value: " + val);
         }
-        testService(p, "MessageDigest", "fU");
+
+        // test Service alias DEF and its associated impl is Bar
+        s = testService(p, "MessageDigest", "DeF");
+        if (s.getAttribute("Xyz") != null) {
+            throw new Exception("DEF mapped to the wrong impl");
+        }
         testService(p, "MessageDigest", "BAR");
         testService(p, "MessageDigest", "baz");
         System.out.println("OK");
@@ -67,5 +76,4 @@ public class CaseSensitiveServices extends Provider {
         }
         return s;
     }
-
 }

-------------

PR: https://git.openjdk.java.net/jdk18/pull/70