Please note that the patch is not even worh considering...for instance, i am trying to close the port without any checks that we are using passive mode!
As Sai suggests, if that's ok for you I would simply return an error code if there are no free available ports in 1.0.x. But, since some users (Fred) are expecting that PASV requests get enqueued I don't know if we should change FTPServer behaviour at this moment. 2010/3/25 Sai Pullabhotla <[email protected]>: > 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 <[email protected]> > 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 <[email protected]> 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 <[email protected]>: >>>> 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 <[email protected]> wrote: >>>>> 2010/3/24 Niklas Gustavsson <[email protected]>: >>>>>> On Wed, Mar 24, 2010 at 6:03 PM, Fred Moore <[email protected]> >>>>>> 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 >>>>>> >>>>> >>>> >>> >> >
