I would like to make a comment on the current implementation -
Do you think we want to wait indefinitely when there is no available
passive port, or should we have a timed wait, say 60 seconds. If we
can't get an available passive port in the timeout period, we should
send an error back to the client. This is mostly based on the fact
that most FTP clients would have their data connection timeout set,
which is very small. Or even better, since the current version is not
supposed to be used with too little passive ports than the number of
concurrent clients, should we error out right way with 4XX message
indicating a data connection could not be opened?
Regards,
Sai Pullabhotla
On Thu, Mar 25, 2010 at 7:51 AM, Niklas Gustavsson <nik...@protocol7.com> wrote:
> Interesting! Could you send the patch without whitespace/formatting
> changes, would make it much easier to follow.
>
> /niklas
>
> On Thu, Mar 25, 2010 at 1:46 PM, David Latorre <dvl...@gmail.com> wrote:
>> hello Sai,
>>
>> I didn't have time to fully review the code but i made these nearly
>> random changes in IODataConnectionFactory and
>> DefaultDataConnectionConfiguration and the server is not getting
>> blocked anymore...
>>
>>
>>
>> Index:
>> main/java/org/apache/ftpserver/impl/DefaultDataConnectionConfiguration.java
>> ===================================================================
>> ---
>> main/java/org/apache/ftpserver/impl/DefaultDataConnectionConfiguration.java
>> (revision
>> 927354)
>> +++
>> main/java/org/apache/ftpserver/impl/DefaultDataConnectionConfiguration.java
>> (working
>> copy)
>> @@ -129,7 +129,8 @@
>> * port will be used.
>> */
>> public synchronized int requestPassivePort() {
>> - int dataPort = -1;
>> +
>> + int dataPort = -1;
>> int loopTimes = 2;
>> Thread currThread = Thread.currentThread();
>>
>> @@ -164,8 +165,7 @@
>> */
>> public synchronized void releasePassivePort(final int port) {
>> passivePorts.releasePort(port);
>> -
>> - notify();
>> + notifyAll();
>> }
>>
>> /**
>> Index: main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java
>> ===================================================================
>> --- main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java
>> (revision
>> 927354)
>> +++ main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java
>> (working
>> copy)
>> @@ -44,446 +44,459 @@
>> *
>> * We can get the FTP data connection using this class. It uses either PORT
>> or
>> * PASV command.
>> - *
>> + *
>> * @author <a href="http://mina.apache.org";>Apache MINA Project</a>
>> */
>> public class IODataConnectionFactory implements ServerDataConnectionFactory
>> {
>>
>> - private final Logger LOG = LoggerFactory
>> - .getLogger(IODataConnectionFactory.class);
>> + private final Logger LOG = LoggerFactory
>> + .getLogger(IODataConnectionFactory.class);
>>
>> - private FtpServerContext serverContext;
>> + private FtpServerContext serverContext;
>>
>> - private Socket dataSoc;
>> + private Socket dataSoc;
>>
>> - ServerSocket servSoc;
>> + ServerSocket servSoc;
>>
>> - InetAddress address;
>> + InetAddress address;
>>
>> - int port = 0;
>> + int port = 0;
>>
>> - long requestTime = 0L;
>> + long requestTime = 0L;
>>
>> - boolean passive = false;
>> + boolean passive = false;
>>
>> - boolean secure = false;
>> + boolean secure = false;
>>
>> - private boolean isZip = false;
>> + private boolean isZip = false;
>>
>> - InetAddress serverControlAddress;
>> + InetAddress serverControlAddress;
>>
>> - FtpIoSession session;
>> + FtpIoSession session;
>>
>> - public IODataConnectionFactory(final FtpServerContext serverContext,
>> - final FtpIoSession session) {
>> - this.session = session;
>> - this.serverContext = serverContext;
>> - if (session.getListener().getDataConnectionConfiguration()
>> - .isImplicitSsl()) {
>> - secure = true;
>> - }
>> - }
>> + public IODataConnectionFactory(final FtpServerContext serverContext,
>> + final FtpIoSession session) {
>> + this.session = session;
>> + this.serverContext = serverContext;
>> + if (session.getListener().getDataConnectionConfiguration()
>> + .isImplicitSsl()) {
>> + secure = true;
>> + }
>> + }
>>
>> - /**
>> - * Close data socket.
>> - * This method must be idempotent as we might call it multiple
>> times during disconnect.
>> - */
>> - public synchronized void closeDataConnection() {
>> + /**
>> + * Close data socket. This method must be idempotent as we might
>> call it
>> + * multiple times during disconnect.
>> + */
>> + public synchronized void closeDataConnection() {
>>
>> - // close client socket if any
>> - if (dataSoc != null) {
>> - try {
>> - dataSoc.close();
>> - } catch (Exception ex) {
>> - LOG.warn("FtpDataConnection.closeDataSocket()", ex);
>> - }
>> - dataSoc = null;
>> - }
>> + // close client socket if any
>> + if (dataSoc != null) {
>> + try {
>> + dataSoc.close();
>> + } catch (Exception ex) {
>> +
>> LOG.warn("FtpDataConnection.closeDataSocket()", ex);
>> + }
>> + dataSoc = null;
>> + }
>>
>> - // close server socket if any
>> - if (servSoc != null) {
>> - try {
>> - servSoc.close();
>> - } catch (Exception ex) {
>> - LOG.warn("FtpDataConnection.closeDataSocket()", ex);
>> - }
>> + // close server socket if any
>> + if (servSoc != null) {
>> + try {
>> + servSoc.close();
>> + } catch (Exception ex) {
>> +
>> LOG.warn("FtpDataConnection.closeDataSocket()", ex);
>> + }
>>
>> - if (session != null) {
>> - DataConnectionConfiguration dcc = session.getListener()
>> - .getDataConnectionConfiguration();
>> - if (dcc != null) {
>> - dcc.releasePassivePort(port);
>> - }
>> - }
>> + servSoc = null;
>> + }
>>
>> - servSoc = null;
>> - }
>> + // release passive ports
>> + if (session != null) {
>> + DataConnectionConfiguration dcc = session.getListener()
>> + .getDataConnectionConfiguration();
>> + if (dcc != null) {
>> + dcc.releasePassivePort(port);
>> + }
>> + }
>> +
>> + // reset request time
>> + requestTime = 0L;
>> + }
>>
>> - // reset request time
>> - requestTime = 0L;
>> - }
>> + /**
>> + * Port command.
>> + */
>> + public synchronized void initActiveDataConnection(
>> + final InetSocketAddress address) {
>>
>> - /**
>> - * Port command.
>> - */
>> - public synchronized void initActiveDataConnection(
>> - final InetSocketAddress address) {
>> + // close old sockets if any
>> + closeDataConnection();
>>
>> - // close old sockets if any
>> - closeDataConnection();
>> + // set variables
>> + passive = false;
>> + this.address = address.getAddress();
>> + port = address.getPort();
>> + requestTime = System.currentTimeMillis();
>> + }
>>
>> - // set variables
>> - passive = false;
>> - this.address = address.getAddress();
>> - port = address.getPort();
>> - requestTime = System.currentTimeMillis();
>> - }
>> + private SslConfiguration getSslConfiguration() {
>> + DataConnectionConfiguration dataCfg = session.getListener()
>> + .getDataConnectionConfiguration();
>>
>> - private SslConfiguration getSslConfiguration() {
>> - DataConnectionConfiguration dataCfg = session.getListener()
>> - .getDataConnectionConfiguration();
>> + SslConfiguration configuration =
>> dataCfg.getSslConfiguration();
>>
>> - SslConfiguration configuration = dataCfg.getSslConfiguration();
>> + // fall back if no configuration has been provided on the
>> data
>> + // connection config
>> + if (configuration == null) {
>> + configuration =
>> session.getListener().getSslConfiguration();
>> + }
>>
>> - // fall back if no configuration has been provided on the
>> data connection config
>> - if (configuration == null) {
>> - configuration = session.getListener().getSslConfiguration();
>> - }
>> + return configuration;
>> + }
>>
>> - return configuration;
>> - }
>> + /**
>> + * Initiate a data connection in passive mode (server listening).
>> + */
>> + public synchronized InetSocketAddress initPassiveDataConnection()
>> + throws DataConnectionException {
>> + LOG.debug("Initiating passive data connection");
>> + // close old sockets if any
>> + closeDataConnection();
>>
>> - /**
>> - * Initiate a data connection in passive mode (server listening).
>> - */
>> - public synchronized InetSocketAddress initPassiveDataConnection()
>> - throws DataConnectionException {
>> - LOG.debug("Initiating passive data connection");
>> - // close old sockets if any
>> - closeDataConnection();
>> + // get the passive port
>> + int passivePort = session.getListener()
>> +
>> .getDataConnectionConfiguration().requestPassivePort();
>> + if (passivePort == -1) {
>> + servSoc = null;
>> + throw new DataConnectionException(
>> + "Cannot find an available passive
>> port.");
>> + }
>>
>> - // get the passive port
>> - int passivePort = session.getListener()
>> - .getDataConnectionConfiguration().requestPassivePort();
>> - if (passivePort == -1) {
>> - servSoc = null;
>> - throw new DataConnectionException(
>> - "Cannot find an available passive port.");
>> - }
>> + // open passive server socket and get parameters
>> + try {
>> + DataConnectionConfiguration dataCfg =
>> session.getListener()
>> + .getDataConnectionConfiguration();
>>
>> - // open passive server socket and get parameters
>> - try {
>> - DataConnectionConfiguration dataCfg = session.getListener()
>> - .getDataConnectionConfiguration();
>> + String passiveAddress = dataCfg.getPassiveAddress();
>>
>> - String passiveAddress = dataCfg.getPassiveAddress();
>> + if (passiveAddress == null) {
>> + address = serverControlAddress;
>> + } else {
>> + address =
>> resolveAddress(dataCfg.getPassiveAddress());
>> + }
>>
>> - if (passiveAddress == null) {
>> - address = serverControlAddress;
>> - } else {
>> - address = resolveAddress(dataCfg.getPassiveAddress());
>> - }
>> + if (secure) {
>> + LOG
>> + .debug(
>> + "Opening SSL
>> passive data connection on address \"{}\" and port {}",
>> + address,
>> passivePort);
>> + SslConfiguration ssl = getSslConfiguration();
>> + if (ssl == null) {
>> + throw new DataConnectionException(
>> + "Data connection SSL
>> required but not configured.");
>> + }
>>
>> - if (secure) {
>> - LOG
>> - .debug(
>> - "Opening SSL passive data connection
>> on address \"{}\" and port {}",
>> - address, passivePort);
>> - SslConfiguration ssl = getSslConfiguration();
>> - if (ssl == null) {
>> - throw new DataConnectionException(
>> - "Data connection SSL required but not
>> configured.");
>> - }
>> + // this method does not actually create the
>> SSL socket, due to a
>> + // JVM bug
>> + //
>> (https://issues.apache.org/jira/browse/FTPSERVER-241).
>> + // Instead, it creates a regular
>> + // ServerSocket that will be wrapped as a
>> SSL socket in
>> + // createDataSocket()
>> + servSoc = new ServerSocket(passivePort, 0,
>> address);
>> + LOG
>> + .debug(
>> + "SSL Passive
>> data connection created on address \"{}\" and port {}",
>> + address,
>> passivePort);
>> + } else {
>> + LOG
>> + .debug(
>> + "Opening
>> passive data connection on address \"{}\" and port {}",
>> + address,
>> passivePort);
>> + servSoc = new ServerSocket(passivePort, 0,
>> address);
>> + LOG
>> + .debug(
>> + "Passive
>> data connection created on address \"{}\" and port {}",
>> + address,
>> passivePort);
>> + }
>> + port = servSoc.getLocalPort();
>> + servSoc.setSoTimeout(dataCfg.getIdleTime() * 1000);
>>
>> - // this method does not actually create the SSL
>> socket, due to a JVM bug
>> - // (https://issues.apache.org/jira/browse/FTPSERVER-241).
>> - // Instead, it creates a regular
>> - // ServerSocket that will be wrapped as a SSL socket
>> in createDataSocket()
>> - servSoc = new ServerSocket(passivePort, 0, address);
>> - LOG
>> - .debug(
>> - "SSL Passive data connection created
>> on address \"{}\" and port {}",
>> - address, passivePort);
>> - } else {
>> - LOG
>> - .debug(
>> - "Opening passive data connection on
>> address \"{}\" and port {}",
>> - address, passivePort);
>> - servSoc = new ServerSocket(passivePort, 0, address);
>> - LOG
>> - .debug(
>> - "Passive data connection created on
>> address \"{}\" and port {}",
>> - address, passivePort);
>> - }
>> - port = servSoc.getLocalPort();
>> - servSoc.setSoTimeout(dataCfg.getIdleTime() * 1000);
>> + // set different state variables
>> + passive = true;
>> + requestTime = System.currentTimeMillis();
>>
>> - // set different state variables
>> - passive = true;
>> - requestTime = System.currentTimeMillis();
>> + return new InetSocketAddress(address, port);
>> + } catch (Exception ex) {
>> + servSoc = null;
>> + closeDataConnection();
>> + throw new DataConnectionException(
>> + "Failed to initate passive data
>> connection: "
>> + + ex.getMessage(),
>> ex);
>> + }
>> + }
>>
>> - return new InetSocketAddress(address, port);
>> - } catch (Exception ex) {
>> - servSoc = null;
>> - closeDataConnection();
>> - throw new DataConnectionException(
>> - "Failed to initate passive data connection: "
>> - + ex.getMessage(), ex);
>> - }
>> - }
>> + /*
>> + * (non-Javadoc)
>> + *
>> + * @see
>> org.apache.ftpserver.FtpDataConnectionFactory2#getInetAddress()
>> + */
>> + public InetAddress getInetAddress() {
>> + return address;
>> + }
>>
>> - /*
>> - * (non-Javadoc)
>> - *
>> - * @see org.apache.ftpserver.FtpDataConnectionFactory2#getInetAddress()
>> - */
>> - public InetAddress getInetAddress() {
>> - return address;
>> - }
>> + /*
>> + * (non-Javadoc)
>> + *
>> + * @see org.apache.ftpserver.FtpDataConnectionFactory2#getPort()
>> + */
>> + public int getPort() {
>> + return port;
>> + }
>>
>> - /*
>> - * (non-Javadoc)
>> - *
>> - * @see org.apache.ftpserver.FtpDataConnectionFactory2#getPort()
>> - */
>> - public int getPort() {
>> - return port;
>> - }
>> + /*
>> + * (non-Javadoc)
>> + *
>> + * @see
>> org.apache.ftpserver.FtpDataConnectionFactory2#openConnection()
>> + */
>> + public DataConnection openConnection() throws Exception {
>> + return new IODataConnection(createDataSocket(), session,
>> this);
>> + }
>>
>> - /*
>> - * (non-Javadoc)
>> - *
>> - * @see org.apache.ftpserver.FtpDataConnectionFactory2#openConnection()
>> - */
>> - public DataConnection openConnection() throws Exception {
>> - return new IODataConnection(createDataSocket(), session, this);
>> - }
>> + /**
>> + * Get the data socket. In case of error returns null.
>> + */
>> + private synchronized Socket createDataSocket() throws Exception {
>>
>> - /**
>> - * Get the data socket. In case of error returns null.
>> - */
>> - private synchronized Socket createDataSocket() throws Exception {
>> + // get socket depending on the selection
>> + dataSoc = null;
>> + DataConnectionConfiguration dataConfig =
>> session.getListener()
>> + .getDataConnectionConfiguration();
>> + try {
>> + if (!passive) {
>> + if (secure) {
>> + LOG.debug("Opening secure active
>> data connection");
>> + SslConfiguration ssl =
>> getSslConfiguration();
>> + if (ssl == null) {
>> + throw new FtpException(
>> + "Data
>> connection SSL not configured");
>> + }
>>
>> - // get socket depending on the selection
>> - dataSoc = null;
>> - DataConnectionConfiguration dataConfig = session.getListener()
>> - .getDataConnectionConfiguration();
>> - try {
>> - if (!passive) {
>> - if (secure) {
>> - LOG.debug("Opening secure active data connection");
>> - SslConfiguration ssl = getSslConfiguration();
>> - if (ssl == null) {
>> - throw new FtpException(
>> - "Data connection SSL not configured");
>> - }
>> + // get socket factory
>> + SSLSocketFactory socFactory =
>> ssl.getSocketFactory();
>>
>> - // get socket factory
>> - SSLSocketFactory socFactory = ssl.getSocketFactory();
>> + // create socket
>> + SSLSocket ssoc = (SSLSocket)
>> socFactory.createSocket();
>> + ssoc.setUseClientMode(false);
>>
>> - // create socket
>> - SSLSocket ssoc = (SSLSocket) socFactory.createSocket();
>> - ssoc.setUseClientMode(false);
>> + // initialize socket
>> + if (ssl.getEnabledCipherSuites() !=
>> null) {
>> +
>> ssoc.setEnabledCipherSuites(ssl
>> +
>> .getEnabledCipherSuites());
>> + }
>> + dataSoc = ssoc;
>> + } else {
>> + LOG.debug("Opening active data
>> connection");
>> + dataSoc = new Socket();
>> + }
>>
>> - // initialize socket
>> - if (ssl.getEnabledCipherSuites() != null) {
>> -
>> ssoc.setEnabledCipherSuites(ssl.getEnabledCipherSuites());
>> - }
>> - dataSoc = ssoc;
>> - } else {
>> - LOG.debug("Opening active data connection");
>> - dataSoc = new Socket();
>> - }
>> + dataSoc.setReuseAddress(true);
>>
>> - dataSoc.setReuseAddress(true);
>> + InetAddress localAddr =
>> resolveAddress(dataConfig
>> + .getActiveLocalAddress());
>>
>> - InetAddress localAddr = resolveAddress(dataConfig
>> - .getActiveLocalAddress());
>> + // if no local address has been configured,
>> make sure we use the
>> + // same as the client connects from
>> + if (localAddr == null) {
>> + localAddr = ((InetSocketAddress)
>> session.getLocalAddress())
>> + .getAddress();
>> + }
>>
>> - // if no local address has been configured, make sure
>> we use the same as the client connects from
>> - if(localAddr == null) {
>> - localAddr =
>> ((InetSocketAddress)session.getLocalAddress()).getAddress();
>> - }
>> + SocketAddress localSocketAddress = new
>> InetSocketAddress(
>> + localAddr,
>> dataConfig.getActiveLocalPort());
>>
>> - SocketAddress localSocketAddress = new
>> InetSocketAddress(localAddr, dataConfig.getActiveLocalPort());
>> -
>> - LOG.debug("Binding active data connection to {}",
>> localSocketAddress);
>> - dataSoc.bind(localSocketAddress);
>> + LOG.debug("Binding active data connection to
>> {}",
>> + localSocketAddress);
>> + dataSoc.bind(localSocketAddress);
>>
>> - dataSoc.connect(new InetSocketAddress(address, port));
>> - } else {
>> + dataSoc.connect(new
>> InetSocketAddress(address, port));
>> + } else {
>>
>> - if (secure) {
>> - LOG.debug("Opening secure passive data connection");
>> - // this is where we wrap the unsecured socket as
>> a SSLSocket. This is
>> - // due to the JVM bug described in FTPSERVER-241.
>> + if (secure) {
>> + LOG.debug("Opening secure passive
>> data connection");
>> + // this is where we wrap the
>> unsecured socket as a
>> + // SSLSocket. This is
>> + // due to the JVM bug described in
>> FTPSERVER-241.
>>
>> - // get server socket factory
>> - SslConfiguration ssl = getSslConfiguration();
>> -
>> - // we've already checked this, but let's do it again
>> - if (ssl == null) {
>> - throw new FtpException(
>> - "Data connection SSL not configured");
>> - }
>> + // get server socket factory
>> + SslConfiguration ssl =
>> getSslConfiguration();
>>
>> - SSLSocketFactory ssocketFactory =
>> ssl.getSocketFactory();
>> + // we've already checked this, but
>> let's do it again
>> + if (ssl == null) {
>> + throw new FtpException(
>> + "Data
>> connection SSL not configured");
>> + }
>>
>> - Socket serverSocket = servSoc.accept();
>> + SSLSocketFactory ssocketFactory =
>> ssl.getSocketFactory();
>>
>> - SSLSocket sslSocket = (SSLSocket) ssocketFactory
>> - .createSocket(serverSocket, serverSocket
>> - .getInetAddress().getHostAddress(),
>> - serverSocket.getPort(), true);
>> - sslSocket.setUseClientMode(false);
>> + Socket serverSocket =
>> servSoc.accept();
>>
>> - // initialize server socket
>> - if (ssl.getClientAuth() == ClientAuth.NEED) {
>> - sslSocket.setNeedClientAuth(true);
>> - } else if (ssl.getClientAuth() == ClientAuth.WANT) {
>> - sslSocket.setWantClientAuth(true);
>> - }
>> + SSLSocket sslSocket = (SSLSocket)
>> ssocketFactory
>> +
>> .createSocket(serverSocket, serverSocket
>> +
>> .getInetAddress().getHostAddress(),
>> +
>> serverSocket.getPort(), true);
>> + sslSocket.setUseClientMode(false);
>>
>> - if (ssl.getEnabledCipherSuites() != null) {
>> - sslSocket.setEnabledCipherSuites(ssl
>> - .getEnabledCipherSuites());
>> - }
>> + // initialize server socket
>> + if (ssl.getClientAuth() ==
>> ClientAuth.NEED) {
>> +
>> sslSocket.setNeedClientAuth(true);
>> + } else if (ssl.getClientAuth() ==
>> ClientAuth.WANT) {
>> +
>> sslSocket.setWantClientAuth(true);
>> + }
>>
>> - dataSoc = sslSocket;
>> - } else {
>> - LOG.debug("Opening passive data connection");
>> + if (ssl.getEnabledCipherSuites() !=
>> null) {
>> +
>> sslSocket.setEnabledCipherSuites(ssl
>> +
>> .getEnabledCipherSuites());
>> + }
>>
>> - dataSoc = servSoc.accept();
>> - }
>> -
>> - if (dataConfig.isPassiveIpCheck()) {
>> + dataSoc = sslSocket;
>> + } else {
>> + LOG.debug("Opening passive data
>> connection");
>> +
>> + dataSoc = servSoc.accept();
>> + }
>> +
>> + if (dataConfig.isPassiveIpCheck()) {
>> // Let's make sure we got the
>> connection from the same
>> // client that we are expecting
>> - InetAddress remoteAddress =
>> ((InetSocketAddress)
>> session.getRemoteAddress()).getAddress();
>> + InetAddress remoteAddress =
>> ((InetSocketAddress) session
>> +
>> .getRemoteAddress()).getAddress();
>> InetAddress dataSocketAddress =
>> dataSoc.getInetAddress();
>> if
>> (!dataSocketAddress.equals(remoteAddress)) {
>> - LOG.warn("Passive IP Check
>> failed. Closing data connection from "
>> - + dataSocketAddress
>> - + " as it does not
>> match the expected address "
>> - + remoteAddress);
>> + LOG
>> +
>> .warn("Passive IP Check failed. Closing data connection from "
>> +
>> + dataSocketAddress
>> +
>> + " as it does not match the expected address "
>> +
>> + remoteAddress);
>> closeDataConnection();
>> return null;
>> }
>> }
>> -
>> - DataConnectionConfiguration dataCfg = session.getListener()
>> - .getDataConnectionConfiguration();
>> -
>> - dataSoc.setSoTimeout(dataCfg.getIdleTime() * 1000);
>> - LOG.debug("Passive data connection opened");
>> - }
>> - } catch (Exception ex) {
>> - closeDataConnection();
>> - LOG.warn("FtpDataConnection.getDataSocket()", ex);
>> - throw ex;
>> - }
>> - dataSoc.setSoTimeout(dataConfig.getIdleTime() * 1000);
>>
>> - // Make sure we initiate the SSL handshake, or we'll
>> - // get an error if we turn out not to send any data
>> - // e.g. during the listing of an empty directory
>> - if (dataSoc instanceof SSLSocket) {
>> - ((SSLSocket) dataSoc).startHandshake();
>> - }
>> + DataConnectionConfiguration dataCfg =
>> session.getListener()
>> +
>> .getDataConnectionConfiguration();
>>
>> - return dataSoc;
>> - }
>> + dataSoc.setSoTimeout(dataCfg.getIdleTime() *
>> 1000);
>> + LOG.debug("Passive data connection opened");
>> + }
>> + } catch (Exception ex) {
>> + closeDataConnection();
>> + LOG.warn("FtpDataConnection.getDataSocket()", ex);
>> + throw ex;
>> + }
>> + dataSoc.setSoTimeout(dataConfig.getIdleTime() * 1000);
>>
>> - /*
>> - * (non-Javadoc)
>> - * Returns an InetAddress object from a hostname or IP address.
>> - */
>> - private InetAddress resolveAddress(String host)
>> - throws DataConnectionException {
>> - if (host == null) {
>> - return null;
>> - } else {
>> - try {
>> - return InetAddress.getByName(host);
>> - } catch (UnknownHostException ex) {
>> - throw new DataConnectionException("Failed to resolve
>> address", ex);
>> - }
>> - }
>> - }
>> + // Make sure we initiate the SSL handshake, or we'll
>> + // get an error if we turn out not to send any data
>> + // e.g. during the listing of an empty directory
>> + if (dataSoc instanceof SSLSocket) {
>> + ((SSLSocket) dataSoc).startHandshake();
>> + }
>>
>> - /*
>> - * (non-Javadoc)
>> - *
>> - * @see org.apache.ftpserver.DataConnectionFactory#isSecure()
>> - */
>> - public boolean isSecure() {
>> - return secure;
>> - }
>> + return dataSoc;
>> + }
>>
>> - /**
>> - * Set the security protocol.
>> - */
>> - public void setSecure(final boolean secure) {
>> - this.secure = secure;
>> - }
>> + /*
>> + * (non-Javadoc) Returns an InetAddress object from a hostname or IP
>> + * address.
>> + */
>> + private InetAddress resolveAddress(String host)
>> + throws DataConnectionException {
>> + if (host == null) {
>> + return null;
>> + } else {
>> + try {
>> + return InetAddress.getByName(host);
>> + } catch (UnknownHostException ex) {
>> + throw new DataConnectionException("Failed to
>> resolve address",
>> + ex);
>> + }
>> + }
>> + }
>>
>> - /*
>> - * (non-Javadoc)
>> - *
>> - * @see org.apache.ftpserver.DataConnectionFactory#isZipMode()
>> - */
>> - public boolean isZipMode() {
>> - return isZip;
>> - }
>> + /*
>> + * (non-Javadoc)
>> + *
>> + * @see org.apache.ftpserver.DataConnectionFactory#isSecure()
>> + */
>> + public boolean isSecure() {
>> + return secure;
>> + }
>>
>> - /**
>> - * Set zip mode.
>> - */
>> - public void setZipMode(final boolean zip) {
>> - isZip = zip;
>> - }
>> + /**
>> + * Set the security protocol.
>> + */
>> + public void setSecure(final boolean secure) {
>> + this.secure = secure;
>> + }
>>
>> - /**
>> - * Check the data connection idle status.
>> - */
>> - public synchronized boolean isTimeout(final long currTime) {
>> + /*
>> + * (non-Javadoc)
>> + *
>> + * @see org.apache.ftpserver.DataConnectionFactory#isZipMode()
>> + */
>> + public boolean isZipMode() {
>> + return isZip;
>> + }
>>
>> - // data connection not requested - not a timeout
>> - if (requestTime == 0L) {
>> - return false;
>> - }
>> + /**
>> + * Set zip mode.
>> + */
>> + public void setZipMode(final boolean zip) {
>> + isZip = zip;
>> + }
>>
>> - // data connection active - not a timeout
>> - if (dataSoc != null) {
>> - return false;
>> - }
>> + /**
>> + * Check the data connection idle status.
>> + */
>> + public synchronized boolean isTimeout(final long currTime) {
>>
>> - // no idle time limit - not a timeout
>> - int maxIdleTime = session.getListener()
>> - .getDataConnectionConfiguration().getIdleTime() * 1000;
>> - if (maxIdleTime == 0) {
>> - return false;
>> - }
>> + // data connection not requested - not a timeout
>> + if (requestTime == 0L) {
>> + return false;
>> + }
>>
>> - // idle time is within limit - not a timeout
>> - if ((currTime - requestTime) < maxIdleTime) {
>> - return false;
>> - }
>> + // data connection active - not a timeout
>> + if (dataSoc != null) {
>> + return false;
>> + }
>>
>> - return true;
>> - }
>> + // no idle time limit - not a timeout
>> + int maxIdleTime = session.getListener()
>> +
>> .getDataConnectionConfiguration().getIdleTime() * 1000;
>> + if (maxIdleTime == 0) {
>> + return false;
>> + }
>>
>> - /**
>> - * Dispose data connection - close all the sockets.
>> - */
>> - public void dispose() {
>> - closeDataConnection();
>> - }
>> + // idle time is within limit - not a timeout
>> + if ((currTime - requestTime) < maxIdleTime) {
>> + return false;
>> + }
>>
>> - /**
>> - * Sets the server's control address.
>> - */
>> - public void setServerControlAddress(final InetAddress
>> serverControlAddress) {
>> - this.serverControlAddress = serverControlAddress;
>> - }
>> + return true;
>> + }
>> +
>> + /**
>> + * Dispose data connection - close all the sockets.
>> + */
>> + public void dispose() {
>> + closeDataConnection();
>> + }
>> +
>> + /**
>> + * Sets the server's control address.
>> + */
>> + public void setServerControlAddress(final InetAddress
>> serverControlAddress) {
>> + this.serverControlAddress = serverControlAddress;
>> + }
>> }
>>
>>
>>
>>
>>
>> 2010/3/25 Sai Pullabhotla <sai.pullabho...@jmethods.com>:
>>> Just so we are all on the same page -
>>>
>>> Do you think the issue is with the LIST command, or the code that
>>> creates passive data connection? Could you briefly explain some of the
>>> issues that made you think it should be rewritten, so everyone gets a
>>> chance to evaluate?
>>>
>>> Thanks.
>>>
>>> Regards,
>>> Sai Pullabhotla
>>>
>>>
>>>
>>>
>>>
>>> On Thu, Mar 25, 2010 at 5:56 AM, David Latorre <dvl...@gmail.com> wrote:
>>>> 2010/3/24 Niklas Gustavsson <nik...@protocol7.com>:
>>>>> On Wed, Mar 24, 2010 at 6:03 PM, Fred Moore <fred.moor...@gmail.com>
>>>>> wrote:
>>>>>> we found an issue related to requestPassivePort() which may lead to an
>>>>>> unresponsive V1.0.4 FTP (or FTP/S) Server, this issue can be reproduced.
>>>>>>
>>>>>> http://issues.apache.org/jira/browse/FTPSERVER-359 contains full
>>>>>> description
>>>>>> of the symptoms and a minimalist java client and server to reproduce it.
>>>>>
>>>>> I haven't yet looked closer at the code you attached. But, I have seen
>>>>> similar behavior myself during performance testing FtpServer. In that
>>>>> case, I had a very similar behavior at around 20 threads. However, the
>>>>> reason for the problem in that test was that the test case uses up
>>>>> file handles (for the sockets) so fast that they will run out. Since
>>>>> sockets hang around for some time also after closing, they were not
>>>>> freed quickly enough and thus FtpServer could not open new ones. Could
>>>>> you please verify that this is not the case here? You could look at
>>>>> netstat output and look into increasing the allowed number of file
>>>>> handles our the timeout time for sockets in your OS.
>>>>
>>>>
>>>> Actually it is quite easy to reproduce this error (I just wrote a
>>>> client test case with throws >20~30 threads that list a directory in
>>>> the server ) and it's not file handle related:
>>>> we have several bugs in our code that cause this behaviour , i
>>>> think we hould rewrite all the request/release passive port mechanism
>>>> as there are several issues with it.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>> /niklas
>>>>>
>>>>
>>>
>>
>
