> [..]
> None of the commandhandlers throw exception except doAuth().
> Even the doData also handles the exception internally I was
> also planning to make onCommand throw exception. If so should
> it should the generic Exception. Should I throw the base
> class Exception?
You are right.
Probably we should only change the way doAuth work by catching specific
Exceptions and doing specific logging.
> > 2) Why do we need a "private final static String
> COMMAND_NAME" inside every
> > command handler? "Is it"/"Will it be" used? The mapping
> between command name
> > and command handler is done at configuration level so I
> don't understand the
> > meaning of that private string.
>
> When I refactored all the commandhandlers I renamed the command name
> constants to COMMAND_NAME. Initially I was planning to use the
> COMMAND_NAME for getting the right commandhandler. But I moved it to
> XML configuration. So as of now it doesnot have any significance. I
> will remove it
> wherever its not used.
Ok!
> > 3) Why did you choose to create a private inner class
> SMTPSessionImpl
> > implementing the SMTPSession instead of make the
> SMTPHandler to implement
> > the interface itself?
>
> The reason for that is that I dont want the 3rd party command handler
> writers to cast SMTPSession to SMTPHandler and access the
> functionality. If you think it makes the design inefficient i will do
> as you advised
My personal opinion about this security issue is that handler writers are
developers. They got an interface and should use it. If they cast to the
real class we can't grant them it will work but I don't see any problem in
doing that.
IMHO the change will clean up the code and I think it's better to have
cleaned code.
> > 4) Why did you choose to create the following states?:
Ok, understood.
> > 5) I've not tested it but I'm not sure you handle the RSET
> correctly. I
> > don't see anywhere the assignment of mode = COMMAND_MODE
> when you reset the
> > session (after an RSET).
>
> At the beginning its reset back to command mode
> while(!sessionEnded) {
> //Reset the current command values
> curCommandName = null;
> curCommandArgument = null;
> mode = COMMAND_MODE; <--------------
>
> I have tested RSET it works fine.
Ops, I missed it :-)
Perfect!
I'm not sure but maybe most of the SMTPSession hard coded "states",
including the "mode" could be moved to the state hashmap?
(e.g: BlockListed, RelayingAllowed, AuthRequired, User)
One more thing:
Can we use the interface LogEnabled for the handlers and pass the logger to
the handlers as like as you currently do for the Configuration?
This allow us to remove the getLogger() from the SMTPSession. (IMHO
SMTPSession should be as simple as possible)
BTW, Good work!
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
