Added check for passive ports allocation to check if the port is not bound by another application (FTPSERVER-302)
git-svn-id: https://svn.apache.org/repos/asf/mina/ftpserver/trunk@782448 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/core/src/main/java/org/apache/ftpserver/DataConnectionConfigurationFactory.java b/core/src/main/java/org/apache/ftpserver/DataConnectionConfigurationFactory.java
index 8c7e877..5a0aa2c 100644
--- a/core/src/main/java/org/apache/ftpserver/DataConnectionConfigurationFactory.java
+++ b/core/src/main/java/org/apache/ftpserver/DataConnectionConfigurationFactory.java
@@ -48,7 +48,7 @@
private String passiveAddress;
private String passiveExternalAddress;
- private PassivePorts passivePorts = new PassivePorts(new int[] { 0 });
+ private PassivePorts passivePorts = new PassivePorts(new int[] { 0 }, true);
private boolean implicitSsl;
/**
@@ -249,7 +249,7 @@
* @param passivePorts The passive ports string
*/
public void setPassivePorts(String passivePorts) {
- this.passivePorts = new PassivePorts(passivePorts);
+ this.passivePorts = new PassivePorts(passivePorts, true);
}
diff --git a/core/src/main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java b/core/src/main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java
index 66224ac..19e5e96 100644
--- a/core/src/main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java
+++ b/core/src/main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java
@@ -154,8 +154,7 @@
}
/**
- * Initiate a data connection in passive mode (server listening). It returns
- * the success flag.
+ * Initiate a data connection in passive mode (server listening).
*/
public synchronized InetSocketAddress initPassiveDataConnection()
throws DataConnectionException {
diff --git a/core/src/main/java/org/apache/ftpserver/impl/PassivePorts.java b/core/src/main/java/org/apache/ftpserver/impl/PassivePorts.java
index 7bed30e..2750c0e 100644
--- a/core/src/main/java/org/apache/ftpserver/impl/PassivePorts.java
+++ b/core/src/main/java/org/apache/ftpserver/impl/PassivePorts.java
@@ -19,6 +19,8 @@
package org.apache.ftpserver.impl;
+import java.io.IOException;
+import java.net.ServerSocket;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
@@ -42,6 +44,8 @@
private String passivePortsString;
+ private boolean checkIfBound;
+
/**
* Parse a string containing passive ports
*
@@ -145,13 +149,13 @@
}
}
- public PassivePorts(final String passivePorts) {
- this(parse(passivePorts));
+ public PassivePorts(final String passivePorts, boolean checkIfBound) {
+ this(parse(passivePorts), checkIfBound);
this.passivePortsString = passivePorts;
}
- public PassivePorts(final int[] passivePorts) {
+ public PassivePorts(final int[] passivePorts, boolean checkIfBound) {
if (passivePorts != null) {
this.passivePorts = passivePorts.clone();
} else {
@@ -159,12 +163,44 @@
}
reservedPorts = new boolean[passivePorts.length];
+ this.checkIfBound = checkIfBound;
}
+ private boolean checkPortUnbound(int port) {
+ // is this check disabled?
+ if(!checkIfBound) {
+ return true;
+ }
+
+ // if using 0 port, it will always be available
+ if(port == 0) {
+ return true;
+ }
+
+ ServerSocket ss = null;
+ try {
+ ss = new ServerSocket(port);
+ ss.setReuseAddress(true);
+ return true;
+ } catch (IOException e) {
+ // port probably in used, check next
+ return false;
+ } finally {
+ if(ss != null) {
+ try {
+ ss.close();
+ } catch (IOException e) {
+ // could not close, check next
+ return false;
+ }
+ }
+ }
+ }
+
public int reserveNextPort() {
// search for a free port
for (int i = 0; i < passivePorts.length; i++) {
- if (!reservedPorts[i]) {
+ if (!reservedPorts[i] && checkPortUnbound(passivePorts[i])) {
if (passivePorts[i] != 0) {
reservedPorts[i] = true;
}
diff --git a/core/src/main/java/org/apache/ftpserver/impl/ServerDataConnectionFactory.java b/core/src/main/java/org/apache/ftpserver/impl/ServerDataConnectionFactory.java
index d07344c..6e06e70 100644
--- a/core/src/main/java/org/apache/ftpserver/impl/ServerDataConnectionFactory.java
+++ b/core/src/main/java/org/apache/ftpserver/impl/ServerDataConnectionFactory.java
@@ -39,7 +39,7 @@
void initActiveDataConnection(InetSocketAddress address);
/**
- * Initate the passive data connection.
+ * Initiate the passive data connection.
*
* @return The {@link InetSocketAddress} on which the data connection if
* bound.
diff --git a/core/src/test/java/org/apache/ftpserver/clienttests/PasvUsedPortTest.java b/core/src/test/java/org/apache/ftpserver/clienttests/PasvUsedPortTest.java
new file mode 100644
index 0000000..b838ab7
--- /dev/null
+++ b/core/src/test/java/org/apache/ftpserver/clienttests/PasvUsedPortTest.java
@@ -0,0 +1,77 @@
+/*
+ * 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
+ *
+ * http://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.ftpserver.clienttests;
+
+import java.net.ServerSocket;
+
+import org.apache.ftpserver.DataConnectionConfigurationFactory;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.listener.ListenerFactory;
+import org.apache.ftpserver.test.TestUtil;
+
+/**
+*
+* @author The Apache MINA Project (dev@mina.apache.org)
+*
+*/
+public class PasvUsedPortTest extends ClientTestTemplate {
+
+ private int passivePort;
+
+ @Override
+ protected FtpServerFactory createServer() throws Exception {
+ FtpServerFactory server = super.createServer();
+
+ ListenerFactory listenerFactory = new ListenerFactory(server
+ .getListener("default"));
+
+ DataConnectionConfigurationFactory dccFactory = new DataConnectionConfigurationFactory();
+
+ passivePort = TestUtil.findFreePort(12444);
+
+ dccFactory.setPassivePorts(passivePort + "-" + (passivePort + 1));
+
+ listenerFactory.setDataConnectionConfiguration(dccFactory
+ .createDataConnectionConfiguration());
+
+ server.addListener("default", listenerFactory.createListener());
+
+ return server;
+ }
+
+ public void testPasvWithUsedPort() throws Exception {
+ // bind to the first available passive port
+ ServerSocket ss = new ServerSocket(passivePort);
+
+ client.login(ADMIN_USERNAME, ADMIN_PASSWORD);
+ client.pasv();
+ assertEquals("227 Entering Passive Mode (127,0,0,1,48,157)", client.getReplyString().trim());
+ client.quit();
+ client.disconnect();
+
+ // close blocking socket
+ ss.close();
+
+ client.connect("localhost", getListenerPort());
+ client.login(ADMIN_USERNAME, ADMIN_PASSWORD);
+ client.pasv();
+ assertEquals("227 Entering Passive Mode (127,0,0,1,48,156)", client.getReplyString().trim());
+ }
+}
diff --git a/core/src/test/java/org/apache/ftpserver/impl/PassivePortsTest.java b/core/src/test/java/org/apache/ftpserver/impl/PassivePortsTest.java
index 1d3218b..2c3ffc0 100644
--- a/core/src/test/java/org/apache/ftpserver/impl/PassivePortsTest.java
+++ b/core/src/test/java/org/apache/ftpserver/impl/PassivePortsTest.java
@@ -29,21 +29,21 @@
public class PassivePortsTest extends TestCase {
public void testParseSingleValue() {
- PassivePorts ports = new PassivePorts("123");
+ PassivePorts ports = new PassivePorts("123", false);
assertEquals(123, ports.reserveNextPort());
assertEquals(-1, ports.reserveNextPort());
}
public void testParseMaxValue() {
- PassivePorts ports = new PassivePorts("65535");
+ PassivePorts ports = new PassivePorts("65535", false);
assertEquals(65535, ports.reserveNextPort());
assertEquals(-1, ports.reserveNextPort());
}
public void testParseMinValue() {
- PassivePorts ports = new PassivePorts("0");
+ PassivePorts ports = new PassivePorts("0", false);
assertEquals(0, ports.reserveNextPort());
assertEquals(0, ports.reserveNextPort());
@@ -54,7 +54,7 @@
public void testParseTooLargeValue() {
try {
- new PassivePorts("65536");
+ new PassivePorts("65536", false);
fail("Must fail due to too high port number");
} catch (IllegalArgumentException e) {
// ok
@@ -63,7 +63,7 @@
public void testParseNonNumericValue() {
try {
- new PassivePorts("foo");
+ new PassivePorts("foo", false);
fail("Must fail due to non numerical port number");
} catch (IllegalArgumentException e) {
// ok
@@ -71,7 +71,7 @@
}
public void testParseListOfValues() {
- PassivePorts ports = new PassivePorts("123, 456,\t\n789");
+ PassivePorts ports = new PassivePorts("123, 456,\t\n789", false);
assertEquals(123, ports.reserveNextPort());
assertEquals(456, ports.reserveNextPort());
@@ -80,7 +80,7 @@
}
public void testParseListOfValuesOrder() {
- PassivePorts ports = new PassivePorts("123, 789, 456");
+ PassivePorts ports = new PassivePorts("123, 789, 456", false);
assertEquals(123, ports.reserveNextPort());
assertEquals(789, ports.reserveNextPort());
@@ -89,7 +89,7 @@
}
public void testParseListOfValuesDuplicate() {
- PassivePorts ports = new PassivePorts("123, 789, 456, 789");
+ PassivePorts ports = new PassivePorts("123, 789, 456, 789", false);
assertEquals(123, ports.reserveNextPort());
assertEquals(789, ports.reserveNextPort());
@@ -98,7 +98,7 @@
}
public void testParseSimpleRange() {
- PassivePorts ports = new PassivePorts("123-125");
+ PassivePorts ports = new PassivePorts("123-125", false);
assertEquals(123, ports.reserveNextPort());
assertEquals(124, ports.reserveNextPort());
@@ -107,7 +107,7 @@
}
public void testParseMultipleRanges() {
- PassivePorts ports = new PassivePorts("123-125, 127-128, 130-132");
+ PassivePorts ports = new PassivePorts("123-125, 127-128, 130-132", false);
assertEquals(123, ports.reserveNextPort());
assertEquals(124, ports.reserveNextPort());
@@ -121,7 +121,7 @@
}
public void testParseMixedRangeAndSingle() {
- PassivePorts ports = new PassivePorts("123-125, 126, 128-129");
+ PassivePorts ports = new PassivePorts("123-125, 126, 128-129", false);
assertEquals(123, ports.reserveNextPort());
assertEquals(124, ports.reserveNextPort());
@@ -133,7 +133,7 @@
}
public void testParseOverlapingRanges() {
- PassivePorts ports = new PassivePorts("123-125, 124-126");
+ PassivePorts ports = new PassivePorts("123-125, 124-126", false);
assertEquals(123, ports.reserveNextPort());
assertEquals(124, ports.reserveNextPort());
@@ -143,7 +143,7 @@
}
public void testParseOverlapingRangesorder() {
- PassivePorts ports = new PassivePorts("124-126, 123-125");
+ PassivePorts ports = new PassivePorts("124-126, 123-125", false);
assertEquals(124, ports.reserveNextPort());
assertEquals(125, ports.reserveNextPort());
@@ -153,7 +153,7 @@
}
public void testParseOpenLowerRange() {
- PassivePorts ports = new PassivePorts("9, -3");
+ PassivePorts ports = new PassivePorts("9, -3", false);
assertEquals(9, ports.reserveNextPort());
assertEquals(1, ports.reserveNextPort());
@@ -163,7 +163,7 @@
}
public void testParseOpenUpperRange() {
- PassivePorts ports = new PassivePorts("65533-");
+ PassivePorts ports = new PassivePorts("65533-", false);
assertEquals(65533, ports.reserveNextPort());
assertEquals(65534, ports.reserveNextPort());
@@ -172,7 +172,7 @@
}
public void testParseOpenUpperRange3() {
- PassivePorts ports = new PassivePorts("65533-, 65532-");
+ PassivePorts ports = new PassivePorts("65533-, 65532-", false);
assertEquals(65533, ports.reserveNextPort());
assertEquals(65534, ports.reserveNextPort());
@@ -182,7 +182,7 @@
}
public void testParseOpenUpperRange2() {
- PassivePorts ports = new PassivePorts("65533-, 1");
+ PassivePorts ports = new PassivePorts("65533-, 1", false);
assertEquals(65533, ports.reserveNextPort());
assertEquals(65534, ports.reserveNextPort());
@@ -192,7 +192,7 @@
}
public void testParseRelease() {
- PassivePorts ports = new PassivePorts("123, 456,789");
+ PassivePorts ports = new PassivePorts("123, 456,789", false);
assertEquals(123, ports.reserveNextPort());
assertEquals(456, ports.reserveNextPort());
