exceptionfactory commented on code in PR #7098:
URL: https://github.com/apache/nifi/pull/7098#discussion_r1153559962
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/RestChangeIngestor.java:
##########
@@ -172,39 +182,53 @@ private void createConnector(Properties properties) {
http.setIdleTimeout(30000L);
jetty.addConnector(http);
- logger.info("Added an http connector on the host '{}' and port '{}'",
new Object[]{http.getHost(), http.getPort()});
+ logger.info("Added an http connector on the host '{}' and port '{}'",
http.getHost(), http.getPort());
}
private void createSecureConnector(Properties properties) {
- SslContextFactory ssl = new SslContextFactory();
-
- if (properties.getProperty(KEYSTORE_LOCATION_KEY) != null) {
- ssl.setKeyStorePath(properties.getProperty(KEYSTORE_LOCATION_KEY));
-
ssl.setKeyStorePassword(properties.getProperty(KEYSTORE_PASSWORD_KEY));
- ssl.setKeyStoreType(properties.getProperty(KEYSTORE_TYPE_KEY));
+ KeyStore keyStore;
+ KeyStore truststore = null;
+
+ try (FileInputStream keyStoreStream = new
FileInputStream(properties.getProperty(KEYSTORE_LOCATION_KEY))) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(KEYSTORE_TYPE_KEY))
+ .inputStream(keyStoreStream)
+
.password(properties.getProperty(KEYSTORE_PASSWORD_KEY).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
}
if (properties.getProperty(TRUSTSTORE_LOCATION_KEY) != null) {
-
ssl.setTrustStorePath(properties.getProperty(TRUSTSTORE_LOCATION_KEY));
-
ssl.setTrustStorePassword(properties.getProperty(TRUSTSTORE_PASSWORD_KEY));
- ssl.setTrustStoreType(properties.getProperty(TRUSTSTORE_TYPE_KEY));
-
ssl.setNeedClientAuth(Boolean.parseBoolean(properties.getProperty(NEED_CLIENT_AUTH_KEY,
"true")));
+ try (FileInputStream trustStoreStream = new
FileInputStream(properties.getProperty(TRUSTSTORE_LOCATION_KEY))) {
+ truststore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(TRUSTSTORE_TYPE_KEY))
+ .inputStream(trustStoreStream)
+
.password(properties.getProperty(TRUSTSTORE_PASSWORD_KEY).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
```suggestion
} catch (final IOException ioe) {
throw new UncheckedIOException("Trust Store loading failed",
ioe);
```
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/RestChangeIngestor.java:
##########
@@ -172,39 +182,53 @@ private void createConnector(Properties properties) {
http.setIdleTimeout(30000L);
jetty.addConnector(http);
- logger.info("Added an http connector on the host '{}' and port '{}'",
new Object[]{http.getHost(), http.getPort()});
+ logger.info("Added an http connector on the host '{}' and port '{}'",
http.getHost(), http.getPort());
}
private void createSecureConnector(Properties properties) {
- SslContextFactory ssl = new SslContextFactory();
-
- if (properties.getProperty(KEYSTORE_LOCATION_KEY) != null) {
- ssl.setKeyStorePath(properties.getProperty(KEYSTORE_LOCATION_KEY));
-
ssl.setKeyStorePassword(properties.getProperty(KEYSTORE_PASSWORD_KEY));
- ssl.setKeyStoreType(properties.getProperty(KEYSTORE_TYPE_KEY));
+ KeyStore keyStore;
+ KeyStore truststore = null;
+
+ try (FileInputStream keyStoreStream = new
FileInputStream(properties.getProperty(KEYSTORE_LOCATION_KEY))) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(KEYSTORE_TYPE_KEY))
+ .inputStream(keyStoreStream)
+
.password(properties.getProperty(KEYSTORE_PASSWORD_KEY).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
Recommend adding a descriptive message to the exception.
```suggestion
} catch (final IOException ioe) {
throw new UncheckedIOException("Key Store loading failed", ioe);
```
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -100,7 +113,42 @@ public static void main(String[] args) throws Exception {
server.join();
}
- private static WebAppContext loadWar(final File warFile, final String
contextPath, final ClassLoader parentClassLoader) throws IOException {
+ private static SSLContext buildSSLContext(C2Properties properties) {
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ File keyStoreFile =
Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
Review Comment:
Placeholders should be used instead of string concatenation:
```suggestion
logger.debug("Loading Key Store [{}]", keyStoreFile.getPath());
```
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -100,7 +113,42 @@ public static void main(String[] args) throws Exception {
server.join();
}
- private static WebAppContext loadWar(final File warFile, final String
contextPath, final ClassLoader parentClassLoader) throws IOException {
+ private static SSLContext buildSSLContext(C2Properties properties) {
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ File keyStoreFile =
Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new
FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+
.password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
+
+ File trustStoreFile =
Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE)).toFile();
+ logger.debug("trustStore path: " + trustStoreFile.getPath());
Review Comment:
```suggestion
logger.debug("Loading Trust Store [{}]", trustStoreFile.getPath());
```
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -100,7 +113,42 @@ public static void main(String[] args) throws Exception {
server.join();
}
- private static WebAppContext loadWar(final File warFile, final String
contextPath, final ClassLoader parentClassLoader) throws IOException {
+ private static SSLContext buildSSLContext(C2Properties properties) {
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ File keyStoreFile =
Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new
FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+
.password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
+
+ File trustStoreFile =
Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE)).toFile();
+ logger.debug("trustStore path: " + trustStoreFile.getPath());
+ try (FileInputStream trustStoreStream = new
FileInputStream(trustStoreFile)) {
+ truststore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE_TYPE))
+ .inputStream(trustStoreStream)
+
.password(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
```suggestion
throw new UncheckedIOException("Trust Store loading failed",
ioe);
```
##########
minifi/minifi-c2/minifi-c2-provider/minifi-c2-provider-util/src/main/java/org/apache/nifi/minifi/c2/provider/util/HttpConnector.java:
##########
@@ -89,6 +102,43 @@ public HttpConnector(String baseUrl, String proxyHost, int
proxyPort, String pro
}
}
+ private SSLContext buildSSLContext() {
+ C2Properties properties = C2Properties.getInstance();
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ Path c2ServerHome = Paths.get(C2_SERVER_HOME);
+ File keyStoreFile =
c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new
FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+
.password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
+
+ File trustStoreFile =
c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE)).toFile();
+ logger.debug("trustStore path: " + trustStoreFile.getPath());
Review Comment:
```suggestion
logger.debug("Loading Trust Store [{}]", trustStoreFile.getPath());
```
##########
minifi/minifi-c2/minifi-c2-provider/minifi-c2-provider-util/src/main/java/org/apache/nifi/minifi/c2/provider/util/HttpConnector.java:
##########
@@ -89,6 +102,43 @@ public HttpConnector(String baseUrl, String proxyHost, int
proxyPort, String pro
}
}
+ private SSLContext buildSSLContext() {
+ C2Properties properties = C2Properties.getInstance();
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ Path c2ServerHome = Paths.get(C2_SERVER_HOME);
+ File keyStoreFile =
c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new
FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+
.password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
+
+ File trustStoreFile =
c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE)).toFile();
+ logger.debug("trustStore path: " + trustStoreFile.getPath());
+ try (FileInputStream trustStoreStream = new
FileInputStream(trustStoreFile)) {
+ truststore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE_TYPE))
+ .inputStream(trustStoreStream)
+
.password(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
```suggestion
throw new UncheckedIOException("Trust Store loading failed",
ioe);
```
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -100,7 +113,42 @@ public static void main(String[] args) throws Exception {
server.join();
}
- private static WebAppContext loadWar(final File warFile, final String
contextPath, final ClassLoader parentClassLoader) throws IOException {
+ private static SSLContext buildSSLContext(C2Properties properties) {
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ File keyStoreFile =
Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new
FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+
.password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
```suggestion
throw new UncheckedIOException("Key Store loading failed", ioe);
```
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/RestChangeIngestor.java:
##########
@@ -172,39 +182,53 @@ private void createConnector(Properties properties) {
http.setIdleTimeout(30000L);
jetty.addConnector(http);
- logger.info("Added an http connector on the host '{}' and port '{}'",
new Object[]{http.getHost(), http.getPort()});
+ logger.info("Added an http connector on the host '{}' and port '{}'",
http.getHost(), http.getPort());
}
private void createSecureConnector(Properties properties) {
- SslContextFactory ssl = new SslContextFactory();
-
- if (properties.getProperty(KEYSTORE_LOCATION_KEY) != null) {
- ssl.setKeyStorePath(properties.getProperty(KEYSTORE_LOCATION_KEY));
-
ssl.setKeyStorePassword(properties.getProperty(KEYSTORE_PASSWORD_KEY));
- ssl.setKeyStoreType(properties.getProperty(KEYSTORE_TYPE_KEY));
+ KeyStore keyStore;
+ KeyStore truststore = null;
+
+ try (FileInputStream keyStoreStream = new
FileInputStream(properties.getProperty(KEYSTORE_LOCATION_KEY))) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(KEYSTORE_TYPE_KEY))
+ .inputStream(keyStoreStream)
+
.password(properties.getProperty(KEYSTORE_PASSWORD_KEY).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
}
if (properties.getProperty(TRUSTSTORE_LOCATION_KEY) != null) {
-
ssl.setTrustStorePath(properties.getProperty(TRUSTSTORE_LOCATION_KEY));
-
ssl.setTrustStorePassword(properties.getProperty(TRUSTSTORE_PASSWORD_KEY));
- ssl.setTrustStoreType(properties.getProperty(TRUSTSTORE_TYPE_KEY));
-
ssl.setNeedClientAuth(Boolean.parseBoolean(properties.getProperty(NEED_CLIENT_AUTH_KEY,
"true")));
+ try (FileInputStream trustStoreStream = new
FileInputStream(properties.getProperty(TRUSTSTORE_LOCATION_KEY))) {
+ truststore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(TRUSTSTORE_TYPE_KEY))
+ .inputStream(trustStoreStream)
+
.password(properties.getProperty(TRUSTSTORE_PASSWORD_KEY).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
}
- // build the connector
- final ServerConnector https = new ServerConnector(jetty, ssl);
+ SSLContext sslContext = new StandardSslContextBuilder()
+ .keyStore(keyStore)
+
.keyPassword(properties.getProperty(KEYSTORE_PASSWORD_KEY).toCharArray())
+ .trustStore(truststore)
+ .build();
- // set host and port
- https.setPort(Integer.parseInt(properties.getProperty(PORT_KEY, "0")));
- https.setHost(properties.getProperty(HOST_KEY, "localhost"));
+ StandardServerConnectorFactory serverConnectorFactory = new
StandardServerConnectorFactory(jetty,
Integer.parseInt(properties.getProperty(PORT_KEY, "0")));
+
serverConnectorFactory.setNeedClientAuth(Boolean.parseBoolean(properties.getProperty(NEED_CLIENT_AUTH_KEY,
"true")));
+ serverConnectorFactory.setSslContext(sslContext);
+
serverConnectorFactory.setIncludeSecurityProtocols(TlsPlatform.getPreferredProtocols().toArray(new
String[0]));
- // Severely taxed environments may have significant delays when
executing.
- https.setIdleTimeout(30000L);
+ ServerConnector https = serverConnectorFactory.getServerConnector();
+ https.setHost(properties.getProperty(HOST_KEY, "localhost"));
// add the connector
jetty.addConnector(https);
- logger.info("Added an https connector on the host '{}' and port '{}'",
new Object[]{https.getHost(), https.getPort()});
+ logger.info("Added an https connector on the host '{}' and port '{}'",
https.getHost(), https.getPort());
Review Comment:
This log could probably be removed, since Jetty logs an informational
message, but if kept, recommend the following adjusted wording.
```suggestion
logger.info("HTTPS Connector added for Host [{}] and Port [{}]",
https.getHost(), https.getPort());
```
##########
minifi/minifi-c2/minifi-c2-provider/minifi-c2-provider-util/src/main/java/org/apache/nifi/minifi/c2/provider/util/HttpConnector.java:
##########
@@ -89,6 +102,43 @@ public HttpConnector(String baseUrl, String proxyHost, int
proxyPort, String pro
}
}
+ private SSLContext buildSSLContext() {
+ C2Properties properties = C2Properties.getInstance();
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ Path c2ServerHome = Paths.get(C2_SERVER_HOME);
+ File keyStoreFile =
c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
Review Comment:
```suggestion
logger.debug("Loading Key Store [{}]", keyStoreFile.getPath());
```
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -134,7 +182,11 @@ private static WebAppContext loadWar(final File warFile,
final String contextPat
// configure the max form size (3x the default)
webappContext.setMaxFormContentSize(600000);
- webappContext.setClassLoader(new WebAppClassLoader(parentClassLoader,
webappContext));
+ try {
+ webappContext.setClassLoader(new
WebAppClassLoader(parentClassLoader, webappContext));
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
Review Comment:
Is this try-catch necessary?
##########
minifi/minifi-c2/minifi-c2-provider/minifi-c2-provider-util/src/main/java/org/apache/nifi/minifi/c2/provider/util/HttpConnector.java:
##########
@@ -89,6 +102,43 @@ public HttpConnector(String baseUrl, String proxyHost, int
proxyPort, String pro
}
}
+ private SSLContext buildSSLContext() {
+ C2Properties properties = C2Properties.getInstance();
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ Path c2ServerHome = Paths.get(C2_SERVER_HOME);
+ File keyStoreFile =
c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new
FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+
.password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
```suggestion
throw new UncheckedIOException("Key Store loading failed", ioe);
```
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/RestChangeIngestor.java:
##########
@@ -172,39 +182,53 @@ private void createConnector(Properties properties) {
http.setIdleTimeout(30000L);
jetty.addConnector(http);
- logger.info("Added an http connector on the host '{}' and port '{}'",
new Object[]{http.getHost(), http.getPort()});
+ logger.info("Added an http connector on the host '{}' and port '{}'",
http.getHost(), http.getPort());
}
private void createSecureConnector(Properties properties) {
- SslContextFactory ssl = new SslContextFactory();
-
- if (properties.getProperty(KEYSTORE_LOCATION_KEY) != null) {
- ssl.setKeyStorePath(properties.getProperty(KEYSTORE_LOCATION_KEY));
-
ssl.setKeyStorePassword(properties.getProperty(KEYSTORE_PASSWORD_KEY));
- ssl.setKeyStoreType(properties.getProperty(KEYSTORE_TYPE_KEY));
+ KeyStore keyStore;
+ KeyStore truststore = null;
+
+ try (FileInputStream keyStoreStream = new
FileInputStream(properties.getProperty(KEYSTORE_LOCATION_KEY))) {
Review Comment:
It could be checked for existence, but if it does not exist, `new
FileInputStream()` will throw a `FileNotFoundException`, which is already
caught, so this approach seems sufficient provided an error message is added to
the `UncheckedIOException`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
