You are viewing a plain text version of this content. The canonical link for it is here.
Posted to kerby@directory.apache.org by "Zheng, Kai" <ka...@intel.com> on 2015/12/23 04:30:37 UTC

FW: directory-kerby git commit: DIRKRB-510 Fail to convert options in KadminTool.

Thanks Jiajia for the fixing. The issues were introduced by my recent commits. I relied totally on the unit tests, but unfortunately we don't cover the tools yet. We should write unit tests for them as well.

Noted you have the fix and changed the behavior of the first functions in KadminOption. I had refined the two functions in all kinds of option class but forgot to change the reference. The behavior of the first one is expected, I guess we would just use the 2nd one for the fix? Thanks.

    public static KadminOption fromName(String name) {
        if (name != null) {
            for (KadminOption ko : values()) {
                if (ko.name().equals(name)) {
                    return ko;
                }
            }
        }
        return NONE;
    }

    public static KadminOption fromOptionName(String optionName) {
        if (optionName != null) {
            for (KadminOption ko : values()) {
                if (ko.optionInfo != null
                    && ko.optionInfo.getName().equals(optionName)) {
                    return ko;
                }
            }
        }
        return NONE;
    }

-----Original Message-----
From: plusplusjiajia@apache.org [mailto:plusplusjiajia@apache.org] 
Sent: Wednesday, December 23, 2015 11:20 AM
To: commits@directory.apache.org
Subject: directory-kerby git commit: DIRKRB-510 Fail to convert options in KadminTool.

Repository: directory-kerby
Updated Branches:
  refs/heads/master 5b6cf76f8 -> 2bb3a5876


DIRKRB-510 Fail to convert options in KadminTool.


Project: http://git-wip-us.apache.org/repos/asf/directory-kerby/repo
Commit: http://git-wip-us.apache.org/repos/asf/directory-kerby/commit/2bb3a587
Tree: http://git-wip-us.apache.org/repos/asf/directory-kerby/tree/2bb3a587
Diff: http://git-wip-us.apache.org/repos/asf/directory-kerby/diff/2bb3a587

Branch: refs/heads/master
Commit: 2bb3a5876ddb961cb4614ad7928767d16c301b55
Parents: 5b6cf76
Author: plusplusjiajia <ji...@intel.com>
Authored: Wed Dec 23 11:27:05 2015 +0800
Committer: plusplusjiajia <ji...@intel.com>
Committed: Wed Dec 23 11:27:05 2015 +0800

----------------------------------------------------------------------
 .../org/apache/kerby/kerberos/kerb/admin/KadminOption.java     | 3 ++-
 .../kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java     | 3 ---
 .../java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java | 2 +-
 .../java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java   | 6 ++++--
 .../java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java   | 6 ++++--
 5 files changed, 11 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/directory-kerby/blob/2bb3a587/kerby-kerb/kerb-admin/src/main/java/org/apache/kerby/kerberos/kerb/admin/KadminOption.java
