Author: sebb
Date: Tue Feb 7 20:02:28 2017
New Revision: 1782074
URL: http://svn.apache.org/viewvc?rev=1782074&view=rev
Log:
NET-588 FTPClient.setPassiveNatWorkaround assumes host is outside site local
range
Modified:
commons/proper/net/trunk/src/changes/changes.xml
commons/proper/net/trunk/src/main/java/org/apache/commons/net/ftp/FTPClient.java
commons/proper/net/trunk/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
Modified: commons/proper/net/trunk/src/changes/changes.xml
URL:
http://svn.apache.org/viewvc/commons/proper/net/trunk/src/changes/changes.xml?rev=1782074&r1=1782073&r2=1782074&view=diff
==============================================================================
--- commons/proper/net/trunk/src/changes/changes.xml [utf-8] (original)
+++ commons/proper/net/trunk/src/changes/changes.xml [utf-8] Tue Feb 7
20:02:28 2017
@@ -87,6 +87,9 @@ without checking it if is a space.
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>
Modified:
commons/proper/net/trunk/src/main/java/org/apache/commons/net/ftp/FTPClient.java
URL:
http://svn.apache.org/viewvc/commons/proper/net/trunk/src/main/java/org/apache/commons/net/ftp/FTPClient.java?rev=1782074&r1=1782073&r2=1782074&view=diff
==============================================================================
---
commons/proper/net/trunk/src/main/java/org/apache/commons/net/ftp/FTPClient.java
(original)
+++
commons/proper/net/trunk/src/main/java/org/apache/commons/net/ftp/FTPClient.java
Tue Feb 7 20:02:28 2017
@@ -406,9 +406,10 @@ implements Configurable
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 @@ implements Configurable
"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 @@ implements Configurable
* 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) {
Modified:
commons/proper/net/trunk/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
URL:
http://svn.apache.org/viewvc/commons/proper/net/trunk/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java?rev=1782074&r1=1782073&r2=1782074&view=diff
==============================================================================
---
commons/proper/net/trunk/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
(original)
+++
commons/proper/net/trunk/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
Tue Feb 7 20:02:28 2017
@@ -21,6 +21,8 @@ package org.apache.commons.net.ftp;
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 @@ public class FTPClientTest extends TestC
}
-}
+ 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());
+ }
+ }
