[Pki-devel] [PATCH] 619 Cleaned up SystemConfigService.validateRequest().
Endi Sukma Dewata
edewata at redhat.com
Mon Jun 29 17:37:47 UTC 2015
The configure() in SystemConfigService method has been modified to
log only the error message in normal responses but log the full
stack trace when unexpected issues occur.
The validateData() in SystemConfigService has been renamed to
validateRequest() for clarity. The log messages have been modified
to include the invalid values entered in the request.
--
Endi S. Dewata
-------------- next part --------------
From fc57d2e1acdc96115adcbe7db3820e7e8475f7b3 Mon Sep 17 00:00:00 2001
From: "Endi S. Dewata" <edewata at redhat.com>
Date: Mon, 29 Jun 2015 10:00:08 -0400
Subject: [PATCH] Cleaned up SystemConfigService.validateRequest().
The configure() in SystemConfigService method has been modified to
log only the error message in normal responses but log the full
stack trace when unexpected issues occur.
The validateData() in SystemConfigService has been renamed to
validateRequest() for clarity. The log messages have been modified
to include the invalid values entered in the request.
---
.../cms/servlet/test/ConfigurationTest.java | 2 +-
.../certsrv/system/SystemConfigClient.java | 2 +-
.../certsrv/system/SystemConfigResource.java | 2 +-
.../dogtagpki/server/rest/SystemConfigService.java | 69 ++++++++++++----------
4 files changed, 41 insertions(+), 34 deletions(-)
diff --git a/base/common/functional/src/com/netscape/cms/servlet/test/ConfigurationTest.java b/base/common/functional/src/com/netscape/cms/servlet/test/ConfigurationTest.java
index bf4dc892822df7d32be5d83c64e99c4dc7620529..69994fa389d31f15e314feba6bfbd3d4803622ba 100644
--- a/base/common/functional/src/com/netscape/cms/servlet/test/ConfigurationTest.java
+++ b/base/common/functional/src/com/netscape/cms/servlet/test/ConfigurationTest.java
@@ -76,7 +76,7 @@ public class ConfigurationTest {
System.exit(1);
}
- public static void main(String args[]) throws NoSuchAlgorithmException, TokenException, IOException, InvalidBERException {
+ public static void main(String args[]) throws Exception {
String host = null;
String port = null;
String cstype = null;
diff --git a/base/common/src/com/netscape/certsrv/system/SystemConfigClient.java b/base/common/src/com/netscape/certsrv/system/SystemConfigClient.java
index 242f0053119d04fcbf635f245bbde1fd58a19ff5..8208915808ebf5992e37b8e6d2ba54bedfadf232 100644
--- a/base/common/src/com/netscape/certsrv/system/SystemConfigClient.java
+++ b/base/common/src/com/netscape/certsrv/system/SystemConfigClient.java
@@ -40,7 +40,7 @@ public class SystemConfigClient extends Client {
configClient = createProxy(SystemConfigResource.class);
}
- public ConfigurationResponse configure(ConfigurationRequest data) {
+ public ConfigurationResponse configure(ConfigurationRequest data) throws Exception {
return configClient.configure(data);
}
}
diff --git a/base/common/src/com/netscape/certsrv/system/SystemConfigResource.java b/base/common/src/com/netscape/certsrv/system/SystemConfigResource.java
index 0cebb607433aea8571ff524df42872e9ae781c43..9c570eb2b6749c2eac1fc36ccf21b98f458369dd 100644
--- a/base/common/src/com/netscape/certsrv/system/SystemConfigResource.java
+++ b/base/common/src/com/netscape/certsrv/system/SystemConfigResource.java
@@ -29,5 +29,5 @@ public interface SystemConfigResource {
@POST
@Path("configure")
- public ConfigurationResponse configure(ConfigurationRequest data);
+ public ConfigurationResponse configure(ConfigurationRequest data) throws Exception;
}
diff --git a/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java b/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java
index 2de087badfa3f1b87ccf2295f00fc6c490c53517..75e3065faedb59f6e38a1778be7df40e2056ea0f 100644
--- a/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java
+++ b/base/server/cms/src/org/dogtagpki/server/rest/SystemConfigService.java
@@ -111,28 +111,38 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
* @see com.netscape.cms.servlet.csadmin.SystemConfigurationResource#configure(com.netscape.cms.servlet.csadmin.data.ConfigurationData)
*/
@Override
- public ConfigurationResponse configure(ConfigurationRequest request) {
+ public ConfigurationResponse configure(ConfigurationRequest request) throws Exception {
+
+ CMS.debug("SystemConfigService: configure()");
+
try {
ConfigurationResponse response = new ConfigurationResponse();
configure(request, response);
return response;
- } catch (Throwable t) {
- CMS.debug(t);
- throw t;
+ } catch (PKIException e) { // normal responses
+ CMS.debug(e.getMessage()); // log the response
+ throw e;
+
+ } catch (Exception e) { // unexpected exceptions
+ CMS.debug(e); // show stack trace for troubleshooting
+ throw e;
+
+ } catch (Error e) { // system errors
+ CMS.debug(e); // show stack trace for troubleshooting
+ throw e;
}
}
- public void configure(ConfigurationRequest data, ConfigurationResponse response) {
+ public void configure(ConfigurationRequest data, ConfigurationResponse response) throws Exception {
+
if (csState.equals("1")) {
throw new BadRequestException("System is already configured");
}
- CMS.debug("SystemConfigService(): configure() called");
- CMS.debug(data.toString());
-
- validateData(data);
+ CMS.debug("SystemConfigService: request: " + data);
+ validateRequest(data);
Collection<String> certList = getCertList(data);
@@ -1020,22 +1030,15 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
}
}
- private void validateData(ConfigurationRequest data) {
- // get required info from CS.cfg
- String preopPin;
- try {
- preopPin = cs.getString("preop.pin");
- } catch (Exception e) {
- CMS.debug("validateData: Failed to get required config form CS.cfg");
- e.printStackTrace();
- throw new PKIException("Unable to retrieve required configuration from configuration files");
- }
+ private void validateRequest(ConfigurationRequest data) throws Exception {
- // get the preop pin and validate it
+ // validate installation pin
String pin = data.getPin();
if (pin == null) {
throw new BadRequestException("No preop pin provided");
}
+
+ String preopPin = cs.getString("preop.pin");
if (!preopPin.equals(pin)) {
throw new BadRequestException("Incorrect pin provided");
}
@@ -1067,6 +1070,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
if (data.getSecurityDomainName() == null) {
throw new BadRequestException("Security Domain Name is not provided");
}
+
} else if (domainType.equals(ConfigurationRequest.EXISTING_DOMAIN) ||
domainType.equals(ConfigurationRequest.NEW_SUBDOMAIN)) {
if (data.getStandAlone()) {
@@ -1079,11 +1083,11 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
}
try {
- @SuppressWarnings("unused")
- URL admin_u = new URL(domainURI); // check for invalid URL
+ new URL(domainURI);
} catch (MalformedURLException e) {
- throw new BadRequestException("Invalid security domain URI");
+ throw new BadRequestException("Invalid security domain URI: " + domainURI, e);
}
+
if ((data.getSecurityDomainUser() == null) || (data.getSecurityDomainPassword() == null)) {
throw new BadRequestException("Security domain user or password not provided");
}
@@ -1109,11 +1113,13 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
throw new BadRequestException("Clone selected, but no clone URI provided");
}
try {
- @SuppressWarnings("unused")
- URL url = new URL(cloneUri); // check for invalid URL
+ URL url = new URL(cloneUri);
// confirm protocol is https
+ if (!"https".equals(url.getProtocol())) {
+ throw new BadRequestException("Clone URI must use HTTPS protocol: " + cloneUri);
+ }
} catch (MalformedURLException e) {
- throw new BadRequestException("Invalid clone URI");
+ throw new BadRequestException("Invalid clone URI: " + cloneUri, e);
}
if (data.getToken().equals(ConfigurationRequest.TOKEN_DEFAULT)) {
@@ -1133,6 +1139,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
throw new BadRequestException("P12 password should not be provided since HSM clones must share their HSM master's private keys");
}
}
+
} else {
data.setClone("false");
}
@@ -1145,7 +1152,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
try {
Integer.parseInt(data.getDsPort()); // check for errors
} catch (NumberFormatException e) {
- throw new BadRequestException("Internal database port is invalid");
+ throw new BadRequestException("Internal database port is invalid: " + data.getDsPort(), e);
}
String basedn = data.getBaseDN();
@@ -1173,7 +1180,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
try {
Integer.parseInt(masterReplicationPort); // check for errors
} catch (NumberFormatException e) {
- throw new BadRequestException("Master replication port is invalid");
+ throw new BadRequestException("Master replication port is invalid: " + masterReplicationPort, e);
}
}
@@ -1181,8 +1188,8 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
if (cloneReplicationPort != null && cloneReplicationPort.length() > 0) {
try {
Integer.parseInt(cloneReplicationPort); // check for errors
- } catch (Exception e) {
- throw new BadRequestException("Clone replication port is invalid");
+ } catch (NumberFormatException e) {
+ throw new BadRequestException("Clone replication port is invalid: " + cloneReplicationPort, e);
}
}
@@ -1293,7 +1300,7 @@ public class SystemConfigService extends PKIService implements SystemConfigResou
try {
Integer.parseInt(data.getAuthdbPort()); // check for errors
} catch (NumberFormatException e) {
- throw new BadRequestException("Authdb port is invalid");
+ throw new BadRequestException("Authentication Database port is invalid: " + data.getAuthdbPort(), e);
}
// TODO check connection with authdb
--
1.9.3
More information about the Pki-devel
mailing list