----------------------------------------------------------------------
diff --git a/kerby-kerb/kerb-admin/src/main/java/org/apache/kerby/kerberos/kerb/admin/KadminOption.java b/kerby-kerb/kerb-admin/src/main/java/org/apache/kerby/kerberos/kerb/admin/KadminOption.java
index e0a6634..bdab4d6 100644
--- a/kerby-kerb/kerb-admin/src/main/java/org/apache/kerby/kerberos/kerb/admin/KadminOption.java
+++ b/kerby-kerb/kerb-admin/src/main/java/org/apache/kerby/kerberos/kerb/admin/KadminOption.java
@@ -52,7 +52,8 @@ public enum KadminOption implements KOption {
     public static KadminOption fromName(String name) {
         if (name != null) {
             for (KadminOption ko : values()) {
-                if (ko.name().equals(name)) {
+                if (ko.optionInfo != null
+                        && ko.optionInfo.getName().equals(name)) {
                     return ko;
                 }
             }

http://git-wip-us.apache.org/repos/asf/directory-kerby/blob/2bb3a587/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java
----------------------------------------------------------------------
diff --git a/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java b/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java
index 0ad5219..640f718 100644
--- a/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java
+++ b/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java
@@ -20,10 +20,8 @@
 package org.apache.kerby.kerberos.kerb.client.preauth.pkinit;
 
 import org.apache.kerby.KOptions;
-import org.apache.kerby.asn1.Asn1;
 import org.apache.kerby.asn1.type.Asn1Integer;
 import org.apache.kerby.asn1.type.Asn1ObjectIdentifier;
-import org.apache.kerby.cms.type.SignedContentInfo;
 import org.apache.kerby.kerberos.kerb.KrbCodec;
 import org.apache.kerby.kerberos.kerb.KrbException;
 import org.apache.kerby.kerberos.kerb.client.KrbContext;
@@ -59,7 +57,6 @@ import org.slf4j.LoggerFactory;
 
 import javax.crypto.interfaces.DHPublicKey;
 import javax.crypto.spec.DHParameterSpec;
-import java.io.IOException;
 import java.math.BigInteger;
 import java.util.Arrays;
 import java.util.Calendar;

http://git-wip-us.apache.org/repos/asf/directory-kerby/blob/2bb3a587/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java
----------------------------------------------------------------------
diff --git a/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java b/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java
index 44ee10b..a4b581d 100644
--- a/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java
+++ b/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java
@@ -59,7 +59,7 @@ public enum KinitOption implements KOption {
     CANONICALIZE(new KOptionInfo("-C", "canonicalize",
         KrbOptionGroup.KDC_FLAGS)),
 
-    USE_PASSWD(new KOptionInfo("using password", "using password",
+    USE_PASSWD(new KOptionInfo("using-password", "using password",
         KrbOptionGroup.KRB)),
     USER_PASSWD(new KOptionInfo("user-passwd", "User plain password",
         KrbOptionGroup.KRB)),

http://git-wip-us.apache.org/repos/asf/directory-kerby/blob/2bb3a587/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java
----------------------------------------------------------------------
diff --git a/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java b/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java
index be16c99..d59867a 100644
--- a/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java
+++ b/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java
@@ -215,7 +215,7 @@ public class KinitTool {
                 kto = KinitOption.NONE;
             }
 
-            if (kto.getOptionInfo().getType() != KOptionType.NOV) {
+            if (kto != KinitOption.NONE && kto.getOptionInfo().getType() != KOptionType.NOV) {
                 // require a parameter
                 param = null;
                 if (i < args.length) {
@@ -231,7 +231,9 @@ public class KinitTool {
             if (error != null) {
                 printUsage(error);
             }
-            ktOptions.add(kto);
+            if (kto != KinitOption.NONE) {
+                ktOptions.add(kto);
+            }
         }
 
         if (principal == null) {

http://git-wip-us.apache.org/repos/asf/directory-kerby/blob/2bb3a587/kerby-tool/kdc-tool/src/main/java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java
----------------------------------------------------------------------
diff --git a/kerby-tool/kdc-tool/src/main/java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java b/kerby-tool/kdc-tool/src/main/java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java
index 76aa5c6..e2f33ff 100644
--- a/kerby-tool/kdc-tool/src/main/java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java
+++ b/kerby-tool/kdc-tool/src/main/java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java
@@ -56,7 +56,7 @@ public class ToolUtil {
                 error = "Invalid parameter:" + opt + " , it does not belong to any option.";
             }
 
-            if (kOption.getOptionInfo().getType() != KOptionType.NOV) {
+            if (kOption != KadminOption.NONE && kOption.getOptionInfo().getType() != KOptionType.NOV) {
                 // require a parameter
                 param = null;
                 if (i <= endIndex) {
@@ -72,7 +72,9 @@ public class ToolUtil {
                 System.out.println(error);
                 return null;
             }
-            kOptions.add(kOption);
+            if (kOption != KadminOption.NONE) {
+                kOptions.add(kOption);
+            }
         }
         return kOptions;
     }


RE: directory-kerby git commit: DIRKRB-510 Fail to convert options in KadminTool.

Posted by "Li, Jiajia" <ji...@intel.com>.
I have tested all the tools, and have worked now. You are right, I use the 2nd one for the fix.

-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com] 
Sent: Wednesday, December 23, 2015 11:31 AM
To: kerby@directory.apache.org
Subject: FW: directory-kerby git commit: DIRKRB-510 Fail to convert options in KadminTool.

Thanks Jiajia for the fixing. The issues were introduced by my recent commits. I relied totally on the unit tests, but unfortunately we don't cover the tools yet. We should write unit tests for them as well.

Noted you have the fix and changed the behavior of the first functions in KadminOption. I had refined the two functions in all kinds of option class but forgot to change the reference. The behavior of the first one is expected, I guess we would just use the 2nd one for the fix? Thanks.

    public static KadminOption fromName(String name) {
        if (name != null) {
            for (KadminOption ko : values()) {
                if (ko.name().equals(name)) {
                    return ko;
                }
            }
        }
        return NONE;
    }

    public static KadminOption fromOptionName(String optionName) {
        if (optionName != null) {
            for (KadminOption ko : values()) {
                if (ko.optionInfo != null
                    && ko.optionInfo.getName().equals(optionName)) {
                    return ko;
                }
            }
        }
        return NONE;
    }

-----Original Message-----
From: plusplusjiajia@apache.org [mailto:plusplusjiajia@apache.org] 
Sent: Wednesday, December 23, 2015 11:20 AM
To: commits@directory.apache.org
Subject: directory-kerby git commit: DIRKRB-510 Fail to convert options in KadminTool.

Repository: directory-kerby
Updated Branches:
  refs/heads/master 5b6cf76f8 -> 2bb3a5876


DIRKRB-510 Fail to convert options in KadminTool.


Project: http://git-wip-us.apache.org/repos/asf/directory-kerby/repo
Commit: http://git-wip-us.apache.org/repos/asf/directory-kerby/commit/2bb3a587
Tree: http://git-wip-us.apache.org/repos/asf/directory-kerby/tree/2bb3a587
Diff: http://git-wip-us.apache.org/repos/asf/directory-kerby/diff/2bb3a587

Branch: refs/heads/master
Commit: 2bb3a5876ddb961cb4614ad7928767d16c301b55
Parents: 5b6cf76
Author: plusplusjiajia <ji...@intel.com>
Authored: Wed Dec 23 11:27:05 2015 +0800
Committer: plusplusjiajia <ji...@intel.com>
Committed: Wed Dec 23 11:27:05 2015 +0800

----------------------------------------------------------------------
 .../org/apache/kerby/kerberos/kerb/admin/KadminOption.java     | 3 ++-
 .../kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java     | 3 ---
 .../java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java | 2 +-
 .../java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java   | 6 ++++--
 .../java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java   | 6 ++++--
 5 files changed, 11 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/directory-kerby/blob/2bb3a587/kerby-kerb/kerb-admin/src/main/java/org/apache/kerby/kerberos/kerb/admin/KadminOption.java
----------------------------------------------------------------------
diff --git a/kerby-kerb/kerb-admin/src/main/java/org/apache/kerby/kerberos/kerb/admin/KadminOption.java b/kerby-kerb/kerb-admin/src/main/java/org/apache/kerby/kerberos/kerb/admin/KadminOption.java
index e0a6634..bdab4d6 100644
--- a/kerby-kerb/kerb-admin/src/main/java/org/apache/kerby/kerberos/kerb/admin/KadminOption.java
+++ b/kerby-kerb/kerb-admin/src/main/java/org/apache/kerby/kerberos/kerb/admin/KadminOption.java
@@ -52,7 +52,8 @@ public enum KadminOption implements KOption {
     public static KadminOption fromName(String name) {
         if (name != null) {
             for (KadminOption ko : values()) {
-                if (ko.name().equals(name)) {
+                if (ko.optionInfo != null
+                        && ko.optionInfo.getName().equals(name)) {
                     return ko;
                 }
             }

http://git-wip-us.apache.org/repos/asf/directory-kerby/blob/2bb3a587/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java
----------------------------------------------------------------------
diff --git a/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java b/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java
index 0ad5219..640f718 100644
--- a/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java
+++ b/kerby-kerb/kerb-client/src/main/java/org/apache/kerby/kerberos/kerb/client/preauth/pkinit/PkinitPreauth.java
@@ -20,10 +20,8 @@
 package org.apache.kerby.kerberos.kerb.client.preauth.pkinit;
 
 import org.apache.kerby.KOptions;
-import org.apache.kerby.asn1.Asn1;
 import org.apache.kerby.asn1.type.Asn1Integer;
 import org.apache.kerby.asn1.type.Asn1ObjectIdentifier;
-import org.apache.kerby.cms.type.SignedContentInfo;
 import org.apache.kerby.kerberos.kerb.KrbCodec;
 import org.apache.kerby.kerberos.kerb.KrbException;
 import org.apache.kerby.kerberos.kerb.client.KrbContext;
@@ -59,7 +57,6 @@ import org.slf4j.LoggerFactory;
 
 import javax.crypto.interfaces.DHPublicKey;
 import javax.crypto.spec.DHParameterSpec;
-import java.io.IOException;
 import java.math.BigInteger;
 import java.util.Arrays;
 import java.util.Calendar;

http://git-wip-us.apache.org/repos/asf/directory-kerby/blob/2bb3a587/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java
----------------------------------------------------------------------
diff --git a/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java b/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java
index 44ee10b..a4b581d 100644
--- a/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java
+++ b/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitOption.java
@@ -59,7 +59,7 @@ public enum KinitOption implements KOption {
     CANONICALIZE(new KOptionInfo("-C", "canonicalize",
         KrbOptionGroup.KDC_FLAGS)),
 
-    USE_PASSWD(new KOptionInfo("using password", "using password",
+    USE_PASSWD(new KOptionInfo("using-password", "using password",
         KrbOptionGroup.KRB)),
     USER_PASSWD(new KOptionInfo("user-passwd", "User plain password",
         KrbOptionGroup.KRB)),

http://git-wip-us.apache.org/repos/asf/directory-kerby/blob/2bb3a587/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java
----------------------------------------------------------------------
diff --git a/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java b/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java
index be16c99..d59867a 100644
--- a/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java
+++ b/kerby-tool/client-tool/src/main/java/org/apache/kerby/kerberos/tool/kinit/KinitTool.java
@@ -215,7 +215,7 @@ public class KinitTool {
                 kto = KinitOption.NONE;
             }
 
-            if (kto.getOptionInfo().getType() != KOptionType.NOV) {
+            if (kto != KinitOption.NONE && kto.getOptionInfo().getType() != KOptionType.NOV) {
                 // require a parameter
                 param = null;
                 if (i < args.length) {
@@ -231,7 +231,9 @@ public class KinitTool {
             if (error != null) {
                 printUsage(error);
             }
-            ktOptions.add(kto);
+            if (kto != KinitOption.NONE) {
+                ktOptions.add(kto);
+            }
         }
 
         if (principal == null) {

http://git-wip-us.apache.org/repos/asf/directory-kerby/blob/2bb3a587/kerby-tool/kdc-tool/src/main/java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java
----------------------------------------------------------------------
diff --git a/kerby-tool/kdc-tool/src/main/java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java b/kerby-tool/kdc-tool/src/main/java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java
index 76aa5c6..e2f33ff 100644
--- a/kerby-tool/kdc-tool/src/main/java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java
+++ b/kerby-tool/kdc-tool/src/main/java/org/apache/kerby/kerberos/tool/kadmin/ToolUtil.java
@@ -56,7 +56,7 @@ public class ToolUtil {
                 error = "Invalid parameter:" + opt + " , it does not belong to any option.";
             }
 
-            if (kOption.getOptionInfo().getType() != KOptionType.NOV) {
+            if (kOption != KadminOption.NONE && kOption.getOptionInfo().getType() != KOptionType.NOV) {
                 // require a parameter
                 param = null;
                 if (i <= endIndex) {
@@ -72,7 +72,9 @@ public class ToolUtil {
                 System.out.println(error);
                 return null;
             }
-            kOptions.add(kOption);
+            if (kOption != KadminOption.NONE) {
+                kOptions.add(kOption);
+            }
         }
         return kOptions;
     }