8307143: CredentialsCache.cacheName should not be static

wangweij · wangweij · commit 31a1f9c49376 · 2024-04-16T01:03:14.000Z
Reviewed-by: valeriep
diff --git a/src/java.security.jgss/share/classes/sun/security/krb5/internal/ccache/CredentialsCache.java b/src/java.security.jgss/share/classes/sun/security/krb5/internal/ccache/CredentialsCache.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -44,8 +44,6 @@
  * @author Yanni Zhang
  */
 public abstract class CredentialsCache {
-    static String cacheName;
-
     public static CredentialsCache getInstance(PrincipalName principal) {
         return FileCredentialsCache.acquireInstance(principal, null);
     }
@@ -105,9 +103,7 @@ public static CredentialsCache create(PrincipalName principal) {
         return (FileCredentialsCache.New(principal));
     }
 
-    public static String cacheName() {
-        return cacheName;
-    }
+    public abstract String cacheName();
 
     public abstract PrincipalName getPrimaryPrincipal();
     public abstract void update(Credentials c);
diff --git a/src/java.security.jgss/share/classes/sun/security/krb5/internal/ccache/FileCredentialsCache.java b/src/java.security.jgss/share/classes/sun/security/krb5/internal/ccache/FileCredentialsCache.java
@@ -66,16 +66,18 @@
  */
 
 public class FileCredentialsCache extends CredentialsCache
-    implements FileCCacheConstants {
+        implements FileCCacheConstants {
     public int version;
     public Tag tag; // optional
     public PrincipalName primaryPrincipal;
     private Vector<Credentials> credentialsList;
 
+    private final String localCacheName;
+
     public static synchronized FileCredentialsCache acquireInstance(
                 PrincipalName principal, String cache) {
         try {
-            FileCredentialsCache fcc = new FileCredentialsCache();
+            String cacheName;
             if (cache == null) {
                 cacheName = FileCredentialsCache.getDefaultCacheName();
             } else {
@@ -85,10 +87,11 @@ public static synchronized FileCredentialsCache acquireInstance(
                 // invalid cache name or the file doesn't exist
                 return null;
             }
+            FileCredentialsCache fcc = new FileCredentialsCache(cacheName);
             if (principal != null) {
                 fcc.primaryPrincipal = principal;
             }
-            fcc.load(cacheName);
+            fcc.load();
             return fcc;
         } catch (IOException | KrbException e) {
             // we don't handle it now, instead we return a null at the end.
@@ -106,13 +109,13 @@ public static FileCredentialsCache acquireInstance() {
     static synchronized FileCredentialsCache New(PrincipalName principal,
                                                 String name) {
         try {
-            FileCredentialsCache fcc = new FileCredentialsCache();
-            cacheName = FileCredentialsCache.checkValidation(name);
+            String cacheName = FileCredentialsCache.checkValidation(name);
             if (cacheName == null) {
-                // invalid cache name or the file doesn't exist
+                // invalid cache name
                 return null;
             }
-            fcc.init(principal, cacheName);
+            FileCredentialsCache fcc = new FileCredentialsCache(cacheName);
+            fcc.init(principal);
             return fcc;
         }
         catch (IOException | KrbException e) {
@@ -122,9 +125,9 @@ static synchronized FileCredentialsCache New(PrincipalName principal,
 
     static synchronized FileCredentialsCache New(PrincipalName principal) {
         try {
-            FileCredentialsCache fcc = new FileCredentialsCache();
-            cacheName = FileCredentialsCache.getDefaultCacheName();
-            fcc.init(principal, cacheName);
+            String cacheName = FileCredentialsCache.getDefaultCacheName();
+            FileCredentialsCache fcc = new FileCredentialsCache(cacheName);
+            fcc.init(principal);
             return fcc;
         }
         catch (IOException | KrbException e) {
@@ -135,29 +138,29 @@ static synchronized FileCredentialsCache New(PrincipalName principal) {
         return null;
     }
 
-    private FileCredentialsCache() {
+    private FileCredentialsCache(String cacheName) {
+        localCacheName = cacheName;
     }
 
-    boolean exists(String cache) {
-        File file = new File(cache);
-        return file.exists();
+    @Override
+    public String cacheName() {
+        return localCacheName;
     }
 
-    synchronized void init(PrincipalName principal, String name)
-        throws IOException, KrbException {
+    synchronized void init(PrincipalName principal)
+            throws IOException, KrbException {
         primaryPrincipal = principal;
-        try (FileOutputStream fos = new FileOutputStream(name);
+        try (FileOutputStream fos = new FileOutputStream(localCacheName);
              CCacheOutputStream cos = new CCacheOutputStream(fos)) {
             version = KRB5_FCC_FVNO_3;
             cos.writeHeader(primaryPrincipal, version);
         }
-        load(name);
+        load();
     }
 
-    synchronized void load(String name) throws IOException, KrbException {
-        PrincipalName p;
-        try (FileInputStream fis = new FileInputStream(name);
-             CCacheInputStream cis = new CCacheInputStream(fis)) {
+    synchronized void load() throws IOException, KrbException {
+        try (FileInputStream fis = new FileInputStream(localCacheName);
+                CCacheInputStream cis = new CCacheInputStream(fis)) {
             version = cis.readVersion();
             if (version == KRB5_FCC_FVNO_4) {
                 tag = cis.readTag();
@@ -167,14 +170,15 @@ synchronized void load(String name) throws IOException, KrbException {
                     cis.setNativeByteOrder();
                 }
             }
-            p = cis.readPrincipal(version);
+            PrincipalName p = cis.readPrincipal(version);
 
             if (primaryPrincipal != null) {
                 if (!(primaryPrincipal.match(p))) {
                     throw new IOException("Primary principals don't match.");
                 }
-            } else
+            } else {
                 primaryPrincipal = p;
+            }
             credentialsList = new Vector<>();
             while (cis.available() > 0) {
                 Object cred = cis.readCred(version);
@@ -245,8 +249,8 @@ public synchronized PrincipalName getPrimaryPrincipal() {
      * Saves the credentials cache file to the disk.
      */
     public synchronized void save() throws IOException, Asn1Exception {
-        try (FileOutputStream fos = new FileOutputStream(cacheName);
-             CCacheOutputStream cos = new CCacheOutputStream(fos)) {
+        try (FileOutputStream fos = new FileOutputStream(localCacheName);
+                CCacheOutputStream cos = new CCacheOutputStream(fos)) {
             cos.writeHeader(primaryPrincipal, version);
             Credentials[] tmp;
             if ((tmp = getCredsList()) != null) {
@@ -533,12 +537,10 @@ public static String checkValidation(String name) {
                 // get absolute directory
                 File temp = new File(fCheck.getParent());
                 // test if the directory exists
-                if (!(temp.isDirectory()))
+                if (!(temp.isDirectory())) {
                     fullname = null;
-                temp = null;
+                }
             }
-            fCheck = null;
-
         } catch (IOException e) {
             fullname = null; // invalid name
         }
@@ -554,7 +556,6 @@ private static String exec(String c) {
         }
         final String[] command = v.toArray(new String[0]);
         try {
-
             @SuppressWarnings("removal")
             Process p =
                 java.security.AccessController.doPrivileged
@@ -582,13 +583,15 @@ private static String exec(String c) {
                 while ((s1 = commandResult.readLine()) != null) {
                     if (s1.length() >= 11) {
                         if ((s1.substring(0, 11)).equalsIgnoreCase
-                            ("KRB5CCNAME=")) {
+                                ("KRB5CCNAME=")) {
                             s1 = s1.substring(11);
                             break;
                         }
                     }
                 }
-            } else     s1 = commandResult.readLine();
+            } else {
+                s1 = commandResult.readLine();
+            }
             commandResult.close();
             return s1;
         } catch (Exception e) {
diff --git a/src/java.security.jgss/share/classes/sun/security/krb5/internal/ccache/MemoryCredentialsCache.java b/src/java.security.jgss/share/classes/sun/security/krb5/internal/ccache/MemoryCredentialsCache.java
diff --git a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java
@@ -144,11 +144,12 @@ public int run(String[] args) {
         switch (action) {
         case 'c':
             if (name == null) {
-                target = CredentialsCache.getInstance();
-                name = CredentialsCache.cacheName();
-            } else
+                CredentialsCache cc = CredentialsCache.getInstance();
+                target = cc;
+                name = cc.cacheName();
+            } else {
                 target = CredentialsCache.getInstance(name);
-
+            }
             if (target != null) {
                 return displayCache();
             } else {
@@ -172,8 +173,9 @@ public int run(String[] args) {
                 printHelp();
                 return -1;
             } else {
-                target = CredentialsCache.getInstance();
-                name = CredentialsCache.cacheName();
+                CredentialsCache cc = CredentialsCache.getInstance();
+                target = cc;
+                name = cc.cacheName();
                 if (target != null) {
                     return displayCache();
                 } else {
diff --git a/test/jdk/sun/security/krb5/ccache/TwoFiles.java b/test/jdk/sun/security/krb5/ccache/TwoFiles.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8307143
+ * @summary CredentialsCache.cacheName should not be static
+ * @modules java.security.jgss/sun.security.krb5
+ *          java.security.jgss/sun.security.krb5.internal.ccache
+ * @library /test/lib
+ */
+
+import jdk.test.lib.Asserts;
+import sun.security.krb5.PrincipalName;
+import sun.security.krb5.internal.ccache.CredentialsCache;
+
+public class TwoFiles {
+    public static void main(String[] args) throws Exception {
+        PrincipalName pn = new PrincipalName("me@HERE");
+        CredentialsCache cc1 = CredentialsCache.create(pn, "cc1");
+        CredentialsCache cc2 = CredentialsCache.create(pn, "cc2");
+        // name is canonicalized
+        Asserts.assertTrue(cc1.cacheName().endsWith("cc1"), cc1.cacheName());
+        Asserts.assertTrue(cc2.cacheName().endsWith("cc2"), cc2.cacheName());
+    }
+}
