diff --git a/src/main/java/org/apache/commons/net/ftp/FTPHTTPClient.java b/src/main/java/org/apache/commons/net/ftp/FTPHTTPClient.java index 4ee0c81b1..83176f8b4 100644 --- a/src/main/java/org/apache/commons/net/ftp/FTPHTTPClient.java +++ b/src/main/java/org/apache/commons/net/ftp/FTPHTTPClient.java @@ -46,7 +46,7 @@ public class FTPHTTPClient extends FTPClient { private final String proxyUserName; private final String proxyPassword; private final Charset charset; - private String tunnelHost; // Save the host when setting up a tunnel (needed for EPSV) + private String tunnelHost; // Save the host when setting up a tunnel (needed for EPSV and PASV) /** * Create an instance using the UTF-8 encoding, with no proxy credentials. @@ -138,7 +138,8 @@ protected Socket _openDataConnection_(final String command, final String arg) th return null; } _parsePassiveModeReply(_replyLines.get(0)); - passiveHost = getPassiveHost(); + // Without the legacy flag, the parser substitutes the control socket's peer, which here is the proxy, not the FTP server + passiveHost = isIpAddressFromPasvResponse() ? getPassiveHost() : tunnelHost; } final Socket socket = _socketFactory_.createSocket(proxyHost, proxyPort); @@ -159,6 +160,7 @@ public void connect(final String host, final int port) throws SocketException, I _socket_ = _socketFactory_.createSocket(proxyHost, proxyPort); _input_ = _socket_.getInputStream(); _output_ = _socket_.getOutputStream(); + tunnelHost = host; final Reader socketIsReader; try { socketIsReader = tunnelHandshake(host, port, _input_, _output_); @@ -173,7 +175,6 @@ private BufferedReader tunnelHandshake(final String host, final int port, final final String connectString = "CONNECT " + host + ":" + port + " HTTP/1.1"; final String hostString = "Host: " + host + ":" + port; - this.tunnelHost = host; output.write(connectString.getBytes(charset)); output.write(CRLF); output.write(hostString.getBytes(charset)); diff --git a/src/test/java/org/apache/commons/net/ftp/FTPHTTPClientTest.java b/src/test/java/org/apache/commons/net/ftp/FTPHTTPClientTest.java new file mode 100644 index 000000000..1ac6fd979 --- /dev/null +++ b/src/test/java/org/apache/commons/net/ftp/FTPHTTPClientTest.java @@ -0,0 +1,99 @@ +/* + * 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 + * + * https://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.commons.net.ftp; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.net.InetAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +import org.junit.jupiter.api.Test; + +class FTPHTTPClientTest { + + private static String readRequestLine(final BufferedReader reader) throws IOException { + final String requestLine = reader.readLine(); + for (String line = reader.readLine(); line != null && !line.isEmpty(); line = reader.readLine()) { + // skip headers + } + return requestLine; + } + + private static void write(final OutputStream out, final String s) throws IOException { + out.write(s.getBytes(StandardCharsets.US_ASCII)); + out.flush(); + } + + /** + * A PASV data connection must be tunneled to the FTP server, not to the proxy's own address on a port chosen by the FTP server. + */ + @Test + void testPassiveDataConnectionTunnelsToControlHost() throws Exception { + final List connectLines = new CopyOnWriteArrayList<>(); + try (ServerSocket proxy = new ServerSocket(0, 2, InetAddress.getLoopbackAddress())) { + final Thread server = new Thread(() -> { + try (Socket control = proxy.accept()) { + final BufferedReader in = new BufferedReader(new InputStreamReader(control.getInputStream(), StandardCharsets.US_ASCII)); + final OutputStream out = control.getOutputStream(); + connectLines.add(readRequestLine(in)); + write(out, "HTTP/1.1 200 Connection established\r\n\r\n"); + write(out, "220 ready\r\n"); + for (String cmd = in.readLine(); cmd != null; cmd = in.readLine()) { + if (cmd.startsWith("PASV")) { + write(out, "227 Entering Passive Mode (10,1,2,3,4,1).\r\n"); + try (Socket data = proxy.accept()) { + final BufferedReader dataIn = new BufferedReader(new InputStreamReader(data.getInputStream(), StandardCharsets.US_ASCII)); + connectLines.add(readRequestLine(dataIn)); + write(data.getOutputStream(), "HTTP/1.1 403 Forbidden\r\n\r\n"); + } + } else { + write(out, "500 unknown\r\n"); + } + } + } catch (final IOException ignored) { + // test ends + } + }); + server.setDaemon(true); + server.start(); + + final FTPHTTPClient client = new FTPHTTPClient(proxy.getInetAddress().getHostAddress(), proxy.getLocalPort()); + client.setDefaultTimeout(10_000); + client.connect("ftp.example.invalid", 21); + try { + client.enterLocalPassiveMode(); + assertThrows(IOException.class, client::listNames); + } finally { + client.disconnect(); + } + server.join(10_000); + } + assertEquals(2, connectLines.size(), connectLines::toString); + assertEquals("CONNECT ftp.example.invalid:21 HTTP/1.1", connectLines.get(0)); + assertEquals("CONNECT ftp.example.invalid:1025 HTTP/1.1", connectLines.get(1)); + } +}