You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by ng...@apache.org on 2009/05/13 22:16:06 UTC
svn commit: r774507 - in /mina/ftpserver/branches/1.0.x/core/src:
main/java/org/apache/ftpserver/command/impl/
main/java/org/apache/ftpserver/impl/
test/java/org/apache/ftpserver/ftpletcontainer/
Author: ngn
Date: Wed May 13 20:16:05 2009
New Revision: 774507
URL: http://svn.apache.org/viewvc?rev=774507&view=rev
Log:
Fix bug where the an open data connection will not be closed on the filter chain exception (FTPSERVER-297)
Fix bug where server will not close the session if an Ftplet returns DISCONNECT in the onConnect callback (FTPSERVER-298)
Adding logging in all places where we close sessions.
Added:
mina/ftpserver/branches/1.0.x/core/src/test/java/org/apache/ftpserver/ftpletcontainer/FtpLetOnConnectTest.java
Modified:
mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/PASS.java
mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/QUIT.java
mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/USER.java
mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/impl/DefaultFtpHandler.java
mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java
Modified: mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/PASS.java
URL: http://svn.apache.org/viewvc/mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/PASS.java?rev=774507&r1=774506&r2=774507&view=diff
==============================================================================
--- mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/PASS.java (original)
+++ mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/PASS.java Wed May 13 20:16:05 2009
@@ -103,7 +103,14 @@
int currAnonLogin = stat.getCurrentAnonymousLoginNumber();
int maxAnonLogin = context.getConnectionConfig()
.getMaxAnonymousLogins();
+ if(maxAnonLogin == 0) {
+ LOG.debug("Currently {} anonymous users logged in, unlimited allowed", currAnonLogin);
+ } else {
+ LOG.debug("Currently {} out of {} anonymous users logged in", currAnonLogin, maxAnonLogin);
+ }
+
if (currAnonLogin >= maxAnonLogin) {
+ LOG.debug("Too many anonymous users logged in, user will be disconnected");
session
.write(LocalizedFtpReply
.translate(
@@ -119,7 +126,13 @@
// login limit check
int currLogin = stat.getCurrentLoginNumber();
int maxLogin = context.getConnectionConfig().getMaxLogins();
+ if(maxLogin == 0) {
+ LOG.debug("Currently {} users logged in, unlimited allowed", currLogin);
+ } else {
+ LOG.debug("Currently {} out of {} users logged in", currLogin, maxLogin);
+ }
if (maxLogin != 0 && currLogin >= maxLogin) {
+ LOG.debug("Too many users logged in, user will be disconnected");
session
.write(LocalizedFtpReply
.translate(
@@ -197,6 +210,8 @@
.getMaxLoginFailures();
if (maxAllowedLoginFailues != 0
&& session.getFailedLogins() >= maxAllowedLoginFailues) {
+ LOG.warn("User exceeded the number of allowed failed logins, session will be closed");
+
session.close(false).awaitUninterruptibly(10000);
}
Modified: mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/QUIT.java
URL: http://svn.apache.org/viewvc/mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/QUIT.java?rev=774507&r1=774506&r2=774507&view=diff
==============================================================================
--- mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/QUIT.java (original)
+++ mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/QUIT.java Wed May 13 20:16:05 2009
@@ -27,6 +27,8 @@
import org.apache.ftpserver.impl.FtpIoSession;
import org.apache.ftpserver.impl.FtpServerContext;
import org.apache.ftpserver.impl.LocalizedFtpReply;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* <strong>Internal class, do not use directly.</strong>
@@ -41,6 +43,8 @@
*/
public class QUIT extends AbstractCommand {
+ private final Logger LOG = LoggerFactory.getLogger(QUIT.class);
+
/**
* Execute command
*/
@@ -51,6 +55,7 @@
session.write(LocalizedFtpReply.translate(session, request, context,
FtpReply.REPLY_221_CLOSING_CONTROL_CONNECTION, "QUIT", null));
+ LOG.debug("QUIT received, closing session");
session.close(false).awaitUninterruptibly(10000);
session.getDataConnection().closeDataConnection();
}
Modified: mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/USER.java
URL: http://svn.apache.org/viewvc/mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/USER.java?rev=774507&r1=774506&r2=774507&view=diff
==============================================================================
--- mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/USER.java (original)
+++ mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/command/impl/USER.java Wed May 13 20:16:05 2009
@@ -34,6 +34,8 @@
import org.apache.ftpserver.impl.ServerFtpStatistics;
import org.apache.ftpserver.usermanager.impl.ConcurrentLoginRequest;
import org.apache.mina.filter.logging.MdcInjectionFilter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* <strong>Internal class, do not use directly.</strong>
@@ -50,6 +52,8 @@
*/
public class USER extends AbstractCommand {
+ private final Logger LOG = LoggerFactory.getLogger(USER.class);
+
/**
* Execute command.
*/
@@ -112,7 +116,14 @@
int currAnonLogin = stat.getCurrentAnonymousLoginNumber();
int maxAnonLogin = context.getConnectionConfig()
.getMaxAnonymousLogins();
+ if(maxAnonLogin == 0) {
+ LOG.debug("Currently {} anonymous users logged in, unlimited allowed", currAnonLogin);
+ } else {
+ LOG.debug("Currently {} out of {} anonymous users logged in", currAnonLogin, maxAnonLogin);
+ }
if (anonymous && (currAnonLogin >= maxAnonLogin)) {
+ LOG.debug("Too many anonymous users logged in, user will be disconnected");
+
session
.write(LocalizedFtpReply
.translate(
@@ -127,7 +138,16 @@
// login limit check
int currLogin = stat.getCurrentLoginNumber();
int maxLogin = context.getConnectionConfig().getMaxLogins();
+
+ if(maxLogin == 0) {
+ LOG.debug("Currently {} users logged in, unlimited allowed", currLogin);
+ } else {
+ LOG.debug("Currently {} out of {} users logged in", currLogin, maxLogin);
+ }
+
if (maxLogin != 0 && currLogin >= maxLogin) {
+ LOG.debug("Too many users logged in, user will be disconnected");
+
session
.write(LocalizedFtpReply
.translate(
@@ -154,6 +174,7 @@
stat.getCurrentUserLoginNumber(configUser, address) + 1);
if (configUser.authorize(loginRequest) == null) {
+ LOG.debug("User logged in too many sessions, user will be disconnected");
session
.write(LocalizedFtpReply
.translate(
@@ -182,6 +203,7 @@
// if not ok - close connection
if (!success) {
+ LOG.debug("User failed to login, session will be closed");
session.close(false).awaitUninterruptibly(10000);
}
}
Modified: mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/impl/DefaultFtpHandler.java
URL: http://svn.apache.org/viewvc/mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/impl/DefaultFtpHandler.java?rev=774507&r1=774506&r2=774507&view=diff
==============================================================================
--- mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/impl/DefaultFtpHandler.java (original)
+++ mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/impl/DefaultFtpHandler.java Wed May 13 20:16:05 2009
@@ -23,6 +23,8 @@
import org.apache.ftpserver.command.Command;
import org.apache.ftpserver.command.CommandFactory;
+import org.apache.ftpserver.ftplet.DataConnection;
+import org.apache.ftpserver.ftplet.DataConnectionFactory;
import org.apache.ftpserver.ftplet.FileSystemView;
import org.apache.ftpserver.ftplet.FtpReply;
import org.apache.ftpserver.ftplet.FtpRequest;
@@ -69,15 +71,28 @@
}
public void sessionOpened(final FtpIoSession session) throws Exception {
- context.getFtpletContainer().onConnect(session.getFtpletSession());
+ FtpletContainer ftplets = context.getFtpletContainer();
- session.updateLastAccessTime();
-
- session.write(LocalizedFtpReply.translate(session, null, context,
- FtpReply.REPLY_220_SERVICE_READY, null, null));
+ FtpletResult ftpletRet;
+ try {
+ ftpletRet = ftplets.onConnect(session.getFtpletSession());
+ } catch (Exception e) {
+ LOG.debug("Ftplet threw exception", e);
+ ftpletRet = FtpletResult.DISCONNECT;
+ }
+ if (ftpletRet == FtpletResult.DISCONNECT) {
+ LOG.debug("Ftplet returned DISCONNECT, session will be closed");
+ session.close(false).awaitUninterruptibly(10000);
+ } else {
+ session.updateLastAccessTime();
+
+ session.write(LocalizedFtpReply.translate(session, null, context,
+ FtpReply.REPLY_220_SERVICE_READY, null, null));
+ }
}
public void sessionClosed(final FtpIoSession session) throws Exception {
+ LOG.debug("Closing session");
try {
context.getFtpletContainer().onDisconnect(
session.getFtpletSession());
@@ -85,6 +100,17 @@
// swallow the exception, we're closing down the session anyways
LOG.warn("Ftplet threw an exception on disconnect", e);
}
+
+ // make sure we close the data connection if it happens to be open
+ try {
+ ServerDataConnectionFactory dc = session.getDataConnection();
+ if(dc != null) {
+ dc.closeDataConnection();
+ }
+ } catch (Exception e) {
+ // swallow the exception, we're closing down the session anyways
+ LOG.warn("Data connection threw an exception on disconnect", e);
+ }
FileSystemView fs = session.getFileSystemView();
if(fs != null) {
@@ -101,7 +127,11 @@
if (stats != null) {
stats.setLogout(session);
stats.setCloseConnection(session);
+ LOG.debug("Statistics login and connection count decreased due to session close");
+ } else {
+ LOG.warn("Statistics not available in session, can not decrease login and connection count");
}
+ LOG.debug("Session closed");
}
public void exceptionCaught(final FtpIoSession session,
@@ -160,6 +190,7 @@
ftpletRet = FtpletResult.DISCONNECT;
}
if (ftpletRet == FtpletResult.DISCONNECT) {
+ LOG.debug("Ftplet returned DISCONNECT, session will be closed");
session.close(false).awaitUninterruptibly(10000);
return;
} else if (ftpletRet != FtpletResult.SKIP) {
@@ -184,6 +215,8 @@
ftpletRet = FtpletResult.DISCONNECT;
}
if (ftpletRet == FtpletResult.DISCONNECT) {
+ LOG.debug("Ftplet returned DISCONNECT, session will be closed");
+
session.close(false).awaitUninterruptibly(10000);
return;
}
Modified: mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java
URL: http://svn.apache.org/viewvc/mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java?rev=774507&r1=774506&r2=774507&view=diff
==============================================================================
--- mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java (original)
+++ mina/ftpserver/branches/1.0.x/core/src/main/java/org/apache/ftpserver/impl/IODataConnectionFactory.java Wed May 13 20:16:05 2009
@@ -87,6 +87,7 @@
/**
* Close data socket.
+ * This method must be idempotent as we might call it multiple times during disconnect.
*/
public synchronized void closeDataConnection() {
@@ -108,9 +109,7 @@
LOG.warn("FtpDataConnection.closeDataSocket()", ex);
}
- FtpServerContext ctx = serverContext;
-
- if (ctx != null) {
+ if (session != null) {
DataConnectionConfiguration dcc = session.getListener()
.getDataConnectionConfiguration();
if (dcc != null) {
Added: mina/ftpserver/branches/1.0.x/core/src/test/java/org/apache/ftpserver/ftpletcontainer/FtpLetOnConnectTest.java
URL: http://svn.apache.org/viewvc/mina/ftpserver/branches/1.0.x/core/src/test/java/org/apache/ftpserver/ftpletcontainer/FtpLetOnConnectTest.java?rev=774507&view=auto
==============================================================================
--- mina/ftpserver/branches/1.0.x/core/src/test/java/org/apache/ftpserver/ftpletcontainer/FtpLetOnConnectTest.java (added)
+++ mina/ftpserver/branches/1.0.x/core/src/test/java/org/apache/ftpserver/ftpletcontainer/FtpLetOnConnectTest.java Wed May 13 20:16:05 2009
@@ -0,0 +1,122 @@
+/*
+ * 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.ftpletcontainer;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.net.SocketException;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import org.apache.commons.net.ftp.FTPConnectionClosedException;
+import org.apache.ftpserver.FtpServerFactory;
+import org.apache.ftpserver.clienttests.ClientTestTemplate;
+import org.apache.ftpserver.ftplet.FtpException;
+import org.apache.ftpserver.ftplet.FtpRequest;
+import org.apache.ftpserver.ftplet.FtpSession;
+import org.apache.ftpserver.ftplet.Ftplet;
+import org.apache.ftpserver.ftplet.FtpletResult;
+import org.apache.ftpserver.impl.DefaultFtpServer;
+import org.apache.ftpserver.test.TestUtil;
+
+/**
+*
+* @author The Apache MINA Project (dev@mina.apache.org)
+* @version $Rev$, $Date$
+*
+*/
+public class FtpLetOnConnectTest extends ClientTestTemplate {
+ private static final byte[] TESTDATA = "TESTDATA".getBytes();
+
+ private static final byte[] DOUBLE_TESTDATA = "TESTDATATESTDATA".getBytes();
+
+ private static final File TEST_FILE1 = new File(ROOT_DIR, "test1.txt");
+
+ private static final File TEST_FILE2 = new File(ROOT_DIR, "test2.txt");
+
+ private static final File TEST_DIR1 = new File(ROOT_DIR, "dir1");
+
+ protected FtpletResult mockReturnValue = FtpletResult.DISCONNECT;
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see org.apache.ftpserver.clienttests.ClientTestTemplate#setUp()
+ */
+ protected void setUp() throws Exception {
+ MockFtplet.callback = new MockFtpletCallback();
+
+ initDirs();
+
+ initServer();
+ }
+
+ protected FtpServerFactory createServer() throws Exception {
+ FtpServerFactory server = super.createServer();
+
+ Map<String, Ftplet> ftplets = new LinkedHashMap<String, Ftplet>();
+ ftplets.put("f1", new MockFtplet());
+ server.setFtplets(ftplets);
+
+ return server;
+ }
+
+ public void testDisconnectOnConnect() throws Exception {
+ MockFtplet.callback = new MockFtpletCallback() {
+ public FtpletResult onConnect(FtpSession session)
+ throws FtpException, IOException {
+ return mockReturnValue;
+ }
+ };
+
+ try {
+ connectClient();
+ fail("Must throw FTPConnectionClosedException");
+ } catch (FTPConnectionClosedException e) {
+ // OK
+ } catch (SocketException e) {
+ // OK
+ }
+ }
+
+ public void testExceptionOnConnect() throws Exception {
+ MockFtplet.callback = new MockFtpletCallback() {
+ public FtpletResult onConnect(FtpSession session)
+ throws FtpException, IOException {
+ throw new FtpException();
+ }
+ };
+
+ try {
+ connectClient();
+ fail("Must throw FTPConnectionClosedException");
+ } catch (FTPConnectionClosedException e) {
+ // OK
+ } catch (SocketException e) {
+ // OK
+ }
+ }
+
+ protected void doConnect() throws Exception {
+ client.connect("localhost", getListenerPort());
+ }
+}