diff -Nrup a/checkstyle.xml b/checkstyle.xml --- a/checkstyle.xml 2020-01-22 16:10:15.000000000 +0100 +++ b/checkstyle.xml 2023-07-01 07:43:13.965096507 +0200 @@ -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 -Nrup a/src/changes/changes.xml b/src/changes/changes.xml --- a/src/changes/changes.xml 2020-01-22 16:10:15.000000000 +0100 +++ b/src/changes/changes.xml 2023-07-01 08:14:13.037853289 +0200 @@ -68,6 +68,9 @@ The <action> type attribute can be add,u <action type="add" dev="ggregory" due-to="Arturo Bernal, Gary Gregory"> Add and use NetConstants. </action> + <action issue="NET-711" type="fix" dev="ggregory" due-to="Jochen Wiedmann, Gary Gregory"> + Add FTP option to toggle use of return host like CURL. + </action> <action type="add" dev="ggregory" due-to="Gary Gregory"> Add and use SocketClient.applySocketAttributes(). </action> diff -Nrup a/src/main/java/org/apache/commons/net/ftp/FTPClient.java b/src/main/java/org/apache/commons/net/ftp/FTPClient.java --- a/src/main/java/org/apache/commons/net/ftp/FTPClient.java 2020-01-22 16:10:15.000000000 +0100 +++ b/src/main/java/org/apache/commons/net/ftp/FTPClient.java 2023-07-01 08:29:19.425039562 +0200 @@ -292,10 +292,10 @@ import org.apache.commons.net.util.NetCo * @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 { @@ -435,6 +435,18 @@ public class FTPClient extends FTP imple 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> * The entries are the systemType (as determined by {@link FTPClient#getSystemType}) @@ -622,6 +634,8 @@ public class FTPClient extends FTP imple /** 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 @@ -926,35 +940,44 @@ public class FTPClient extends FTP imple 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; - } - } 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); + 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); + } + } + } else { + // Post-3.8 behavior + if (_socket_ == null) { + pasvHost = null; // For unit testing. + } else { + pasvHost = _socket_.getInetAddress().getHostAddress(); } } + this.passiveHost = pasvHost; + this.passivePort = pasvPort; } /** @@ -4031,4 +4054,41 @@ public class FTPClient extends FTP imple { 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 -Nrup a/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java b/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java --- a/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java 2020-01-22 16:10:15.000000000 +0100 +++ b/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java 2023-07-01 08:34:43.956971065 +0200 @@ -91,6 +91,7 @@ public class FTPClientTest extends TestC public String getSystemType() throws IOException { return systemType; } + public void setSystemType(final String type) { systemType = type; } @@ -151,44 +152,68 @@ public class FTPClientTest extends TestC 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()); } 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 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()); } @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 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 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 testParsePassiveModeReplyForLocalAddressWithSimpleNatWorkaroundStrategy() throws Exception { @@ -200,7 +225,11 @@ public class FTPClientTest extends TestC } }); + 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()); } }