NET-588 FTPClient.setPassiveNatWorkaround assumes host is outside site local range
git-svn-id: https://svn.apache.org/repos/asf/commons/proper/net/trunk@1782074 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 6d64229..c782ca5 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -87,6 +87,9 @@
The POP3Mail examples can now get password from console, stdin or an environment variable.
">
+ <action issue="NET-588" type="fix" dev="sebb" due-to="Dave Nice / Thai H">
+ FTPClient.setPassiveNatWorkaround assumes host is outside site local range
+ </action>
<action issue="NET-610" type="fix" dev="sebb" due-to="Sergey Yanzin">
FTPClient.mlistFile incorrectly handles MLST reply
</action>
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 dc22e37..7df1218 100644
--- a/src/main/java/org/apache/commons/net/ftp/FTPClient.java
+++ b/src/main/java/org/apache/commons/net/ftp/FTPClient.java
@@ -406,9 +406,10 @@
private int __controlKeepAliveReplyTimeout=1000;
/**
- * Enable or disable replacement of internal IP in passive mode. Default enabled.
+ * Enable or disable replacement of internal IP in passive mode. Default enabled
+ * using {code NatServerResolverImpl}.
*/
- private boolean __passiveNatWorkaround = true;
+ private HostnameResolver __passiveNatWorkaroundStrategy = new NatServerResolverImpl(this);
/** Pattern for PASV mode responses. Groups: (n,n,n,n),(n),(n) */
private static final java.util.regex.Pattern __PARMS_PAT;
@@ -582,18 +583,13 @@
"Could not parse passive port information.\nServer Reply: " + reply);
}
- if (__passiveNatWorkaround) {
+ if (__passiveNatWorkaroundStrategy != null) {
try {
- InetAddress host = InetAddress.getByName(__passiveHost);
- // reply is a local address, but target is not - assume NAT box changed the PASV reply
- if (host.isSiteLocalAddress()) {
- InetAddress remote = getRemoteAddress();
- if (!remote.isSiteLocalAddress()){
- String hostAddress = remote.getHostAddress();
- fireReplyReceived(0,
- "[Replacing site local address "+__passiveHost+" with "+hostAddress+"]\n");
- __passiveHost = hostAddress;
- }
+ String passiveHost = __passiveNatWorkaroundStrategy.resolve(__passiveHost);
+ if (!__passiveHost.equals(passiveHost)) {
+ fireReplyReceived(0,
+ "[Replacing PASV mode reply address "+__passiveHost+" with "+passiveHost+"]\n");
+ __passiveHost = passiveHost;
}
} catch (UnknownHostException e) { // Should not happen as we are passing in an IP address
throw new MalformedServerReplyException(
@@ -3784,9 +3780,65 @@
* The default is true, i.e. site-local replies are replaced.
* @param enabled true to enable replacing internal IP's in passive
* mode.
+ * @deprecated use {@link #setPassiveNatWorkaroundStrategy(HostnameResolver)} instead
*/
+ @Deprecated
public void setPassiveNatWorkaround(boolean enabled) {
- this.__passiveNatWorkaround = enabled;
+ if (enabled) {
+ this.__passiveNatWorkaroundStrategy = new NatServerResolverImpl(this);
+ } else {
+ this.__passiveNatWorkaroundStrategy = null;
+ }
+ }
+
+ /**
+ * Set the workaround strategy to replace the PASV mode reply addresses.
+ * This gets around the problem that some NAT boxes may change the reply.
+ *
+ * The default implementation is {@code NatServerResolverImpl}, i.e. site-local
+ * replies are replaced.
+ * @param resolver strategy to replace internal IP's in passive mode
+ * or null to disable the workaround (i.e. use PASV mode reply address.)
+ * @since 3.6
+ */
+ public void setPassiveNatWorkaroundStrategy(HostnameResolver resolver) {
+ this.__passiveNatWorkaroundStrategy = resolver;
+ }
+
+ /**
+ * Strategy interface for updating host names received from FTP server
+ * for passive NAT workaround.
+ *
+ * @since 3.6
+ */
+ public static interface HostnameResolver {
+ String resolve(String hostname) throws UnknownHostException;
+ }
+
+ /**
+ * Default strategy for passive NAT workaround (site-local
+ * replies are replaced.)
+ */
+ public static class NatServerResolverImpl implements HostnameResolver {
+ private FTPClient client;
+
+ public NatServerResolverImpl(FTPClient client) {
+ this.client = client;
+ }
+
+ @Override
+ public String resolve(String hostname) throws UnknownHostException {
+ String newHostname = hostname;
+ InetAddress host = InetAddress.getByName(newHostname);
+ // reply is a local address, but target is not - assume NAT box changed the PASV reply
+ if (host.isSiteLocalAddress()) {
+ InetAddress remote = this.client.getRemoteAddress();
+ if (!remote.isSiteLocalAddress()){
+ newHostname = remote.getHostAddress();
+ }
+ }
+ return newHostname;
+ }
}
private OutputStream getBufferedOutputStream(OutputStream outputStream) {
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 c89b5b7..e43f075 100644
--- a/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
+++ b/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
@@ -21,6 +21,8 @@
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
import org.apache.commons.net.ftp.parser.UnixFTPEntryParser;
@@ -125,4 +127,76 @@
}
-}
+ private static class PassiveNatWorkAroundLocalClient extends FTPClient {
+ private String passiveModeServerIP;
+
+ public PassiveNatWorkAroundLocalClient(String passiveModeServerIP) {
+ this.passiveModeServerIP = passiveModeServerIP;
+ }
+
+ @Override
+ public InetAddress getRemoteAddress() {
+ try {
+ return InetAddress.getByName(passiveModeServerIP);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ }
+
+ public void testParsePassiveModeReplyForLocalAddressWithNatWorkaround() throws Exception {
+ FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
+ assertEquals("8.8.8.8", client.getPassiveHost());
+ }
+
+ public void testParsePassiveModeReplyForNonLocalAddressWithNatWorkaround() throws Exception {
+ FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
+ client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
+ assertEquals("8.8.4.4", client.getPassiveHost());
+ }
+
+ @SuppressWarnings("deprecation") // testing deprecated code
+ public void testParsePassiveModeReplyForLocalAddressWithNatWorkaroundDisabled() throws Exception {
+ FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
+ client.setPassiveNatWorkaround(false);
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
+ assertEquals("172.16.204.138", client.getPassiveHost());
+ }
+
+ @SuppressWarnings("deprecation") // testing deprecated code
+ public void testParsePassiveModeReplyForNonLocalAddressWithNatWorkaroundDisabled() throws Exception {
+ FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
+ client.setPassiveNatWorkaround(false);
+ client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
+ assertEquals("8.8.4.4", client.getPassiveHost());
+ }
+
+ public void testParsePassiveModeReplyForLocalAddressWithoutNatWorkaroundStrategy() throws Exception {
+ FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
+ client.setPassiveNatWorkaroundStrategy(null);
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
+ assertEquals("172.16.204.138", client.getPassiveHost());
+ }
+
+ public void testParsePassiveModeReplyForNonLocalAddressWithoutNatWorkaroundStrategy() throws Exception {
+ FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
+ client.setPassiveNatWorkaroundStrategy(null);
+ client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
+ assertEquals("8.8.4.4", client.getPassiveHost());
+ }
+
+ public void testParsePassiveModeReplyForLocalAddressWithSimpleNatWorkaroundStrategy() throws Exception {
+ FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
+ client.setPassiveNatWorkaroundStrategy(new FTPClient.HostnameResolver() {
+ @Override
+ public String resolve(String hostname) throws UnknownHostException {
+ return "4.4.4.4";
+ }
+
+ });
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
+ assertEquals("4.4.4.4", client.getPassiveHost());
+ }
+ }
