You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2022/11/07 15:06:53 UTC
[commons-net] branch master updated: FTP client trusts the host from PASV response by default
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-net.git
The following commit(s) were added to refs/heads/master by this push:
new b0bff89f FTP client trusts the host from PASV response by default
b0bff89f is described below
commit b0bff89f70cfea70009e22f87639816cc3993974
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Mon Nov 7 10:01:26 2022 -0500
FTP client trusts the host from PASV response by default
Adapted patch from Jochen Wiedmann
---
checkstyle.xml | 2 +-
src/changes/changes.xml | 3 +
.../java/org/apache/commons/net/ftp/FTPClient.java | 96 ++++++++++++++++++----
.../org/apache/commons/net/ftp/FTPClientTest.java | 29 +++++++
4 files changed, 111 insertions(+), 19 deletions(-)
diff --git a/checkstyle.xml b/checkstyle.xml
index 787cb1ae..a34cbfc1 100644
--- a/checkstyle.xml
+++ b/checkstyle.xml
@@ -49,7 +49,7 @@ limitations under the License.
</module>
<module name="LineLength">
- <property name="max" value="132"/>
+ <property name="max" value="160"/>
</module>
<module name="TreeWalker">
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index cb8160b1..f2dbd685 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -74,6 +74,9 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Arturo Bernal">
Use Math.min and Math.max method instead of manual calculations. #104.
</action>
+ <action type="fix" dev="ggregory" due-to="Jochen Wiedmann, Gary Gregory">
+ FTP client trusts the host from PASV response by default.
+ </action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="Gary Gregory">
[FTP] Add FTPClient.mdtmInstant(String).
diff --git a/src/main/java/org/apache/commons/net/ftp/FTPClient.java b/src/main/java/org/apache/commons/net/ftp/FTPClient.java
index 09b5f02f..ec91c6db 100644
--- a/src/main/java/org/apache/commons/net/ftp/FTPClient.java
+++ b/src/main/java/org/apache/commons/net/ftp/FTPClient.java
@@ -295,10 +295,10 @@ import org.apache.commons.net.util.NetConstants;
* @see FTPFileEntryParserFactory
* @see DefaultFTPFileEntryParserFactory
* @see FTPClientConfig
- *
* @see org.apache.commons.net.MalformedServerReplyException
*/
public class FTPClient extends FTP implements Configurable {
+
// @since 3.0
private static class CSL implements CopyStreamListener {
@@ -437,6 +437,18 @@ public class FTPClient extends FTP implements Configurable {
*/
public static final String FTP_SYSTEM_TYPE_DEFAULT = "org.apache.commons.net.ftp.systemType.default";
+ /**
+ * The system property that defines the default for {@link #isIpAddressFromPasvResponse()}. This property, if present, configures the default for the
+ * following: If the client receives the servers response for a PASV request, then that response will contain an IP address. If this property is true, then
+ * the client will use that IP address, as requested by the server. This is compatible to version {@code 3.8.0}, and before. If this property is false, or
+ * absent, then the client will ignore that IP address, and instead use the remote address of the control connection.
+ *
+ * @see #isIpAddressFromPasvResponse()
+ * @see #setIpAddressFromPasvResponse(boolean)
+ * @since 3.9.0
+ */
+ public static final String FTP_IP_ADDRESS_FROM_PASV_RESPONSE = "org.apache.commons.net.ftp.ipAddressFromPasvResponse";
+
/**
* The name of an optional systemType properties file ({@value}), which is loaded
* using {@link Class#getResourceAsStream(String)}.<br>
@@ -625,6 +637,8 @@ public class FTPClient extends FTP implements Configurable {
/** Map of FEAT responses. If null, has not been initialized. */
private HashMap<String, Set<String>> featuresMap;
+ private boolean ipAddressFromPasvResponse = Boolean.parseBoolean(System.getProperty(FTPClient.FTP_IP_ADDRESS_FROM_PASV_RESPONSE));
+
/**
* Default FTPClient constructor. Creates a new FTPClient instance
* with the data connection mode set to
@@ -928,35 +942,44 @@ public class FTPClient extends FTP implements Configurable {
protected void _parsePassiveModeReply(final String reply) throws MalformedServerReplyException {
final Matcher m = PARMS_PAT.matcher(reply);
if (!m.find()) {
- throw new MalformedServerReplyException(
- "Could not parse passive host information.\nServer Reply: " + reply);
+ throw new MalformedServerReplyException("Could not parse passive host information.\nServer Reply: " + reply);
}
- this.passiveHost = "0,0,0,0".equals(m.group(1)) ? _socket_.getInetAddress().getHostAddress()
- : m.group(1).replace(',', '.'); // Fix up to look like IP address
+ int pasvPort;
+ // Fix up to look like IP address
+ String pasvHost = "0,0,0,0".equals(m.group(1)) ? _socket_.getInetAddress().getHostAddress() : m.group(1).replace(',', '.');
try {
final int oct1 = Integer.parseInt(m.group(2));
final int oct2 = Integer.parseInt(m.group(3));
- passivePort = (oct1 << 8) | oct2;
+ pasvPort = (oct1 << 8) | oct2;
} catch (final NumberFormatException e) {
- throw new MalformedServerReplyException(
- "Could not parse passive port information.\nServer Reply: " + reply);
+ throw new MalformedServerReplyException("Could not parse passive port information.\nServer Reply: " + reply);
}
- if (passiveNatWorkaroundStrategy != null) {
- try {
- final String newPassiveHost = passiveNatWorkaroundStrategy.resolve(this.passiveHost);
- if (!this.passiveHost.equals(newPassiveHost)) {
- fireReplyReceived(0,
- "[Replacing PASV mode reply address " + this.passiveHost + " with " + newPassiveHost + "]\n");
- this.passiveHost = newPassiveHost;
+ if (isIpAddressFromPasvResponse()) {
+ // Pre-3.9.0 behavior
+ if (passiveNatWorkaroundStrategy != null) {
+ try {
+ final String newPassiveHost = passiveNatWorkaroundStrategy.resolve(pasvHost);
+ if (!pasvHost.equals(newPassiveHost)) {
+ fireReplyReceived(0, "[Replacing PASV mode reply address " + this.passiveHost + " with " + newPassiveHost + "]\n");
+ pasvHost = newPassiveHost;
+ }
+ } catch (final UnknownHostException e) { // Should not happen as we are passing in an IP address
+ throw new MalformedServerReplyException("Could not parse passive host information.\nServer Reply: " + reply);
}
- } catch (final UnknownHostException e) { // Should not happen as we are passing in an IP address
- throw new MalformedServerReplyException(
- "Could not parse passive host information.\nServer Reply: " + reply);
+ }
+ } else {
+ // Post-3.8 behavior
+ if (_socket_ == null) {
+ pasvHost = null; // For unit testing.
+ } else {
+ pasvHost = _socket_.getInetAddress().getHostAddress();
}
}
+ this.passiveHost = pasvHost;
+ this.passivePort = pasvPort;
}
/**
@@ -4140,4 +4163,41 @@ public class FTPClient extends FTP implements Configurable {
{
return FTPReply.isPositiveCompletion(smnt(pathname));
}
+
+ /**
+ * Returns, whether the IP address from the server's response should be used.
+ * Until 3.9.0, this has always been the case. Beginning with 3.9.0,
+ * that IP address will be silently ignored, and replaced with the remote
+ * IP address of the control connection, unless this configuration option is
+ * given, which restores the old behavior. To enable this by default, use
+ * the system property {@link FTPClient#FTP_IP_ADDRESS_FROM_PASV_RESPONSE}.
+ * @return True, if the IP address from the server's response will be used
+ * (pre-3.9 compatible behavior), or false (ignore that IP address).
+ *
+ * @see FTPClient#FTP_IP_ADDRESS_FROM_PASV_RESPONSE
+ * @see #setIpAddressFromPasvResponse(boolean)
+ * @since 3.9.0
+ */
+ public boolean isIpAddressFromPasvResponse() {
+ return ipAddressFromPasvResponse;
+ }
+
+ /**
+ * Sets whether the IP address from the server's response should be used.
+ * Until 3.9.0, this has always been the case. Beginning with 3.9.0,
+ * that IP address will be silently ignored, and replaced with the remote
+ * IP address of the control connection, unless this configuration option is
+ * given, which restores the old behavior. To enable this by default, use
+ * the system property {@link FTPClient#FTP_IP_ADDRESS_FROM_PASV_RESPONSE}.
+ *
+ * @param usingIpAddressFromPasvResponse True, if the IP address from the
+ * server's response should be used (pre-3.9.0 compatible behavior), or
+ * false (ignore that IP address).
+ * @see FTPClient#FTP_IP_ADDRESS_FROM_PASV_RESPONSE
+ * @see #isIpAddressFromPasvResponse
+ * @since 3.9.0
+ */
+ public void setIpAddressFromPasvResponse(boolean usingIpAddressFromPasvResponse) {
+ this.ipAddressFromPasvResponse = usingIpAddressFromPasvResponse;
+ }
}
diff --git a/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java b/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
index fcba3a76..33458063 100644
--- a/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
+++ b/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
@@ -37,6 +37,7 @@ public class FTPClientTest extends TestCase {
public String getSystemType() throws IOException {
return systemType;
}
+
public void setSystemType(final String type) {
systemType = type;
}
@@ -98,52 +99,80 @@ public class FTPClientTest extends TestCase {
public void testParsePassiveModeReplyForLocalAddressWithNatWorkaround() throws Exception {
final FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
+ client.setIpAddressFromPasvResponse(true);
client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
assertEquals("8.8.8.8", client.getPassiveHost());
+ client.setIpAddressFromPasvResponse(false);
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
+ assertNull(client.getPassiveHost());
}
@SuppressWarnings("deprecation") // testing deprecated code
public void testParsePassiveModeReplyForLocalAddressWithNatWorkaroundDisabled() throws Exception {
final FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
client.setPassiveNatWorkaround(false);
+ client.setIpAddressFromPasvResponse(true);
client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
assertEquals("172.16.204.138", client.getPassiveHost());
+ client.setIpAddressFromPasvResponse(false);
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
+ assertNull(client.getPassiveHost());
}
public void testParsePassiveModeReplyForLocalAddressWithoutNatWorkaroundStrategy() throws Exception {
final FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
client.setPassiveNatWorkaroundStrategy(null);
+ client.setIpAddressFromPasvResponse(true);
client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
assertEquals("172.16.204.138", client.getPassiveHost());
+ client.setIpAddressFromPasvResponse(false);
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
+ assertNull(client.getPassiveHost());
}
public void testParsePassiveModeReplyForLocalAddressWithSimpleNatWorkaroundStrategy() throws Exception {
final FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
client.setPassiveNatWorkaroundStrategy(hostname -> "4.4.4.4");
+ client.setIpAddressFromPasvResponse(true);
client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
assertEquals("4.4.4.4", client.getPassiveHost());
+ client.setIpAddressFromPasvResponse(false);
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
+ assertNull(client.getPassiveHost());
}
public void testParsePassiveModeReplyForNonLocalAddressWithNatWorkaround() throws Exception {
final FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
+ client.setIpAddressFromPasvResponse(true);
client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
assertEquals("8.8.4.4", client.getPassiveHost());
+ client.setIpAddressFromPasvResponse(false);
+ client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
+ assertNull(client.getPassiveHost());
}
@SuppressWarnings("deprecation") // testing deprecated code
public void testParsePassiveModeReplyForNonLocalAddressWithNatWorkaroundDisabled() throws Exception {
final FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
client.setPassiveNatWorkaround(false);
+ client.setIpAddressFromPasvResponse(true);
client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
assertEquals("8.8.4.4", client.getPassiveHost());
+ client.setIpAddressFromPasvResponse(false);
+ client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
+ assertNull(client.getPassiveHost());
}
public void testParsePassiveModeReplyForNonLocalAddressWithoutNatWorkaroundStrategy() throws Exception {
final FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
client.setPassiveNatWorkaroundStrategy(null);
+ client.setIpAddressFromPasvResponse(true);
client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
assertEquals("8.8.4.4", client.getPassiveHost());
+ client.setIpAddressFromPasvResponse(false);
+ client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
+ assertNull(client.getPassiveHost());
}
public void testParserCachingNullKey() throws Exception {