You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by alopresto <gi...@git.apache.org> on 2018/05/07 16:42:38 UTC
[GitHub] nifi pull request #2683: NIFI-5146
GitHub user alopresto opened a pull request:
https://github.com/apache/nifi/pull/2683
NIFI-5146
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [x] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
- [x] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
- [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
- [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
### For documentation related changes:
- [x] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/alopresto/nifi NIFI-5146
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/2683.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2683
----
commit 52d47c089973f8d8b56c528f6cbfd3500188da69
Author: Andy LoPresto <al...@...>
Date: 2018-05-03T23:02:19Z
NIFI-5146 Added logic to check for simultaneous configuration of HTTP and HTTPS connectors in JettyServer.
commit c4630a8081f5399d672dae1b10840990ca17f787
Author: Andy LoPresto <al...@...>
Date: 2018-05-05T01:16:34Z
NIFI-5146 Added test logging resources.
Added unit tests.
commit e3e0d6540d1cd26057025d4fa18cacc61d5946bc
Author: Andy LoPresto <al...@...>
Date: 2018-05-05T03:26:01Z
NIFI-5146 Refactored shared functionality to generic method which accepts lambdas.
Fixed unit test with logging side effects.
commit 7bd5be9297a0adeb202f28a781d24d0a174b20fe
Author: Andy LoPresto <al...@...>
Date: 2018-05-07T16:40:58Z
NIFI-5146 Added note about exclusive HTTP/HTTPS behavior to Admin Guide.
Fixed typos.
----
---
[GitHub] nifi pull request #2683: NIFI-5146 Only support HTTP or HTTPS operation for ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/2683
---
[GitHub] nifi issue #2683: NIFI-5146 Only support HTTP or HTTPS operation for NiFi AP...
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on the issue:
https://github.com/apache/nifi/pull/2683
If NiFi is configured with both HTTP and HTTPS settings present, startup will fail and the error will look like the following:
```
2018-05-04 10:01:27,990 WARN [main] org.apache.nifi.web.server.JettyServer Both the HTTP and HTTPS connectors are configured in nifi.properties. Only one of these connectors should be configured. See the NiFi Admin Guide for more details
2018-05-04 10:01:27,990 WARN [main] org.apache.nifi.web.server.JettyServer HTTP connector: http://:8080
2018-05-04 10:01:27,991 WARN [main] org.apache.nifi.web.server.JettyServer HTTPS connector: https://:8443
2018-05-04 10:01:27,991 ERROR [main] org.apache.nifi.web.server.JettyServer NiFi only supports one mode of HTTP or HTTPS operation, not both simultaneously. Check the nifi.properties file and ensure that either the HTTP hostname and port or the HTTPS hostname and port are empty
2018-05-04 10:01:27,994 WARN [main] org.apache.nifi.web.server.JettyServer Failed to start web server... shutting down.
java.lang.IllegalStateException: Only one of the HTTP and HTTPS connectors can be configured at one time
at org.apache.nifi.web.server.JettyServer.configureConnectors(JettyServer.java:608)
at org.apache.nifi.web.server.JettyServer.<init>(JettyServer.java:153)
at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
at org.apache.nifi.NiFi.<init>(NiFi.java:150)
at org.apache.nifi.NiFi.<init>(NiFi.java:71)
at org.apache.nifi.NiFi.main(NiFi.java:292)
2018-05-04 10:01:27,995 INFO [Thread-1] org.apache.nifi.NiFi Initiating shutdown of Jetty web server...
2018-05-04 10:01:27,996 INFO [Thread-1] org.apache.nifi.NiFi Jetty web server shutdown completed (nicely or otherwise).
```
---
[GitHub] nifi issue #2683: NIFI-5146 Only support HTTP or HTTPS operation for NiFi AP...
Posted by kevdoran <gi...@git.apache.org>.
Github user kevdoran commented on the issue:
https://github.com/apache/nifi/pull/2683
Will review...
---
[GitHub] nifi pull request #2683: NIFI-5146 Only support HTTP or HTTPS operation for ...
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2683#discussion_r186512542
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java ---
@@ -601,106 +601,144 @@ private void configureConnectors(final Server server) throws ServerConfiguration
httpConfiguration.setRequestHeaderSize(headerSize);
httpConfiguration.setResponseHeaderSize(headerSize);
- if (props.getPort() != null) {
- final Integer port = props.getPort();
- if (port < 0 || (int) Math.pow(2, 16) <= port) {
- throw new ServerConfigurationException("Invalid HTTP port: " + port);
- }
+ // Check if both HTTP and HTTPS connectors are configured and fail if both are configured
+ if (bothHttpAndHttpsConnectorsConfigured(props)) {
+ logger.error("NiFi only supports one mode of HTTP or HTTPS operation, not both simultaneously. " +
+ "Check the nifi.properties file and ensure that either the HTTP hostname and port or the HTTPS hostname and port are empty");
+ startUpFailure(new IllegalStateException("Only one of the HTTP and HTTPS connectors can be configured at one time"));
+ }
- logger.info("Configuring Jetty for HTTP on port: " + port);
+ if (props.getSslPort() != null) {
+ configureHttpsConnector(server, httpConfiguration);
+ } else if (props.getPort() != null) {
+ configureHttpConnector(server, httpConfiguration);
+ } else {
+ logger.error("Neither the HTTP nor HTTPS connector was configured in nifi.properties");
+ startUpFailure(new IllegalStateException("Must configure HTTP or HTTPS connector"));
+ }
+ }
- final List<Connector> serverConnectors = Lists.newArrayList();
+ /**
+ * Configures an HTTPS connector and adds it to the server.
+ *
+ * @param server the Jetty server instance
+ * @param httpConfiguration the configuration object for the HTTPS protocol settings
+ */
+ private void configureHttpsConnector(Server server, HttpConfiguration httpConfiguration) {
+ String hostname = props.getProperty(NiFiProperties.WEB_HTTP_HOST);
--- End diff --
Good catch. Copied and pasted too many times.
---
[GitHub] nifi issue #2683: NIFI-5146 Only support HTTP or HTTPS operation for NiFi AP...
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on the issue:
https://github.com/apache/nifi/pull/2683
@kevdoran Fixed the copy/paste error and added an explicit unit test to ensure that mistake is not made again. Thanks.
---
[GitHub] nifi pull request #2683: NIFI-5146 Only support HTTP or HTTPS operation for ...
Posted by kevdoran <gi...@git.apache.org>.
Github user kevdoran commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2683#discussion_r186499568
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java ---
@@ -601,106 +601,144 @@ private void configureConnectors(final Server server) throws ServerConfiguration
httpConfiguration.setRequestHeaderSize(headerSize);
httpConfiguration.setResponseHeaderSize(headerSize);
- if (props.getPort() != null) {
- final Integer port = props.getPort();
- if (port < 0 || (int) Math.pow(2, 16) <= port) {
- throw new ServerConfigurationException("Invalid HTTP port: " + port);
- }
+ // Check if both HTTP and HTTPS connectors are configured and fail if both are configured
+ if (bothHttpAndHttpsConnectorsConfigured(props)) {
+ logger.error("NiFi only supports one mode of HTTP or HTTPS operation, not both simultaneously. " +
+ "Check the nifi.properties file and ensure that either the HTTP hostname and port or the HTTPS hostname and port are empty");
+ startUpFailure(new IllegalStateException("Only one of the HTTP and HTTPS connectors can be configured at one time"));
+ }
- logger.info("Configuring Jetty for HTTP on port: " + port);
+ if (props.getSslPort() != null) {
+ configureHttpsConnector(server, httpConfiguration);
+ } else if (props.getPort() != null) {
+ configureHttpConnector(server, httpConfiguration);
+ } else {
+ logger.error("Neither the HTTP nor HTTPS connector was configured in nifi.properties");
+ startUpFailure(new IllegalStateException("Must configure HTTP or HTTPS connector"));
+ }
+ }
- final List<Connector> serverConnectors = Lists.newArrayList();
+ /**
+ * Configures an HTTPS connector and adds it to the server.
+ *
+ * @param server the Jetty server instance
+ * @param httpConfiguration the configuration object for the HTTPS protocol settings
+ */
+ private void configureHttpsConnector(Server server, HttpConfiguration httpConfiguration) {
+ String hostname = props.getProperty(NiFiProperties.WEB_HTTP_HOST);
--- End diff --
Unless I'm missing something, this should be `NiFiProperties.WEB_HTTPS_HOST`
---