Mark Proctor wrote:
The onLogin request.getUser() returns null. So I started looking at the
PASS class:
String userName = request.getUserArgument()
User user = request.getUser()
So here you are requesting user, which is always null, but even if it
did return a value you never use it anyway as user is later nulled as
it's actually set by the authentication code. Further to this you call
the onLogin method before you do the request.setUser(user), so I guess
I'm asking should User be available from onLogin or not?
As it works right now, the Ftplet is given the possibility of denying
the login. Maybe we should change this so that we first set the status
as it would be logged in, and then if the Ftplet returns RET_SKIP we
reset the status so that the user is now set as denied access. I would
be happy to make this change if you think it makes sense.
If not, this
should probably be documented. Either way the fist user assigment and
null check is redundant. I also notied that you are swollowing
Exceptions thrown by the FtpLets, even null pointers, these should be
atleast logged, as it makes tracking down bugs that are occuring in them
difficult.
By this you mean for example:
try{
ftpletRet = ftpletContainer.onLogin(request, out);
} catch(Exception e) {
ftpletRet = FtpletEnum.RET_DISCONNECT;
}
Adding a log there would make sense, possibly at DEBUG level?
/niklas
