Skip to content

Commit

Permalink
Code clean up in CertRecordProcessor
Browse files Browse the repository at this point in the history 
* Make logger final
* Remove unnecessary Boolean literals
* Invert negated if statements for readability
* Use built in logger formatting
* Rename onlySomeReasons to not hide the field
* Replace synchronized HashTable and Vector instances (Vector/HashTable
are technically still used as they are
the implementations of List/Map passed in by at least some of the
calling code)
  • Loading branch information
@ckelleyRH
ckelleyRH committed Jun 12, 2023
1 parent 5fa7ede commit 9614f95
Showing 1 changed file with 25 additions and 25 deletions.
50 changes: 25 additions & 25 deletions base/ca/src/main/java/com/netscape/ca/CertRecordProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

import java.math.BigInteger;
import java.util.Date;
import java.util.Hashtable;
import java.util.Vector;
import java.util.List;
import java.util.Map;

import org.mozilla.jss.netscape.security.util.BitArray;
import org.mozilla.jss.netscape.security.x509.CRLExtensions;
Expand All @@ -39,9 +39,9 @@

public class CertRecordProcessor extends ElementProcessor {

public static org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(CertRecordProcessor.class);
public static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(CertRecordProcessor.class);

private Hashtable<BigInteger, RevokedCertificate> crlCerts;
private Map<BigInteger, RevokedCertificate> crlCerts;
private boolean allowExtensions;
private CRLIssuingPoint issuingPoint;

Expand All @@ -50,7 +50,7 @@ public class CertRecordProcessor extends ElementProcessor {
private BitArray onlySomeReasons;

public CertRecordProcessor(
Hashtable<BigInteger, RevokedCertificate> crlCerts,
Map<BigInteger, RevokedCertificate> crlCerts,
CRLIssuingPoint ip,
boolean allowExtensions) {

Expand All @@ -65,7 +65,7 @@ private boolean initCRLIssuingDistPointExtension() {
CMSCRLExtensions exts = null;

if (issuingDistPointAttempted ) {
return issuingDistPointEnabled == true && onlySomeReasons != null;
return issuingDistPointEnabled && onlySomeReasons != null;
}

issuingDistPointAttempted = true;
Expand All @@ -78,7 +78,7 @@ private boolean initCRLIssuingDistPointExtension() {
boolean isIssuingDistPointExtEnabled = false;
isIssuingDistPointExtEnabled = exts.isCRLExtensionEnabled(IssuingDistributionPointExtension.NAME);

if (isIssuingDistPointExtEnabled == false) {
if (!isIssuingDistPointExtEnabled) {
issuingDistPointEnabled = false;
return false;
}
Expand All @@ -87,10 +87,9 @@ private boolean initCRLIssuingDistPointExtension() {

// Get info out of the IssuingDistPointExtension
CRLExtensions ext = new CRLExtensions();
Vector<String> extNames = exts.getCRLExtensionNames();
List<String> extNames = exts.getCRLExtensionNames();

for (int i = 0; i < extNames.size(); i++) {
String extName = extNames.elementAt(i);
for (String extName : extNames) {
if (extName.equals(IssuingDistributionPointExtension.NAME)) {
exts.addToCRLExtensions(ext, extName, null);
}
Expand All @@ -101,6 +100,7 @@ private boolean initCRLIssuingDistPointExtension() {
try {
issuingDistExt = ext.get(IssuingDistributionPointExtension.NAME);
} catch (Exception e) {
// Swallowing exception?
}

IssuingDistributionPointExtension iExt = null;
Expand All @@ -115,19 +115,19 @@ private boolean initCRLIssuingDistPointExtension() {
issuingDistributionPoint = iExt.getIssuingDistributionPoint();
}

BitArray onlySomeReasons = null;
BitArray osr = null;

if (issuingDistributionPoint != null) {
onlySomeReasons = issuingDistributionPoint.getOnlySomeReasons();
osr = issuingDistributionPoint.getOnlySomeReasons();
}

boolean applyReasonMatch = false;

if (onlySomeReasons != null) {
applyReasonMatch = !onlySomeReasons.toString().equals("0000000");
logger.debug("applyReasonMatch " + applyReasonMatch);
if (applyReasonMatch == true) {
this.onlySomeReasons = onlySomeReasons;
if (osr != null) {
applyReasonMatch = !osr.toString().equals("0000000");
logger.debug("applyReasonMatch {}", applyReasonMatch);
if (applyReasonMatch) {
this.onlySomeReasons = osr;
result = true;
}
}
Expand Down Expand Up @@ -161,7 +161,7 @@ private boolean checkOnlySomeReasonsExtension(CRLExtensions entryExts) {
CRLReasonExtension theReason = (CRLReasonExtension) crlReasonExt;
reason = (RevocationReason) theReason.get("value");
reasonIndex = reason.getCode();
logger.debug("revoked reason " + reason);
logger.debug("revoked reason {}", reason);
} catch (Exception e) {
return includeCert;
}
Expand All @@ -172,10 +172,10 @@ private boolean checkOnlySomeReasonsExtension(CRLExtensions entryExts) {
boolean reasonMatch = false;
if (onlySomeReasons != null) {
reasonMatch = onlySomeReasons.get(reasonIndex);
if (reasonMatch != true) {
includeCert = false;
if (reasonMatch) {
logger.debug("onlySomeReasons match! reason: {}", reason);
} else {
logger.debug("onlySomeReasons match! reason: " + reason);
includeCert = false;
}
}

Expand All @@ -187,15 +187,15 @@ public boolean checkRevokedCertExtensions(CRLExtensions crlExtensions) {
// For now just check the onlySomeReason CRL IssuingDistributionPoint extension
boolean includeCert = true;

if ((crlExtensions == null) || (allowExtensions == false)) {
if ((crlExtensions == null) || (!allowExtensions)) {
return includeCert;
}

boolean inited = initCRLIssuingDistPointExtension();

// If the CRLIssuingDistPointExtension is not available or
// if onlySomeReasons does not apply, bail.
if (inited == false) {
if (!inited) {
return includeCert;
}

Expand Down Expand Up @@ -227,8 +227,8 @@ public void process(Object o) {

boolean includeCert = checkRevokedCertExtensions(crlExts);

if (includeCert == true) {
logger.info("CertRecordProcessor: Adding cert " + certID.toHexString() + " into CRL");
if (includeCert) {
logger.info("CertRecordProcessor: Adding cert {} into CRL", certID.toHexString()); //NOSONAR
crlCerts.put(serialNumber, newRevokedCert);
}
}
Expand Down

0 comments on commit 9614f95

Please sign in to comment.