8231950: keytool -ext camel-case shorthand not working
authorweijun
Mon, 04 Nov 2019 14:26:18 +0800
changeset 58902 197238c30630
parent 58901 2700c409ff10
child 58903 eeb1c0da2126
8231950: keytool -ext camel-case shorthand not working Reviewed-by: mullan
src/java.base/share/classes/sun/security/tools/keytool/Main.java
test/jdk/sun/security/tools/keytool/ExtOptionCamelCase.java
test/jdk/sun/security/tools/keytool/KeyToolTest.java
--- a/src/java.base/share/classes/sun/security/tools/keytool/Main.java	Sun Nov 03 18:02:29 2019 -0500
+++ b/src/java.base/share/classes/sun/security/tools/keytool/Main.java	Mon Nov 04 14:26:18 2019 +0800
@@ -58,6 +58,7 @@
 import java.text.Collator;
 import java.text.MessageFormat;
 import java.util.*;
+import java.util.function.BiFunction;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.math.BigInteger;
@@ -4109,15 +4110,51 @@
     }
 
     /**
-     * Match a command (may be abbreviated) with a command set.
-     * @param s the command provided
+     * Match a command with a command set. The match can be exact, or
+     * partial, or case-insensitive.
+     *
+     * @param s the command provided by user
      * @param list the legal command set. If there is a null, commands after it
-     * are regarded experimental, which means they are supported but their
-     * existence should not be revealed to user.
+     *      are regarded experimental, which means they are supported but their
+     *      existence should not be revealed to user.
      * @return the position of a single match, or -1 if none matched
      * @throws Exception if s is ambiguous
      */
     private static int oneOf(String s, String... list) throws Exception {
+
+        // First, if there is an exact match, returns it.
+        int res = oneOfMatch((a,b) -> a.equals(b), s, list);
+        if (res >= 0) {
+            return res;
+        }
+
+        // Second, if there is one single camelCase or prefix match, returns it.
+        // This regex substitution removes all lowercase letters not at the
+        // beginning, so "keyCertSign" becomes "kCS".
+        res = oneOfMatch((a,b) -> a.equals(b.replaceAll("(?<!^)[a-z]", ""))
+                || b.startsWith(a), s, list);
+        if (res >= 0) {
+            return res;
+        }
+
+        // Finally, retry the 2nd step ignoring case
+        return oneOfMatch((a,b) -> a.equalsIgnoreCase(b.replaceAll("(?<!^)[a-z]", ""))
+                || b.toUpperCase(Locale.ROOT).startsWith(a.toUpperCase(Locale.ROOT)),
+                s, list);
+    }
+
+    /**
+     * Match a command with a command set.
+     *
+     * @param matcher a BiFunction which returns {@code true} if the 1st
+     *               argument (user input) matches the 2nd one (full command)
+     * @param s the command provided by user
+     * @param list the legal command set
+     * @return the position of a single match, or -1 if none matched
+     * @throws Exception if s is ambiguous
+     */
+    private static int oneOfMatch(BiFunction<String,String,Boolean> matcher,
+            String s, String... list) throws Exception {
         int[] match = new int[list.length];
         int nmatch = 0;
         int experiment = Integer.MAX_VALUE;
@@ -4127,25 +4164,8 @@
                 experiment = i;
                 continue;
             }
