You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "Vinz2168 (via GitHub)" <gi...@apache.org> on 2026/10/08 14:48:39 UTC
[PR] NIFI-14719 Added Passive Port Range property to ListenFTP [nifi]
Vinz2168 opened a new pull request, #11784:
URL: https://github.com/apache/nifi/pull/11784
# Summary
[NIFI-14719](https://issues.apache.org/jira/browse/NIFI-14719)
Adds an optional `Passive Port Range` property to `ListenFTP` (format `<start>-<end>`, e.g. `50000-50099`) that restricts the ports the embedded FTP server uses for passive mode data connections. This makes it possible to open a narrow, predictable set of ports on firewalls and to publish them from containers such as Docker. When the property is not set, the current behavior (any available port) is unchanged.
Changes:
- `ListenFTP`: new `Passive Port Range` property supporting Environment Expression Language, with a validator that checks the `<start>-<end>` format, the 1-65535 bounds and that start <= end
- `NifiFtpServer`: the listener always builds a single `DataConnectionConfiguration`, applying the passive ports and, when an SSL Context Service is configured, the implicit TLS settings for data connections, so the two options combine correctly
- `additionalDetails.md`: documented the new property
- `TestListenFTP`: validation tests and a test that starts the server and checks that the `PASV` reply advertises a port within the configured range
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [x] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [x] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
- [x] Pull request contains [commits signed](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits) with a registered key indicating `Verified` status
### Pull Request Formatting
- [x] Pull Request based on current revision of the `main` branch
- [x] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `./mvnw clean install -P contrib-check`
- [ ] JDK 21
- [ ] JDK 25
`nifi-standard-processors` verified with `-P contrib-check` (checkstyle, PMD, RAT) and `TestListenFTP` passing.
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
No new dependencies.
### Documentation
- [x] Documentation formatting appears as expected in rendered files
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14719 Added Passive Port Range property to ListenFTP [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #11784:
URL: https://github.com/apache/nifi/pull/11784#discussion_r4353641393
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ftp/NifiFtpServer.java:
##########
@@ -176,10 +182,16 @@ private ConnectionConfig createConnectionConfig(boolean anonymousLoginEnabled) {
return connectionConfigFactory.createConnectionConfig();
}
- private Listener createListener(String bindAddress, int port, SSLContextProvider sslContextProvider) throws FtpServerConfigurationException {
+ private Listener createListener(String bindAddress, int port, SSLContextProvider sslContextProvider, String passivePortRange) throws FtpServerConfigurationException {
ListenerFactory listenerFactory = new ListenerFactory();
listenerFactory.setServerAddress(bindAddress);
listenerFactory.setPort(port);
+
+ DataConnectionConfigurationFactory dataConnectionConfigurationFactory = new DataConnectionConfigurationFactory();
+ if (passivePortRange != null) {
+ dataConnectionConfigurationFactory.setPassivePorts(passivePortRange);
+ }
Review Comment:
The `null` check can be removed now that the property is required
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestListenFTP.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.net.ftp.FTPClient;
+import org.apache.commons.net.ftp.FTPReply;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.io.IOException;
+import java.net.ServerSocket;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class TestListenFTP {
+
+ private static final String LOCALHOST = "127.0.0.1";
+
+ private static final Pattern PASSIVE_REPLY_PATTERN = Pattern.compile("\\((\\d+),(\\d+),(\\d+),(\\d+),(\\d+),(\\d+)\\)");
+
+ private TestRunner runner;
+
+ @BeforeEach
+ void setRunner() {
+ runner = TestRunners.newTestRunner(ListenFTP.class);
+ }
+
+ @AfterEach
+ void shutdownRunner() {
+ runner.shutdown();
+ }
+
+ @Test
+ void testPassivePortRangeDefaultValid() {
+ runner.assertValid();
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"50000-50099", "50000-50000", "1-65535", "${PASSIVE_PORT_RANGE}"})
+ void testPassivePortRangeValid(final String passivePortRange) {
+ runner.setProperty(ListenFTP.PASSIVE_PORT_RANGE, passivePortRange);
+
+ runner.assertValid();
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"50000", "50099-50000", "0-100", "50000-65536", "50000-50099,50200", "start-end", " "})
+ void testPassivePortRangeInvalid(final String passivePortRange) {
+ runner.setProperty(ListenFTP.PASSIVE_PORT_RANGE, passivePortRange);
+
+ runner.assertNotValid();
+ }
+
+ @Test
+ void testPassiveModeUsesConfiguredPortRange() throws IOException {
+ final int port = getAvailablePort();
+ final int passivePort = getAvailablePort();
+ runner.setProperty(ListenFTP.ADDRESS, LOCALHOST);
+ runner.setProperty(ListenFTP.PORT, Integer.toString(port));
+ runner.setProperty(ListenFTP.PASSIVE_PORT_RANGE, passivePort + "-" + passivePort);
+
+ runner.run(1, false);
+
+ final FTPClient client = new FTPClient();
+ try {
+ client.connect(LOCALHOST, port);
+ assertTrue(client.login("anonymous", ""));
+
+ assertEquals(FTPReply.ENTERING_PASSIVE_MODE, client.pasv());
+ assertEquals(passivePort, getPassivePort(client.getReplyString()));
+ } finally {
+ client.disconnect();
+ runner.stop();
+ }
+ }
+
+ private int getPassivePort(final String passiveReply) {
+ final Matcher matcher = PASSIVE_REPLY_PATTERN.matcher(passiveReply);
+ assertTrue(matcher.find(), "Unexpected passive mode reply: " + passiveReply);
+ return Integer.parseInt(matcher.group(5)) * 256 + Integer.parseInt(matcher.group(6));
+ }
+
+ private int getAvailablePort() throws IOException {
Review Comment:
This approach can be unstable based on timing. The value of `0` can be configured at the lower level, but may require adding a method to the Processor to retrieve the value determined.
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestListenFTP.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.processors.standard;
+
+import org.apache.commons.net.ftp.FTPClient;
+import org.apache.commons.net.ftp.FTPReply;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.io.IOException;
+import java.net.ServerSocket;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class TestListenFTP {
+
+ private static final String LOCALHOST = "127.0.0.1";
+
+ private static final Pattern PASSIVE_REPLY_PATTERN = Pattern.compile("\\((\\d+),(\\d+),(\\d+),(\\d+),(\\d+),(\\d+)\\)");
+
+ private TestRunner runner;
+
+ @BeforeEach
+ void setRunner() {
+ runner = TestRunners.newTestRunner(ListenFTP.class);
+ }
+
+ @AfterEach
+ void shutdownRunner() {
+ runner.shutdown();
+ }
+
+ @Test
+ void testPassivePortRangeDefaultValid() {
+ runner.assertValid();
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"50000-50099", "50000-50000", "1-65535", "${PASSIVE_PORT_RANGE}"})
+ void testPassivePortRangeValid(final String passivePortRange) {
+ runner.setProperty(ListenFTP.PASSIVE_PORT_RANGE, passivePortRange);
+
+ runner.assertValid();
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"50000", "50099-50000", "0-100", "50000-65536", "50000-50099,50200", "start-end", " "})
+ void testPassivePortRangeInvalid(final String passivePortRange) {
+ runner.setProperty(ListenFTP.PASSIVE_PORT_RANGE, passivePortRange);
+
+ runner.assertNotValid();
+ }
+
+ @Test
+ void testPassiveModeUsesConfiguredPortRange() throws IOException {
+ final int port = getAvailablePort();
+ final int passivePort = getAvailablePort();
+ runner.setProperty(ListenFTP.ADDRESS, LOCALHOST);
+ runner.setProperty(ListenFTP.PORT, Integer.toString(port));
+ runner.setProperty(ListenFTP.PASSIVE_PORT_RANGE, passivePort + "-" + passivePort);
+
+ runner.run(1, false);
+
+ final FTPClient client = new FTPClient();
+ try {
+ client.connect(LOCALHOST, port);
+ assertTrue(client.login("anonymous", ""));
+
+ assertEquals(FTPReply.ENTERING_PASSIVE_MODE, client.pasv());
+ assertEquals(passivePort, getPassivePort(client.getReplyString()));
+ } finally {
+ client.disconnect();
+ runner.stop();
+ }
+ }
+
+ private int getPassivePort(final String passiveReply) {
+ final Matcher matcher = PASSIVE_REPLY_PATTERN.matcher(passiveReply);
+ assertTrue(matcher.find(), "Unexpected passive mode reply: " + passiveReply);
+ return Integer.parseInt(matcher.group(5)) * 256 + Integer.parseInt(matcher.group(6));
Review Comment:
I would be helpful to declare the intermediate variables for the parsed integers
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14719 Added Passive Port Range property to ListenFTP [nifi]
Posted by "Vinz2168 (via GitHub)" <gi...@apache.org>.
Vinz2168 commented on PR #11784:
URL: https://github.com/apache/nifi/pull/11784#issuecomment-6106378655
Thanks for the review @exceptionfactory! I pushed a new commit addressing the recommendations:
- Made `Passive Port Range` required with a default value of `1025-65534`, and updated the documentation accordingly
- Changed the property description to use a multiline string
- Placed each method call on a separate line in the `ValidationResult.Builder` chains
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14719 Added Passive Port Range property to ListenFTP [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #11784:
URL: https://github.com/apache/nifi/pull/11784#discussion_r4239260052
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenFTP.java:
##########
@@ -119,9 +122,45 @@ public class ListenFTP extends AbstractSessionFactoryProcessor implements Listen
.sensitive(true)
.build();
+ private static final Pattern PORT_RANGE_PATTERN = Pattern.compile("^(\\d{1,5})-(\\d{1,5})$");
+
+ private static final Validator PORT_RANGE_VALIDATOR = (subject, input, context) -> {
+ if (context.isExpressionLanguageSupported(subject) && context.isExpressionLanguagePresent(input)) {
+ return new ValidationResult.Builder().subject(subject).input(input).explanation("Expression Language Present").valid(true).build();
+ }
+
+ final String explanation;
+ final Matcher matcher = PORT_RANGE_PATTERN.matcher(input);
+ if (matcher.matches()) {
+ final int startPort = Integer.parseInt(matcher.group(1));
+ final int endPort = Integer.parseInt(matcher.group(2));
+ if (startPort < 1 || endPort > 65535) {
+ explanation = "Ports must be between 1 and 65535";
+ } else if (startPort > endPort) {
+ explanation = "Start port must be less than or equal to end port";
+ } else {
+ explanation = null;
+ }
+ } else {
+ explanation = "Port range must be in the format <start>-<end>, e.g. 50000-50099";
+ }
+ return new ValidationResult.Builder().subject(subject).input(input).explanation(explanation).valid(explanation == null).build();
+ };
+
+ public static final PropertyDescriptor PASSIVE_PORT_RANGE = new PropertyDescriptor.Builder()
+ .name("Passive Port Range")
+ .description("The range of ports, in the format <start>-<end> (e.g. 50000-50099), that the FTP server uses for data connections in passive mode. "
+ + "Restricting the range simplifies firewall and container port mapping configurations. "
+ + "If not set, any available port is used for each passive data connection.")
+ .required(false)
Review Comment:
Instead of making this property optional, what do you think about having a default of `1025-65534`?
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenFTP.java:
##########
@@ -119,9 +122,45 @@ public class ListenFTP extends AbstractSessionFactoryProcessor implements Listen
.sensitive(true)
.build();
+ private static final Pattern PORT_RANGE_PATTERN = Pattern.compile("^(\\d{1,5})-(\\d{1,5})$");
+
+ private static final Validator PORT_RANGE_VALIDATOR = (subject, input, context) -> {
+ if (context.isExpressionLanguageSupported(subject) && context.isExpressionLanguagePresent(input)) {
+ return new ValidationResult.Builder().subject(subject).input(input).explanation("Expression Language Present").valid(true).build();
+ }
+
+ final String explanation;
+ final Matcher matcher = PORT_RANGE_PATTERN.matcher(input);
+ if (matcher.matches()) {
+ final int startPort = Integer.parseInt(matcher.group(1));
+ final int endPort = Integer.parseInt(matcher.group(2));
+ if (startPort < 1 || endPort > 65535) {
+ explanation = "Ports must be between 1 and 65535";
+ } else if (startPort > endPort) {
+ explanation = "Start port must be less than or equal to end port";
+ } else {
+ explanation = null;
+ }
+ } else {
+ explanation = "Port range must be in the format <start>-<end>, e.g. 50000-50099";
+ }
+ return new ValidationResult.Builder().subject(subject).input(input).explanation(explanation).valid(explanation == null).build();
Review Comment:
For readability, it would be helpful to have each method call on one line.
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenFTP.java:
##########
@@ -119,9 +122,45 @@ public class ListenFTP extends AbstractSessionFactoryProcessor implements Listen
.sensitive(true)
.build();
+ private static final Pattern PORT_RANGE_PATTERN = Pattern.compile("^(\\d{1,5})-(\\d{1,5})$");
+
+ private static final Validator PORT_RANGE_VALIDATOR = (subject, input, context) -> {
+ if (context.isExpressionLanguageSupported(subject) && context.isExpressionLanguagePresent(input)) {
+ return new ValidationResult.Builder().subject(subject).input(input).explanation("Expression Language Present").valid(true).build();
+ }
+
+ final String explanation;
+ final Matcher matcher = PORT_RANGE_PATTERN.matcher(input);
+ if (matcher.matches()) {
+ final int startPort = Integer.parseInt(matcher.group(1));
+ final int endPort = Integer.parseInt(matcher.group(2));
+ if (startPort < 1 || endPort > 65535) {
+ explanation = "Ports must be between 1 and 65535";
+ } else if (startPort > endPort) {
+ explanation = "Start port must be less than or equal to end port";
+ } else {
+ explanation = null;
+ }
+ } else {
+ explanation = "Port range must be in the format <start>-<end>, e.g. 50000-50099";
+ }
+ return new ValidationResult.Builder().subject(subject).input(input).explanation(explanation).valid(explanation == null).build();
+ };
+
+ public static final PropertyDescriptor PASSIVE_PORT_RANGE = new PropertyDescriptor.Builder()
+ .name("Passive Port Range")
+ .description("The range of ports, in the format <start>-<end> (e.g. 50000-50099), that the FTP server uses for data connections in passive mode. "
Review Comment:
A multiline string should be used instead of concatenation for descriptions.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14719 Added Passive Port Range property to ListenFTP [nifi]
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory merged PR #11784:
URL: https://github.com/apache/nifi/pull/11784
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14719 Added Passive Port Range property to ListenFTP [nifi]
Posted by "Vinz2168 (via GitHub)" <gi...@apache.org>.
Vinz2168 commented on PR #11784:
URL: https://github.com/apache/nifi/pull/11784#issuecomment-6411313490
Thanks for the detailed explanation @exceptionfactory, that makes sense. I went with the third option: a new `Passive Port Selection` property backed by a `PassivePortSelection` enum with `RANDOM` (default, preserving the current behavior) and `CONFIGURED`, with `Passive Port Range` depending on `CONFIGURED` and no longer having a default value. The passive ports are applied to the FTP server only when `CONFIGURED` is selected.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14719 Added Passive Port Range property to ListenFTP [nifi]
Posted by "Vinz2168 (via GitHub)" <gi...@apache.org>.
Vinz2168 commented on PR #11784:
URL: https://github.com/apache/nifi/pull/11784#issuecomment-6380917941
Thanks for the second round @exceptionfactory! I pushed a new commit:
- Removed the `null` check in `NifiFtpServer` now that the property is required
- Scoped `TestListenFTP` down to valid and invalid property values, removing the test that selected an available port before starting the Processor (this also removes the passive reply parsing, so the intermediate variables are no longer needed)
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NIFI-14719 Added Passive Port Range property to ListenFTP [nifi]
Posted by "Vinz2168 (via GitHub)" <gi...@apache.org>.
Vinz2168 commented on PR #11784:
URL: https://github.com/apache/nifi/pull/11784#issuecomment-6338706575
@exceptionfactory gentle ping: the recommendations from your review are addressed in the latest commit (required property with a `1025-65534` default, multiline description, one method call per line), and all checks are green. Let me know if anything else should change. Thanks!
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org