You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by Anagha Mudigonda <an...@gmail.com> on 2005/08/10 17:48:48 UTC
Status Update - James Fast Fail
I have checked out from the trunk and ported all the Commandhandlers.
There are calls mapping to old code so that latest code (changed since
i checked out) can be ported easily. By mappings what I mean is To
port 'state' variable, I added getSessionState() method to
SMTPSession.
This was not part of my original design. These will be deprecated in
the next version.
Right now I am testing thoroughly. I can provide this first version of the code
for review this weekend. How can I check in the code as I dont have priveleges?
Summary of changes made:
1. SMTPServer
-----------------------
configure method
(LogEnabled)cmdMap).enableLogging(getLogger());
cmdMap.load(handlerConfiguration.getChild("commandmap"));
newHandler() method
theHandler.setCommandHandlerMap(cmdMap);
2. *NEW* class CommandHandlerMap
---------------------------------------------------------
loads the commandHandlers from the config file.
CommandHandlerMap
public class CommandHandlerMap extends AbstractLogEnabled {
void load(Configuration configuration) throws ConfigurationException {
addToMap(unknownHandler.getCommandName(), unknownHandler);
if(configuration != null) {
Configuration[] children =
configuration.getChildren("commandhandler");
if ( children != null ) {
ClassLoader classLoader = getClass().getClassLoader();
for ( int i = 0 ; i < children.length ; i++ ) {
String commandName = children[i].getAttribute("name");
String className = children[i].getAttribute("class");
if(className != null) {
try {
CommandHandler cmdHandler =
(CommandHandler)classLoader.loadClass(className).newInstance();
//configure the command handler
cmdHandler.configure(children[i]);
addToMap(commandName, cmdHandler);
...
...
}}}
ArrayList getCommandHandlers(String command) {
...
...
return handlers;
}
}
SMTPHandler
-------------------
The SMTPHandler parses command and checks if there are any commandHandlers
handleConnection method
ArrayList handlers = cmdMap.getCommandHandlers(curCommandName);
int count = handlers.size();
for(int i = 0; i < count; i++) {
if(session.isSessionEnded()) {
break;
}
((CommandHandler)handlers.get(i)).processCommand(session);
}
The config file config.xml
------------------------------------
<handler>
..
..
<commandmap>
<commandhandler name="HELP"
class="org.apache.james.smtpserver.HelpCmdHandler"></commandhandler>
<commandhandler name="MAIL"
class="org.apache.james.smtpserver.MailCmdHandler"></commandhandler>
<commandhandler name="RCPT"
class="org.apache.james.smtpserver.RcptCmdHandler"></commandhandler>
<commandhandler name="EHLO"
class="org.apache.james.smtpserver.EhloCmdHandler"></commandhandler>
<commandhandler name="HELO"
class="org.apache.james.smtpserver.HeloCmdHandler"></commandhandler>
<commandhandler name="RSET"
class="org.apache.james.smtpserver.RsetCmdHandler"></commandhandler>
<commandhandler name="VRFY"
class="org.apache.james.smtpserver.VrfyCmdHandler"></commandhandler>
<commandhandler name="EXPN"
class="org.apache.james.smtpserver.ExpnCmdHandler"></commandhandler>
<commandhandler name="QUIT"
class="org.apache.james.smtpserver.QuitCmdHandler"></commandhandler>
<commandhandler name="AUTH"
class="org.apache.james.smtpserver.AuthCmdHandler"></commandhandler>
<commandhandler name="DATA"
class="org.apache.james.smtpserver.DataCmdHandler"></commandhandler>
</commandmap>
</handler>
SMTPSession
---------------------
The SMTPSession Interface looks like this
package org.apache.james.smtpserver;
import org.apache.james.core.MailHeaders;
import org.apache.james.core.MailImpl;
import java.net.Socket;
import org.apache.avalon.framework.logger.AbstractLogEnabled;
import java.util.HashMap;
import java.io.IOException;
import java.io.InputStream;
import org.apache.james.util.watchdog.Watchdog;
import org.apache.avalon.framework.logger.Logger;
public interface SMTPSession
{
//Write Response
void writeResponse(String respString);
//Register connectionhandlers to handle Message
void registerMessageHandler(ConnectHandler connHandler);
//Register commandhandlers to handle Message
void registerMessageHandler(CommandHandler cmdHandler);
//Command sequence handling
void addCommandSequenceMap(String command, String[]
predecessorCmds, String failMessage);
boolean isCommandAllowed(String cmdString);
//State information
String getSender();
String[] getRecepients();
String setSender(String sender);
String addRecepient(String recepient);
String addRecepient(String[] recepients);
String removeRecepient(String[] recepient);
String removeRecepient(String recepient);
String getCommandName();
String getCommandArgument();
MailImpl getMessage();
//whether we should return just the socket(??)
String getRemoteHost();
String getRemoteIPAddress();
//Message state modified
void startMessage();
void endMessage();
void abortMessage();
//Message state modifier
void endSession();
void startSession();
void abortSession();
boolean isSessionEnded();
//TO be deprecated
HashMap getState();
void resetState();
public String readCommandLine() throws IOException;
Logger getLogger();
StringBuffer getResponseBuffer();
String clearResponseBuffer();
SMTPHandlerConfigurationData getConfigurationData();
boolean isBlockListed();
boolean isRelayingAllowed();
boolean isAuthRequired();
String getUser();
void setUser(String user);
Watchdog getWatchdog();
InputStream getInputStream();
String getSessionID();
}
-- Anagha
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Anagha Mudigonda <an...@gmail.com>.
thanks stefano.
also can you give me a quick feedback on the changes i mentioned.
--anagha
On 8/10/05, Stefano Bagnara <ap...@bago.org> wrote:
> > Right now I am testing thoroughly. I can provide this first
> > version of the code for review this weekend. How can I check
> > in the code as I dont have priveleges?
>
> You should wait an "official" answer from Danny or Noel...
>
> IMHO, the best thing you can do is to create a patch against the current
> trunk (or whatever version you prefer) and submit the "patch" to JIRA giving
> permissions to include the code to ASF (it is a flag when you add an
> attachment in JIRA).
>
> This way we can test the code and decide wether to include it in the next
> 2.3 release or move the integration to the following release.
>
> Stefano
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: James Fast Fail - another idea
Posted by Anagha Mudigonda <an...@gmail.com>.
On 8/18/05, Stefano Bagnara <ap...@bago.org> wrote:
> Hi Anagha,
>
> I thought about a MatcherMessageHandler.
> This handler would take a standard james matcher configuration as an
> argument and an additional configuration could say how to reply based on the
> result of the matcher.
> E.g:
> <handler class="org.apache.james.smtpserver.MatcherMessageHandler">
> <!-- reject the message if the sender is in fake domain -->
> <rule match="SenderInFakeDomain=64.55.105.9,64.94.110.11"
> reject="matchany" />
> <!-- reject the message if match all the recipients: then only when the
> only recipient is trash@xxx.yyy -->
> <rule match="RecipientIs=trash@xxx.yyy" reject="matchall" />
> </handler>
>
> What do you think?
> Could it be of any use?
> Would it be better to add a full "processor" configuration as a
> messagehandler?
I think its a good idea to reuse mailet pieces of code for message handlers.
After I refine the core framework and do all the clean up, I will get
started with that.
I am planning to give next patch this weekend.
--anagha
>
> Stefano
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
James Fast Fail - another idea
Posted by Stefano Bagnara <ap...@bago.org>.
Hi Anagha,
I thought about a MatcherMessageHandler.
This handler would take a standard james matcher configuration as an
argument and an additional configuration could say how to reply based on the
result of the matcher.
E.g:
<handler class="org.apache.james.smtpserver.MatcherMessageHandler">
<!-- reject the message if the sender is in fake domain -->
<rule match="SenderInFakeDomain=64.55.105.9,64.94.110.11"
reject="matchany" />
<!-- reject the message if match all the recipients: then only when the
only recipient is trash@xxx.yyy -->
<rule match="RecipientIs=trash@xxx.yyy" reject="matchall" />
</handler>
What do you think?
Could it be of any use?
Would it be better to add a full "processor" configuration as a
messagehandler?
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Anagha Mudigonda <an...@gmail.com>.
On 8/17/05, Stefano Bagnara <ap...@bago.org> wrote:
> I think it's better to have this committed and changes between versions in
> svn than having only patches in JIRA so I merged and applied the patch. I
> tested the code and it seems to work: we could always revert later if
> needed.
>
> Anagha: can you add a check while loading the configuration that will avoid
> to start when there is no command map configured?
> It should also add a WARNING when basic commands are missing (I would say:
> QUIT, HELO, MAIL, RCPT, DATA).
done!
>
> > I too want to make SMTPSession as possible. Currently I have
> > various interfaces for making refactoring easy. they will be
> > removed. I dont like the getWatchDog() and various other
> > interfaces in SMTPSession.
> > any suggestions?
>
> No ideas now, sorry.
> Stefano
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Anagha Mudigonda <an...@gmail.com>.
On 8/17/05, Danny Angus <Da...@slc.co.uk> wrote:
>
> > I think it's better to have this committed and
> > changes between versions in svn
>
> +1 this is what we agreed with the students.
>
> Anagha:
> Now just submit incremental patches, you're doing a great job.
> Sorry I don't have time to pay closer attention!
> If you want people to test it, post to this list, but also consider posting
> to the users list.
thanks Danny. I am having great time working on this project:)
-- anagha
>
> Everyone:
> Does Gump publish "nightlies" for James?
>
>
> d.
>
>
> ***************************************************************************
> The information in this e-mail is confidential and for use by the addressee(s) only. If you are not the intended recipient (or responsible for delivery of the message to the intended recipient) please notify us immediately on 0141 306 2050 and delete the message from your computer. You may not copy or forward it or use or disclose its contents to any other person. As Internet communications are capable of data corruption Student Loans Company Limited does not accept any responsibility for changes made to this message after it was sent. For this reason it may be inappropriate to rely on advice or opinions contained in an e-mail without obtaining written confirmation of it. Neither Student Loans Company Limited or the sender accepts any liability or responsibility for viruses as it is your responsibility to scan attachments (if any). Opinions and views expressed in this e-mail are those of the sender and may not reflect the opinions and views of The Student Loans Company Limit
> ed.
>
> This footnote also confirms that this email message has been swept for the presence of computer viruses.
>
> **************************************************************************
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Ahmed Mohombe <am...@yahoo.com>.
> Noel,
> The reason I though of it was to let people get binaries with the SoC stuff
> in it.
> That might get us a bigger pool of testers than making people use svn
> would.
+ 1.
And it will avoid the situation where what users check out, does not
compile. (I suppose the nightly builds should compile at least :) ).
Ahmed.
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Nightly builds
Posted by Stefano Bagnara <ap...@bago.org>.
> I have configured it, so we'll see what happens tonight. The
> results will go under
> http://cvs.apache.org/builds/james/nightly/.
Tonight something didn't work, but now it seems working again.
Thank you!
We should also update the james download page that points to "Latest Test
Build" as http://cvs.apache.org/dist/james/
Stefano
PS: we should also add the mailet api mailing list in the
http://james.apache.org/mail.html page.
I would do this, but I'm more reluctant to change the public website (I have
more confidence with the sources repository :-) ).
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
RE: Nightly builds
Posted by "Noel J. Bergman" <no...@devtech.com>.
Danny Angus wrote:
> Noel J Bergman wrote:
> > I can look to setup a cron task on one of my systems
> > at home to post unsigned nightly builds.
> That would be exceptionally kind, and much appreciated. :-)
I have configured it, so we'll see what happens tonight. The results will
go under http://cvs.apache.org/builds/james/nightly/. I've provided what I
hope is a suitable disclaimer:
This is an AUTOMATED NIGHTLY TEST BUILD for Apache JAMES.
Such nightly builds are automatically generated from the
current source repository for testing. Assuming that
they even compile, these unreleased builds may not even
load, may have undocumented features, known defects, and
any number of other issues. See the Apache JAMES mailing
lists if you have any questions.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Nightly builds
Posted by Danny Angus <Da...@slc.co.uk>.
Noel J Bergman wrote:
> That's fine, and I can look to setup a cron task on one of my systems at
> home to post unsigned nightly builds. But using GUMP is a non-starter.
That would be exceptionally kind, and much appreciated. :-)
I have heard that my local telephone exchange has finally been upgraded and
I can get ASDL at home, which should end my (self imposed, I gave up ISDN2e
out of spite over a huge bill) three year sentence in the dial-up ghetto.
But until I get round to it I can't match your offer.
d.
***************************************************************************
The information in this e-mail is confidential and for use by the addressee(s) only. If you are not the intended recipient (or responsible for delivery of the message to the intended recipient) please notify us immediately on 0141 306 2050 and delete the message from your computer. You may not copy or forward it or use or disclose its contents to any other person. As Internet communications are capable of data corruption Student Loans Company Limited does not accept any responsibility for changes made to this message after it was sent. For this reason it may be inappropriate to rely on advice or opinions contained in an e-mail without obtaining written confirmation of it. Neither Student Loans Company Limited or the sender accepts any liability or responsibility for viruses as it is your responsibility to scan attachments (if any). Opinions and views expressed in this e-mail are those of the sender and may not reflect the opinions and views of The Student Loans Company Limit
ed.
This footnote also confirms that this email message has been swept for the presence of computer viruses.
**************************************************************************
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Nightly builds
Posted by "Noel J. Bergman" <no...@devtech.com>.
Danny Angus wrote:
> The reason I though of [using GUMP] was to let people get
> binaries with the SoC stuff in it. That might get us at
> bigger pool of testers than making people use svn would.
That's fine, and I can look to setup a cron task on one of my systems at
home to post unsigned nightly builds. But using GUMP is a non-starter.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
RE: Status Update - James Fast Fail
Posted by Danny Angus <Da...@slc.co.uk>.
Noel,
The reason I though of it was to let people get binaries with the SoC stuff
in it.
That might get us a bigger pool of testers than making people use svn
would.
Thats all.
d.
|---------+---------------------------->
| | "Noel J. Bergman"|
| | <noel@devtech.com|
| | > |
| | |
| | 17/08/2005 22:31 |
| | Please respond to|
| | "James Developers|
| | List" |
|---------+---------------------------->
>---------------------------------------------------------------------------------------------------------------------------------|
| |
| To: "James Developers List" <se...@james.apache.org> |
| cc: |
| Subject: RE: Status Update - James Fast Fail |
>---------------------------------------------------------------------------------------------------------------------------------|
> > Does Gump publish "nightlies" for James?
> it would be cool to have nighly published somewhere
? now that we have gump working again :-)
Absolutely not! GUMP is explicitly an untrusted system that is not
permitted to export artifacts other than text reports.
We can, if there is sufficient reason, publish our own nightly builds. I
can try to set that up this weekend when I am home.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
***************************************************************************
The information in this e-mail is confidential and for use by the addressee(s) only. If you are not the intended recipient (or responsible for delivery of the message to the intended recipient) please notify us immediately on 0141 306 2050 and delete the message from your computer. You may not copy or forward it or use or disclose its contents to any other person. As Internet communications are capable of data corruption Student Loans Company Limited does not accept any responsibility for changes made to this message after it was sent. For this reason it may be inappropriate to rely on advice or opinions contained in an e-mail without obtaining written confirmation of it. Neither Student Loans Company Limited or the sender accepts any liability or responsibility for viruses as it is your responsibility to scan attachments (if any). Opinions and views expressed in this e-mail are those of the sender and may not reflect the opinions and views of The Student Loans Company Limit
ed.
This footnote also confirms that this email message has been swept for the presence of computer viruses.
**************************************************************************
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Stefano Bagnara <ap...@bago.org>.
> > > Does Gump publish "nightlies" for James?
>
> > it would be cool to have nighly published somewhere
> ? now that we have gump working again :-)
>
> Absolutely not! GUMP is explicitly an untrusted system that
> is not permitted to export artifacts other than text reports.
>
> We can, if there is sufficient reason, publish our own
> nightly builds. I can try to set that up this weekend when I am home.
The current trunk is almost untested. I run the trunk from 1 month ago in
production, but it could be more tested if we had nightly/weekly binary
distributions.
The nightly builds could help speeding up the testing process and the
release cycles.
A milestone release or an alpha release would help also now that we solved
the BXA problems removing the crypto libs.
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
RE: Status Update - James Fast Fail
Posted by "Noel J. Bergman" <no...@devtech.com>.
> > Does Gump publish "nightlies" for James?
> it would be cool to have nighly published somewhere
? now that we have gump working again :-)
Absolutely not! GUMP is explicitly an untrusted system that is not
permitted to export artifacts other than text reports.
We can, if there is sufficient reason, publish our own nightly builds. I
can try to set that up this weekend when I am home.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Stefano Bagnara <ap...@bago.org>.
> Everyone:
> Does Gump publish "nightlies" for James?
I asked the same a few days ago: it would be cool to have nighly published
somewhere now that we have gump working again :-)
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Danny Angus <Da...@slc.co.uk>.
> I think it's better to have this committed and
> changes between versions in svn
+1 this is what we agreed with the students.
Anagha:
Now just submit incremental patches, you're doing a great job.
Sorry I don't have time to pay closer attention!
If you want people to test it, post to this list, but also consider posting
to the users list.
Everyone:
Does Gump publish "nightlies" for James?
d.
***************************************************************************
The information in this e-mail is confidential and for use by the addressee(s) only. If you are not the intended recipient (or responsible for delivery of the message to the intended recipient) please notify us immediately on 0141 306 2050 and delete the message from your computer. You may not copy or forward it or use or disclose its contents to any other person. As Internet communications are capable of data corruption Student Loans Company Limited does not accept any responsibility for changes made to this message after it was sent. For this reason it may be inappropriate to rely on advice or opinions contained in an e-mail without obtaining written confirmation of it. Neither Student Loans Company Limited or the sender accepts any liability or responsibility for viruses as it is your responsibility to scan attachments (if any). Opinions and views expressed in this e-mail are those of the sender and may not reflect the opinions and views of The Student Loans Company Limit
ed.
This footnote also confirms that this email message has been swept for the presence of computer viruses.
**************************************************************************
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Stefano Bagnara <ap...@bago.org>.
I think it's better to have this committed and changes between versions in
svn than having only patches in JIRA so I merged and applied the patch. I
tested the code and it seems to work: we could always revert later if
needed.
Anagha: can you add a check while loading the configuration that will avoid
to start when there is no command map configured?
It should also add a WARNING when basic commands are missing (I would say:
QUIT, HELO, MAIL, RCPT, DATA).
> I too want to make SMTPSession as possible. Currently I have
> various interfaces for making refactoring easy. they will be
> removed. I dont like the getWatchDog() and various other
> interfaces in SMTPSession.
> any suggestions?
No ideas now, sorry.
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Anagha Mudigonda <an...@gmail.com>.
On 8/17/05, Stefano Bagnara <ap...@bago.org> wrote:
> 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.
I agree with you. I will make the change
>
> 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)
You are right! Lot of clean up needed.
I did not add to state hashmap. because the comment said it should store
only MAIL-RCPT-DATA in compliance with the RFC implementation guideline
>
> 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)
>
makes sense, will do the change.
I too want to make SMTPSession as possible. Currently I have various
interfaces for making refactoring easy. they will be removed. I dont
like the getWatchDog() and various other interfaces in SMTPSession.
any suggestions?
> BTW, Good work!
thank you.
--Anagha
> Stefano
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Stefano Bagnara <ap...@bago.org>.
> [..]
> 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: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Anagha Mudigonda <an...@gmail.com>.
stefano, thanks for the feedback. find my responses inline
-- Anagha
On 8/17/05, Stefano Bagnara <ap...@bago.org> wrote:
> A few notes/questions from a first review:
>
> 1) Your tipical onCommand implementation is:
>
> public void onCommand(SMTPSession session) {
> //deviation from the Main code
> //Instead of throwing exception just end the session
> try{
> doAUTH(session, session.getCommandArgument());
> } catch (Exception ex) {
> session.getLogger().error("Exception occured:" +
> ex.getMessage());
> session.endSession();
> }
> }
>
> You catch "Exception" and return a generic error closing the session.
> I'm not sure but IIRC the original code didn't catch the exception in every
> command but has specific catch outside.
> It could be probably better to add the possible exception to the
> CommandHandler / ConnectHandler interfaces and add the "throws" in the
> onCommand/onConnect so you don't have to catch the generic exception while
> handling the command.
> The main thing I don't like is the "catch(Exception ex)" that will catch
> EVERY problem and hide the whole stacktrace/cause.
>
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?
> 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.
>
> 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
>
> 4) Why did you choose to create the following states?:
> private final static byte COMMAND_MODE = 1;
> private final static byte RESPONSE_MODE = 2;
> private final static byte MESSAGE_RECEIVED_MODE = 3;
> private final static byte MESSAGE_ABORT_MODE = 4;
> Can you explain the meaning of the states and their usage?
> In particular I'm interested to know why do you need to add the
> RESPONSE_MODE ?
>
At the beginning
mode = command_mode
once the commandHandler writes response, the mode is changed to
RESPONSE_MODE. This will cause to skip the next command handlers
configured for that command.
For instance:
There are 2 commandhandlers MailAddressChecker and MailCmdHandler for
MAIL command. If MailAddressChecker validation of the MAIL FROM
address is successful, the MailCmdHandlers will be executed.
Incase it fails, it has to write response. Once we write response
there is no need to execute the MailCmdHandler.
Thus RESPONSE_MODE is used to skip execution of remaining
commandhandlers for that command
Next, Once MAIL message is received the DataCmdHandler and any other
equivalent commandHandler will call setMail method.
this will change the mode to MAIL_RECEIVED_MODE
This mode will trigger the message handlers to be execute. Message handlers
can abort message. In that message will not spooled.
All the handlers can abort session, in that case it will get out of
the main loop
and session will end.
> 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.
>
> Stefano
>
> PS: I didn't run the code, only merged it with my code and read it.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Stefano Bagnara <ap...@bago.org>.
A few notes/questions from a first review:
1) Your tipical onCommand implementation is:
public void onCommand(SMTPSession session) {
//deviation from the Main code
//Instead of throwing exception just end the session
try{
doAUTH(session, session.getCommandArgument());
} catch (Exception ex) {
session.getLogger().error("Exception occured:" +
ex.getMessage());
session.endSession();
}
}
You catch "Exception" and return a generic error closing the session.
I'm not sure but IIRC the original code didn't catch the exception in every
command but has specific catch outside.
It could be probably better to add the possible exception to the
CommandHandler / ConnectHandler interfaces and add the "throws" in the
onCommand/onConnect so you don't have to catch the generic exception while
handling the command.
The main thing I don't like is the "catch(Exception ex)" that will catch
EVERY problem and hide the whole stacktrace/cause.
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.
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?
4) Why did you choose to create the following states?:
private final static byte COMMAND_MODE = 1;
private final static byte RESPONSE_MODE = 2;
private final static byte MESSAGE_RECEIVED_MODE = 3;
private final static byte MESSAGE_ABORT_MODE = 4;
Can you explain the meaning of the states and their usage?
In particular I'm interested to know why do you need to add the
RESPONSE_MODE ?
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).
Stefano
PS: I didn't run the code, only merged it with my code and read it.
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Anagha Mudigonda <an...@gmail.com>.
oops!
I have created a new patch that has all newly added the files
http://issues.apache.org/jira/secure/attachment/12311785/16Aug2005JAMESFastFail.patch
--anagha
On 8/16/05, Stefano Bagnara <ap...@bago.org> wrote:
> > I have submitted the patch to JIRA. This is initial version.
> > I have done basic testing to check that code doesnot break anything.
> > i will continue to test and will provide next version based
> > on the comments from you guys.
> >
> > http://issues.apache.org/jira/browse/JAMES-407
> >
> > Download the patch from
> > http://issues.apache.org/jira/secure/attachment/12311775/16Aug
> > 2005JAMESFastFail.patch
>
> The patch does not include the *CmdHandler.
> Can you submit that code too?
>
> Stefano
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Stefano Bagnara <ap...@bago.org>.
> I have submitted the patch to JIRA. This is initial version.
> I have done basic testing to check that code doesnot break anything.
> i will continue to test and will provide next version based
> on the comments from you guys.
>
> http://issues.apache.org/jira/browse/JAMES-407
>
> Download the patch from
> http://issues.apache.org/jira/secure/attachment/12311775/16Aug
> 2005JAMESFastFail.patch
The patch does not include the *CmdHandler.
Can you submit that code too?
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Anagha Mudigonda <an...@gmail.com>.
I have submitted the patch to JIRA. This is initial version.
I have done basic testing to check that code doesnot break anything.
i will continue to test and will provide next version based on the
comments from you guys.
http://issues.apache.org/jira/browse/JAMES-407
Download the patch from
http://issues.apache.org/jira/secure/attachment/12311775/16Aug2005JAMESFastFail.patch
--anagha
On 8/15/05, Danny Angus <da...@gmail.com> wrote:
> Yes, official answer is to submit a patch, we'll review it and commit it.
>
> On 10/08/05, Stefano Bagnara <ap...@bago.org> wrote:
> > > Right now I am testing thoroughly. I can provide this first
> > > version of the code for review this weekend. How can I check
> > > in the code as I dont have priveleges?
> >
> > You should wait an "official" answer from Danny or Noel...
> >
> > IMHO, the best thing you can do is to create a patch against the current
> > trunk (or whatever version you prefer) and submit the "patch" to JIRA giving
> > permissions to include the code to ASF (it is a flag when you add an
> > attachment in JIRA).
> >
> > This way we can test the code and decide wether to include it in the next
> > 2.3 release or move the integration to the following release.
> >
> > Stefano
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> > For additional commands, e-mail: server-dev-help@james.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Danny Angus <da...@gmail.com>.
Yes, official answer is to submit a patch, we'll review it and commit it.
On 10/08/05, Stefano Bagnara <ap...@bago.org> wrote:
> > Right now I am testing thoroughly. I can provide this first
> > version of the code for review this weekend. How can I check
> > in the code as I dont have priveleges?
>
> You should wait an "official" answer from Danny or Noel...
>
> IMHO, the best thing you can do is to create a patch against the current
> trunk (or whatever version you prefer) and submit the "patch" to JIRA giving
> permissions to include the code to ASF (it is a flag when you add an
> attachment in JIRA).
>
> This way we can test the code and decide wether to include it in the next
> 2.3 release or move the integration to the following release.
>
> Stefano
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: Status Update - James Fast Fail
Posted by Stefano Bagnara <ap...@bago.org>.
> Right now I am testing thoroughly. I can provide this first
> version of the code for review this weekend. How can I check
> in the code as I dont have priveleges?
You should wait an "official" answer from Danny or Noel...
IMHO, the best thing you can do is to create a patch against the current
trunk (or whatever version you prefer) and submit the "patch" to JIRA giving
permissions to include the code to ASF (it is a flag when you add an
attachment in JIRA).
This way we can test the code and decide wether to include it in the next
2.3 release or move the integration to the following release.
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org