-            if (one.toLowerCase(Locale.ENGLISH)
-                    .startsWith(s.toLowerCase(Locale.ENGLISH))) {
+            if (matcher.apply(s, one)) {
                 match[nmatch++] = i;
-            } else {
-                StringBuilder sb = new StringBuilder();
-                boolean first = true;
-                for (char c: one.toCharArray()) {
-                    if (first) {
-                        sb.append(c);
-                        first = false;
-                    } else {
-                        if (!Character.isLowerCase(c)) {
-                            sb.append(c);
-                        }
-                    }
-                }
-                if (sb.toString().equalsIgnoreCase(s)) {
-                    match[nmatch++] = i;
-                }
             }
         }
         if (nmatch == 0) {
@@ -4159,7 +4179,7 @@
             }
             StringBuilder sb = new StringBuilder();
             MessageFormat form = new MessageFormat(rb.getString
-                ("command.{0}.is.ambiguous."));
+                    ("command.{0}.is.ambiguous."));
             Object[] source = {s};
             sb.append(form.format(source));
             sb.append("\n    ");
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/sun/security/tools/keytool/ExtOptionCamelCase.java	Mon Nov 04 14:26:18 2019 +0800
@@ -0,0 +1,261 @@
+/*
+ * Copyright (c) 2019, 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 8231950
+ * @modules java.base/sun.security.tools.keytool
+ *          java.base/sun.security.util
+ *          java.base/sun.security.x509
+ * @compile -XDignore.symbol.file ExtOptionCamelCase.java
+ * @summary keytool -ext camel-case shorthand not working
+ */
+
+import sun.security.tools.keytool.Main;
+import sun.security.util.DerValue;
+import sun.security.x509.BasicConstraintsExtension;
+import sun.security.x509.CertificateExtensions;
+import sun.security.x509.Extension;
+import sun.security.x509.KeyUsageExtension;
+
+import java.io.ByteArrayOutputStream;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.security.KeyPairGenerator;
+import java.security.PublicKey;
+import java.util.List;
+
+public class ExtOptionCamelCase {
+    static Method createV3Extensions;
+    static Constructor<Main> ctor;
+    static PublicKey pk;
+    static Method oneOf;
+
+    public static void main(String[] args) throws Exception {
+
+        prepare();
+
+        // Unseen ext name
+        testCreateFail("abc");
+
+        // camelCase match, both cases work
+        testCreate("bc", BasicConstraintsExtension.class);
+        testCreate("BC", BasicConstraintsExtension.class);
+
+        // Prefix match
+        testCreate("BasicC", BasicConstraintsExtension.class);
+
+        // Ambiguous, digitalSignature or dataEncipherment?
+        testCreateFail("ku=d");
+
+        // prefix match
+        testCreate("ku:c=dig", KeyUsageExtension.class,
+                x -> x.get(KeyUsageExtension.DIGITAL_SIGNATURE));
+
+        // camelCase match
+        testCreate("ku=kE", KeyUsageExtension.class,
+                x -> x.get(KeyUsageExtension.KEY_ENCIPHERMENT));
+
+        // camelCase match must be only 1st+CAPITALs
+        testCreateFail("ku=KeUs");
+
+        // camelCase match, must be only 1st + all CAPITALs
+        testCreate("ku=kCS", KeyUsageExtension.class,
+                x -> x.get(KeyUsageExtension.KEY_CERTSIGN));
+
+        // ... not all CAPITALs
+        testCreateFail("ku=kC");
+
+        // ... has lowercase letters
+        testCreateFail("ku=keCeSi");
+
+        // Ambiguous, keyAgreement or keyCertSign
+        testCreateFail("ku:c=ke");
+
+        // camelCase natch
+        testCreate("ku:c=dE", KeyUsageExtension.class,
+                x -> x.get(KeyUsageExtension.DATA_ENCIPHERMENT));
+        // prefix match
+        testCreate("ku:c=de", KeyUsageExtension.class,
+                x -> x.get(KeyUsageExtension.DECIPHER_ONLY));
+
+        // camelCase match
+        testCreate("ku:c=kA", KeyUsageExtension.class,
+                x -> x.get(KeyUsageExtension.KEY_AGREEMENT));
+
+        // camelCase match, fallback
+        testCreate("ku:c=ka", KeyUsageExtension.class,
+                x -> x.get(KeyUsageExtension.KEY_AGREEMENT));
+
+        // Testing oneOf() directly
+        testOneOf("a", -1, "b", "c"); // -1 means not found
+        testOneOf("a", -2, "ab", "ac"); // -2 means ambiguous
+
+        testOneOf("a", 0, "a", "ac"); //exact match
+        testOneOf("a", 0, "a", "b");
+        testOneOf("ac", 1, "a", "ac");
+
+        testOneOf("a", 0, "abc", "bcd");
+        testOneOf("ab", 0, "abc", "ABC");
+        testOneOf("ab", 0, "abc", "aBC");
+        testOneOf("ab", 0, "abc", "Abc");
+        testOneOf("AB", 1, "abc", "ABC");
+        testOneOf("aB", 0, "abcBcd", "abcDef");
+        testOneOf("ab", -2, "abcBcd", "abcDef");
+        testOneOf("aB", -2, "abcBcdEfg", "abcDef");
+
+        testOneOf("ab", 0, "abcDef", "axyBuv");
+        testOneOf("aB", 1, "abcDef", "axyBuv");
+        testOneOf("a", -2, "abcDef", "axyBuv");
+
+        testOneOf("aBC", -1, "a12BxyCuvDmn"); // 12 is not removed
+        testOneOf("a12BCD", 0, "a12BxyCuvDmn");
+        testOneOf("a12BC", -1, "a12BxyCuvDmn"); // must be full
+
+        // Fallback
+        testOneOf("bc", 0, "BasicConstraints");
+        testOneOf("BC", 0, "BasicConstraints");
+        testOneOf("BasicConstraints", 0, "BasicConstraints");
+        testOneOf("basicconstraints", 0, "BasicConstraints");
+        testOneOf("Basic", 0, "BasicConstraints");
+        testOneOf("basic", 0, "BasicConstraints");
+
+        testOneOf("BaCo", -1, "BasicConstraints");
+    }
+
+    // Expose some private methods
+    static void prepare() throws Exception {
+        createV3Extensions = Main.class.getDeclaredMethod(
+                "createV3Extensions",
+                CertificateExtensions.class,
+                CertificateExtensions.class,
+                List.class,
+                PublicKey.class,
+                PublicKey.class);
+        createV3Extensions.setAccessible(true);
+        ctor = Main.class.getDeclaredConstructor();
+        ctor.setAccessible(true);
+        KeyPairGenerator kpg = KeyPairGenerator.getInstance("EC");
+        pk = kpg.generateKeyPair().getPublic();
+
+        oneOf = Main.class.getDeclaredMethod(
+                "oneOf", String.class, String[].class);
+        oneOf.setAccessible(true);
+    }
+
+    /**
+     * Ensures the given type of extension is created with the option
+     */
+    static <T extends Extension> void testCreate(String option, Class<T> clazz)
+            throws Exception {
+        testCreate(option, clazz, null);
+    }
+
+    /**
+     * Ensures an option is invalid and will be rejected
+     */
+    static <T extends Extension> void testCreateFail(String option)
+            throws Exception {
+        testCreate(option, null, null);
+    }
+
+    /**
+     * Ensures the given type of extension is created and match the rule
+     * with the option.
+     *
+     * @param option the -ext option provided to keytool
+     * @param clazz the expected extension to create, null means none
+     * @param rule a predicate to check if the extension created is acceptable
+     * @param <T> the extected extension type
+     * @throws Exception if test result is unexpected
+     */
+    static <T extends Extension> void testCreate(String option, Class<T> clazz,
+            PredicateWithException<T> rule) throws Exception {
+        try {
+            CertificateExtensions exts = (CertificateExtensions)
+                    createV3Extensions.invoke(ctor.newInstance(),
+                            null, null, List.of(option), pk, null);
+
+            // ATTENTION: the extensions created above might contain raw
+            // extensions (not of a subtype) and we need to store and reload
+            // it to resolve them to subtypes.
+            ByteArrayOutputStream bout = new ByteArrayOutputStream();
+            exts.encode(bout);
+            exts = new CertificateExtensions(new DerValue(bout.toByteArray()).data);
+
+            if (clazz == null) {
+                throw new Exception("Should fail");
+            } else {
+                for (Extension e : exts.getAllExtensions()) {
+                    if (e.getClass() == clazz) {
+                        if (rule == null || rule.test((T) e)) {
+                            return;
+                        }
+                    }
+                }
+                throw new Exception("Unexpected result: " + exts);
+            }
+        } catch (InvocationTargetException e) {
+            if (clazz == null) {
+                return;
+            } else {
+                throw e;
+            }
+        }
+    }
+
+    @FunctionalInterface
+    interface PredicateWithException<T> {
+        boolean test(T t) throws Exception;
+    }
+
+    /**
+     * Ensures oneOf returns the expected result.
+     *
+     * @param s input
+     * @param expected expected value, -2 if ambiguous, -1 if no match
+     * @param items existing strings to match
+     * @throws Exception if test result is unexpected
+     */
+    static void testOneOf(String s, int expected, String... items)
+            throws Exception {
+        try {
+            int res = (int)oneOf.invoke(null, s, items);
+            if (expected == -2) {
+                throw new Exception("Should fail");
+            } else {
+                if (expected != res) {
+                    throw new Exception(
+                            "Expected " + expected + ", actually " + res);
+                }
+            }
+        } catch (InvocationTargetException e) {
+            if (expected == -2) {
+                return;
+            } else {
+                throw e;
+            }
+        }
+    }
+}
--- a/test/jdk/sun/security/tools/keytool/KeyToolTest.java	Sun Nov 03 18:02:29 2019 -0500
+++ b/test/jdk/sun/security/tools/keytool/KeyToolTest.java	Mon Nov 04 14:26:18 2019 +0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2019, 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
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @author weijun.wang
+ * @bug 6251120 8231950
  * @summary Testing keytool
  *
  * Run through autotest.sh and manualtest.sh
@@ -1319,11 +1319,13 @@
         // cRLSign cannot be cs
         testFail("", pre + "ku6 -ext KU=cs");
         testOK("", pre + "ku11 -ext KU=nr");
-        // ke also means keyAgreement
+        // ke means keyAgreement and keyCertSign...
         testFail("", pre + "ku12 -ext KU=ke");
         testOK("", pre + "ku12 -ext KU=keyE");
+        testOK("", pre + "ku12a -ext KU=kE"); // kE is only keyEncipherment
         // de also means decipherOnly
-        testFail("", pre + "ku13 -ext KU=de");
+        testOK("", pre + "ku13a -ext KU=de"); // de is decipherOnly
+        testOK("", pre + "ku13b -ext KU=dE"); // dE is dataEncipherment
         testOK("", pre + "ku13 -ext KU=dataE");
         testOK("", pre + "ku14 -ext KU=ka");
         testOK("", pre + "ku15 -ext KU=kcs");