>From 69386ae8fcf17e383ad98586dd4511840d5dac71 Mon Sep 17 00:00:00 2001
From: Abhishek Koneru <akoneru@redhat.com>
Date: Fri, 15 Jun 2012 10:28:40 -0400
Subject: [PATCH] Fixes for Coverity Issues of type Null Returns - Part 3

---
 .../com/netscape/certsrv/usrgrp/IUGSubsystem.java  |    6 ++-
 .../netscape/cmscore/profile/ProfileSubsystem.java |    2 +-
 .../com/netscape/cmscore/security/KeyCertUtil.java |    7 +++-
 .../com/netscape/cmscore/usrgrp/UGSubsystem.java   |   41 ++++++++++++++-----
 .../com/netscape/cmscore/request/RequestTest.java  |   42 ++++++++++++++++----
 base/kra/src/com/netscape/kra/RecoveryService.java |    6 +++
 6 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/base/common/src/com/netscape/certsrv/usrgrp/IUGSubsystem.java b/base/common/src/com/netscape/certsrv/usrgrp/IUGSubsystem.java
index c101187c497f8aa8e7d8b7aae67ec91ca70f9030..bbd051324d15605a7edb179a76b1c1b7e08935b3 100644
--- a/base/common/src/com/netscape/certsrv/usrgrp/IUGSubsystem.java
+++ b/base/common/src/com/netscape/certsrv/usrgrp/IUGSubsystem.java
@@ -120,16 +120,18 @@ public interface IUGSubsystem extends ISubsystem, IUsrGrp {
      *
      * @param filter the search filter
      * @return a list of groups that match the given search filter
+     * @throws EUsrGrpException
      */
-    public Enumeration<IGroup> findGroups(String filter);
+    public Enumeration<IGroup> findGroups(String filter) throws EUsrGrpException;
 
     /**
      * Find a group for the given name
      *
      * @param name the given name
      * @return a group that matched the given name
+     * @throws EUsrGrpException
      */
-    public IGroup findGroup(String name);
+    public IGroup findGroup(String name) throws EUsrGrpException;
 
     /**
      * List groups. This method is more efficient than findGroups because
diff --git a/base/common/src/com/netscape/cmscore/profile/ProfileSubsystem.java b/base/common/src/com/netscape/cmscore/profile/ProfileSubsystem.java
index 7da1cc33221c2deb9899c91b3e128e698e0b683c..45807d6e93d7dccf491f0d6b435255ed9f531341 100644
--- a/base/common/src/com/netscape/cmscore/profile/ProfileSubsystem.java
+++ b/base/common/src/com/netscape/cmscore/profile/ProfileSubsystem.java
@@ -97,7 +97,7 @@ public class ProfileSubsystem implements IProfileSubsystem {
             String classid = subStore.getString(PROP_CLASS_ID);
             IPluginInfo info = registry.getPluginInfo("profile", classid);
             if (info == null) {
-                throw new EBaseException("No plugins for type : profile with id " + classid);
+                throw new EBaseException("No plugins for type : profile, with id " + classid);
             }
             String configPath = subStore.getString(PROP_CONFIG);
 
diff --git a/base/common/src/com/netscape/cmscore/security/KeyCertUtil.java b/base/common/src/com/netscape/cmscore/security/KeyCertUtil.java
index 844052e4bbea1e2edf5231552c6a566c1c717db7..80ad36c2d4e90c2b1aa57b92f48b49d50fe868a3 100644
--- a/base/common/src/com/netscape/cmscore/security/KeyCertUtil.java
+++ b/base/common/src/com/netscape/cmscore/security/KeyCertUtil.java
@@ -1056,8 +1056,11 @@ public class KeyCertUtil {
         String dn = "ou=certificateRepository,ou=ca," + baseDN;
         BigInteger serialno = null;
         LDAPEntry entry = conn.read(dn);
-        String serialnoStr = (String) entry.getAttribute(
-                "serialno").getStringValues().nextElement();
+        LDAPAttribute serialNo = entry.getAttribute("serialno");
+        if (serialNo == null) {
+            throw new LDAPException("No value for attribute serial number in LDAP entry " + entry.getDN());
+        }
+        String serialnoStr = (String) serialNo.getStringValues().nextElement();
 
         serialno = BigIntegerMapper.BigIntegerFromDB(serialnoStr);
         LDAPAttribute attr = new LDAPAttribute("serialno");
diff --git a/base/common/src/com/netscape/cmscore/usrgrp/UGSubsystem.java b/base/common/src/com/netscape/cmscore/usrgrp/UGSubsystem.java
index 95e213541df0d046860fcdfcb0849cf348b21ca1..c7e83867dc9323a0b76935447925e106ce0f5121 100644
--- a/base/common/src/com/netscape/cmscore/usrgrp/UGSubsystem.java
+++ b/base/common/src/com/netscape/cmscore/usrgrp/UGSubsystem.java
@@ -416,8 +416,11 @@ public final class UGSubsystem implements IUGSubsystem {
      * @return the User entity.
      */
     protected IUser lbuildUser(LDAPEntry entry) throws EUsrGrpException {
-        IUser id = createUser(this, (String)
-                entry.getAttribute("uid").getStringValues().nextElement());
+        LDAPAttribute uid = entry.getAttribute("uid");
+        if (uid == null) {
+            throw new EUsrGrpException("No User with UID " + uid);
+        }
+        IUser id = createUser(this, (String) uid.getStringValues().nextElement());
         LDAPAttribute cnAttr = entry.getAttribute("cn");
 
         if (cnAttr != null) {
@@ -472,8 +475,11 @@ public final class UGSubsystem implements IUGSubsystem {
      * @return the User entity.
      */
     protected IUser buildUser(LDAPEntry entry) throws EUsrGrpException {
-        IUser id = createUser(this, (String)
-                entry.getAttribute("uid").getStringValues().nextElement());
+        LDAPAttribute uid = entry.getAttribute("uid");
+        if (uid == null) {
+            throw new EUsrGrpException("No User with UID " + uid + "for LDAP Entry " + entry.getDN());
+        }
+        IUser id = createUser(this, (String) uid.getStringValues().nextElement());
         LDAPAttribute cnAttr = entry.getAttribute("cn");
 
         if (cnAttr != null) {
@@ -1119,7 +1125,7 @@ public final class UGSubsystem implements IUGSubsystem {
         }
     }
 
-    protected Enumeration<IGroup> buildGroups(LDAPSearchResults res) {
+    protected Enumeration<IGroup> buildGroups(LDAPSearchResults res) throws EUsrGrpException {
         Vector<IGroup> v = new Vector<IGroup>();
 
         while (res.hasMoreElements()) {
@@ -1132,8 +1138,9 @@ public final class UGSubsystem implements IUGSubsystem {
 
     /**
      * Finds groups.
+     * @throws EUsrGrpException
      */
-    public Enumeration<IGroup> findGroups(String filter) {
+    public Enumeration<IGroup> findGroups(String filter) throws EUsrGrpException {
         if (filter == null) {
             return null;
         }
@@ -1160,7 +1167,7 @@ public final class UGSubsystem implements IUGSubsystem {
         }
     }
 
-    public IGroup findGroup(String filter) {
+    public IGroup findGroup(String filter) throws EUsrGrpException {
         Enumeration<IGroup> groups = findGroups(filter);
 
         if (groups == null || !groups.hasMoreElements())
@@ -1205,9 +1212,14 @@ public final class UGSubsystem implements IUGSubsystem {
 
     /**
      * builds an instance of a Group entry
+     * @throws EUsrGrpException
      */
-    protected IGroup buildGroup(LDAPEntry entry) {
-        String groupName = (String) entry.getAttribute("cn").getStringValues().nextElement();
+    protected IGroup buildGroup(LDAPEntry entry) throws EUsrGrpException {
+        LDAPAttribute cn=entry.getAttribute("cn");
+        if (cn == null) {
+            throw new EUsrGrpException("Cannot build group. No Attribute cn in LDAP Entry " + entry.getDN());
+        }
+        String groupName = (String) cn.getStringValues().nextElement();
         IGroup grp = createGroup(this, groupName);
 
         LDAPAttribute grpDesc = entry.getAttribute("description");
@@ -1357,7 +1369,9 @@ public final class UGSubsystem implements IUGSubsystem {
     public boolean isMemberOf(String userid, String groupname) {
         try {
             IUser user = getUser(userid);
-            return isMemberOfLdapGroup(user.getUserDN(), groupname);
+            if (user != null) {
+                return isMemberOfLdapGroup(user.getUserDN(), groupname);
+            }
         } catch (Exception e) {
             /* do nothing */
         }
@@ -1630,7 +1644,12 @@ public final class UGSubsystem implements IUGSubsystem {
     protected boolean isMatched(String dn1, String dn2) {
         String rdn1[] = LDAPDN.explodeDN(dn1, false);
         String rdn2[] = LDAPDN.explodeDN(dn2, false);
-
+        if ((rdn1 == null) ^ (rdn2 == null)) {
+            return false;
+        }
+        if ((rdn1 == null) && (rdn2 == null)) {
+            return true;
+        }
         if (rdn1.length == rdn2.length) {
             for (int j = 0; j < rdn1.length; j++) {
                 if (!rdn1[j].equalsIgnoreCase(rdn2[j])) {
diff --git a/base/common/test/com/netscape/cmscore/request/RequestTest.java b/base/common/test/com/netscape/cmscore/request/RequestTest.java
index 0e98ab084dbb6258a8756f606995074e5ce4198c..1cea946e1a3dfc8782389825266636981eb47c3a 100644
--- a/base/common/test/com/netscape/cmscore/request/RequestTest.java
+++ b/base/common/test/com/netscape/cmscore/request/RequestTest.java
@@ -1,6 +1,5 @@
 package com.netscape.cmscore.request;
 
-import java.io.IOException;
 import java.math.BigInteger;
 import java.security.cert.CRLException;
 import java.security.cert.CertificateEncodingException;
@@ -237,7 +236,7 @@ public class RequestTest extends CMSBaseTestCase {
         assertFalse(request.setExtData("key", (Integer) null));
     }
 
-    public void testGetSetExtDataIntegerArray() {
+    public void testGetSetExtDataIntegerArray() throws Exception {
         Integer[] data = new Integer[] {
                 Integer.valueOf(5),
                 Integer.valueOf(23),
@@ -245,6 +244,7 @@ public class RequestTest extends CMSBaseTestCase {
         };
         assertTrue(request.setExtData("topkey1", data));
         Integer[] retval = request.getExtDataInIntegerArray("topkey1");
+        checkReturnValue(retval, "topkey1");
         assertEquals(3, retval.length);
         assertEquals(data[0], retval[0]);
         assertEquals(data[1], retval[1]);
@@ -276,7 +276,7 @@ public class RequestTest extends CMSBaseTestCase {
         assertFalse(request.setExtData("key", (BigInteger) null));
     }
 
-    public void testGetSetExtDataBigIntegerArray() {
+    public void testGetSetExtDataBigIntegerArray() throws Exception {
         BigInteger[] data = new BigInteger[] {
                 new BigInteger("111111111"),
                 new BigInteger("222222222"),
@@ -284,6 +284,7 @@ public class RequestTest extends CMSBaseTestCase {
         };
         assertTrue(request.setExtData("topkey1", data));
         BigInteger[] retval = request.getExtDataInBigIntegerArray("topkey1");
+        checkReturnValue(retval, "topkey1");
         assertEquals(3, retval.length);
         assertEquals(data[0], retval[0]);
         assertEquals(data[1], retval[1]);
@@ -340,7 +341,7 @@ public class RequestTest extends CMSBaseTestCase {
         assertFalse(request.setExtData("key", (X509CertImpl) null));
     }
 
-    public void testGetSetCertArray() throws CertificateException {
+    public void testGetSetCertArray() throws Exception {
         // this test is also pretty weak, but fortunately relies on the
         // building blocks.
         X509CertImpl[] vals = new X509CertImpl[] {
@@ -354,6 +355,7 @@ public class RequestTest extends CMSBaseTestCase {
 
         assertFalse(cmsStub.aToBCalled);
         X509CertImpl[] retval = request.getExtDataInCertArray("key");
+        checkReturnValue(retval, "key");
         assertTrue(cmsStub.aToBCalled);
 
         assertEquals(2, retval.length);
@@ -363,7 +365,7 @@ public class RequestTest extends CMSBaseTestCase {
         assertFalse(request.setExtData("key", (X509CertImpl[]) null));
     }
 
-    public void testGetSetStringArray() {
+    public void testGetSetStringArray() throws Exception {
         String[] value = new String[] { "blue", "green", "red", "orange" };
         assertTrue(request.setExtData("key", value));
 
@@ -380,6 +382,7 @@ public class RequestTest extends CMSBaseTestCase {
         assertEquals("orange", hashValue.get("3"));
 
         String[] retval = request.getExtDataInStringArray("key");
+        checkReturnValue(retval, "key");
         assertEquals(4, retval.length);
         assertEquals("blue", retval[0]);
         assertEquals("green", retval[1]);
@@ -394,6 +397,7 @@ public class RequestTest extends CMSBaseTestCase {
         request.setExtData("kevin", hashValue);
 
         retval = request.getExtDataInStringArray("kevin");
+        checkReturnValue(retval, "kevin");
         assertEquals(7, retval.length);
         assertEquals("square", retval[0]);
         assertNull(retval[1]);
@@ -536,7 +540,7 @@ public class RequestTest extends CMSBaseTestCase {
         assertFalse(request.setExtData("key", (RevokedCertImpl[]) null));
     }
 
-    public void testGetSetCertExts() throws IOException {
+    public void testGetSetCertExts() throws Exception {
         CertificateExtensions exts = new CertificateExtensions();
         BasicConstraintsExtension ext = new BasicConstraintsExtension(false, 1);
 
@@ -551,19 +555,21 @@ public class RequestTest extends CMSBaseTestCase {
         assertTrue(request.mExtData.containsKey("key2"));
 
         retval = request.getExtDataInCertExts("key2");
+        checkReturnValue(retval, "key2");
         assertNotNull(retval);
         assertEquals(1, retval.size());
 
         assertFalse(request.setExtData("key", (CertificateExtensions) null));
     }
 
-    public void testGetSetCertSubjectName() throws IOException {
+    public void testGetSetCertSubjectName() throws Exception {
         CertificateSubjectName name = new CertificateSubjectName(
                 new X500Name("cn=kevin"));
         assertTrue(request.setExtData("key", name));
         assertTrue(request.mExtData.containsKey("key"));
 
         CertificateSubjectName retval = request.getExtDataInCertSubjectName("key");
+        checkReturnValue(retval, "key");
         assertNotNull(retval);
         // the 'CN=' is uppercased at some point
         assertEquals("cn=kevin",
@@ -590,6 +596,28 @@ public class RequestTest extends CMSBaseTestCase {
         assertFalse(request.setExtData("key", (AuthToken) null));
     }
 
+    public void checkReturnValue(Object retVal, String key) throws Exception {
+        if (retVal == null) {
+            String type = "";
+            if (retVal instanceof String[]) {
+                type = "a String array";
+            } else if (retVal instanceof Integer[]) {
+                type = "an Integer array";
+            } else if (retVal instanceof BigInteger[]) {
+                type = "a BigInteger array";
+            } else if (retVal instanceof CertificateSubjectName) {
+                type = "a CertificateSubjectName";
+            } else if (retVal instanceof CertificateExtensions) {
+                type = "a CertificateExtensions";
+            } else if (retVal instanceof X509CertImpl[]) {
+                type = "a X509CertImpl array";
+            } else if (retVal instanceof IAuthToken) {
+                type = "an IAuthToken";
+            }
+            throw new Exception("Unable to get " + key + "as " + type + " object.");
+        }
+    }
+
     /**
      * CMSMemoryStub
      *
diff --git a/base/kra/src/com/netscape/kra/RecoveryService.java b/base/kra/src/com/netscape/kra/RecoveryService.java
index 19d6b515798c714a62a37a0342724d855080ffb4..be1cedd8166fe534ae94275d1b4228c2c54f9b9e 100644
--- a/base/kra/src/com/netscape/kra/RecoveryService.java
+++ b/base/kra/src/com/netscape/kra/RecoveryService.java
@@ -450,6 +450,9 @@ public class RecoveryService implements IService {
             // create p12
             X509Certificate x509cert =
                     request.getExtDataInCert(ATTR_USER_CERT);
+            if (x509cert == null) {
+                throw new EKRAException(CMS.getUserMessage("CMS_KRA_INVALID_KEYRECORD"));
+            }
             String pwd = (String) params.get(ATTR_TRANSPORT_PWD);
 
             // add certificate
@@ -587,6 +590,9 @@ public class RecoveryService implements IService {
             // create p12
             X509Certificate x509cert =
                     request.getExtDataInCert(ATTR_USER_CERT);
+            if (x509cert == null) {
+                throw new EKRAException(CMS.getUserMessage("CMS_KRA_INVALID_KEYRECORD"));
+            }
             String pwd = (String) params.get(ATTR_TRANSPORT_PWD);
 
             // add certificate
-- 
1.7.10.2

