You are viewing a plain text version of this content. The canonical link for it is here.
Posted to ftpserver-users@mina.apache.org by Sai Pullabhotla <sa...@jmethods.com> on 2008/11/20 18:18:59 UTC
Patch for FTPSERVER-222
Attached is the patch for the Jira issue FTPSERVER-222 which calls
back the Ftplet.afterCommand method with the actual reply that was
sent to the client.
I've tested it reasonably and works as expected. However, there is an
issue when the Ftplet.beforeCommand sends a response other than the
DEFAULT, then afterCommand is not called any more. I know this was
recently changed to behave this way, but I think the afterCommand MUST
be called no matter what. Let me know what you think.
Sai Pullabhotla
Phone: (402) 408-5753
Fax: (402) 408-6861
www.jMethods.com
Re: Patch for FTPSERVER-222
Posted by David Latorre <dv...@gmail.com>.
I just checked Spring AOP documentation and this is the kind of things they do:
-beforeAdvice: this is a method executed before a method invocation.
If this 'advice' (interceptor, ftplet method or however you call it)
throws an Exception it will abort All the execution flow (other
interceptors included). So this is basically what we are doing now,
but instead of relying on Exceptions , we are using different return
values.
-afterReturningAdvice: this is executed after the method invocation.
It will be called only if the method returned "successfully" (it
didn't throw an exception). So it works as our afterCommand.
-afterThrowingAdvice: this would be executed only if the invoked
method threw an Exception.
-afterFinallyAdvice: this will be executed no matter what's the return
value of the invoked method.
By the way, I'm glad that this approach is similar to the one I
myself suggested :P Our difference here would be that we want to
execute afterThrowing and afterFinallyAdvice even if the beforeAdvice
failed. For this to work correctly we have to have commands return a
value or we might need to investigate the ftpreplies sent or maybe
both of them.
* There's also an interceptor called aroundAdvice: inside its code
our ftplet would be able to execute any code, invoke the command and
continue executing its own code. This interceptor seems powerful but
I do not think we should try to emulate its behaviour.
So maybe we could implement something like this. Although there're
tricky parts as what are the expected behaviours depending on the type
of result : SKIP or DISCONNECT or when an Exception is thrown from a
ftplet. By the way, I'm not really fond of our closing the connection
when a documented checked exception is thrown from a ftplet.
david
2008/11/21 David Latorre <dv...@gmail.com>
>
>
> 2008/11/20 Niklas Gustavsson <ni...@protocol7.com>
>>
>> On Thu, Nov 20, 2008 at 10:02 PM, Sai Pullabhotla
>> <sa...@jmethods.com> wrote:
>> > How about a scenario where I use two Ftplets one for access control
>> > and another for logging? Potentially, I could buy these Ftplets from
>> > two different vendors and I do not have access to the source code. So,
>> > my logging Ftplet does not work as expected.
>>
>> Buying Ftplets from vendors... that would be the day :-)
>>
>> Anyways, I think you make a good case. David, you were in on the last
>> discussion on this topic. What's your thoughts?
>
> I think Sai's suggestion makes sense. Although if you 'bought two Ftplet's from different vendors' what would the logging ftplet do when access is denied ? Log that there was an unknown error ( that access was denied) ? Probably the first ftplet did that already but providing more information....
> And then, if different vendors provided you with beforeCommand ftplets, we have the same case here, the first one in being executed can return a SKIP or DISCONNECT result so the others won't be executed.
> Anyway, there are several ideas, for example we could add another return code which skips the command but executes the afterCommands.
> Anyway if we have the return code now (and we can guess from it that everything worked! I should check if that's true ...) it is not that important that we can SKIP everything.
> Besides, we also can also implement some kind of "Exception handling ": afterSuccessfulCommand(), afterCommandError(), afterCommand(). Last once would execute always, the other ones just in case the command completed correctly or an error occurred. This would allow us for a finer control of the execution flow. Maybe we could return a value in commands as well so we can know if the command finished successfully.
> Our FTPLets remind me very much of AOP, so let me check how they implement this kind of behaviour ( do you know this? , please step in!).
> What are your thoughts?
>
>
>>
>> /niklas
>
Re: Patch for FTPSERVER-222
Posted by David Latorre <dv...@gmail.com>.
2008/11/20 Niklas Gustavsson <ni...@protocol7.com>
> On Thu, Nov 20, 2008 at 10:02 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
> > How about a scenario where I use two Ftplets one for access control
> > and another for logging? Potentially, I could buy these Ftplets from
> > two different vendors and I do not have access to the source code. So,
> > my logging Ftplet does not work as expected.
>
> Buying Ftplets from vendors... that would be the day :-)
>
> Anyways, I think you make a good case. David, you were in on the last
> discussion on this topic. What's your thoughts?
>
I think Sai's suggestion makes sense. Although if you 'bought two Ftplet's
from different vendors' what would the logging ftplet do when access is
denied ? Log that there was an unknown error ( that access was denied) ?
Probably the first ftplet did that already but providing more
information....
And then, if different vendors provided you with beforeCommand ftplets,
we have the same case here, the first one in being executed can return a
SKIP or DISCONNECT result so the others won't be executed.
Anyway, there are several ideas, for example we could add another return
code which skips the command but executes the afterCommands.
Anyway if we have the return code now (and we can guess from it that
everything worked! I should check if that's true ...) it is not that
important that we can SKIP everything.
Besides, we also can also implement some kind of "Exception handling ":
afterSuccessfulCommand(), afterCommandError(), afterCommand(). Last once
would execute always, the other ones just in case the command completed
correctly or an error occurred. This would allow us for a finer control of
the execution flow. Maybe we could return a value in commands as well so we
can know if the command finished successfully.
Our FTPLets remind me very much of AOP, so let me check how they implement
this kind of behaviour ( do you know this? , please step in!).
What are your thoughts?
>
> /niklas
>
Re: Patch for FTPSERVER-222
Posted by Niklas Gustavsson <ni...@protocol7.com>.
On Thu, Nov 20, 2008 at 10:02 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> How about a scenario where I use two Ftplets one for access control
> and another for logging? Potentially, I could buy these Ftplets from
> two different vendors and I do not have access to the source code. So,
> my logging Ftplet does not work as expected.
Buying Ftplets from vendors... that would be the day :-)
Anyways, I think you make a good case. David, you were in on the last
discussion on this topic. What's your thoughts?
/niklas
Re: Patch for FTPSERVER-222
Posted by Sai Pullabhotla <sa...@jmethods.com>.
How about a scenario where I use two Ftplets one for access control
and another for logging? Potentially, I could buy these Ftplets from
two different vendors and I do not have access to the source code. So,
my logging Ftplet does not work as expected.
> The reason why I don't think it
> makes much sense to call afterCommand is that we (FtpServer) has no
> clue what the Ftplet has done, it knows way better than us when the
> command (or commands) is complete).
I still am not sure what is the harm in calling the afterCommand
method after the command is processed (in this case after the ftplet's
beginCommand has finished).
Sai Pullabhotla
Phone: (402) 408-5753
Fax: (402) 408-6861
www.jMethods.com
On Thu, Nov 20, 2008 at 2:03 PM, Niklas Gustavsson <ni...@protocol7.com> wrote:
> On Thu, Nov 20, 2008 at 8:26 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> I attached the patch as a file attachment to the bug. Hope that is
>> what you wanted me to do. If not let me know.
>
> Yes, perfect.
>
>> As far as calling afterCommand everytime, it is logical for an Ftplet
>> to always expect a notification when a command has finished running
>> (because that's what the method is intended for). For example, if I
>> have an Ftplet that traps the beforeCommand method and checks to see
>> if the user is authorized to run the specific command (for e.g. RETR,
>> DELE etc). From this method, I send 5xx reply if the user is not
>> authorized to execute the command. Now, from the same Ftplet, I
>> implement the afterCommand method and use it to do some kind of
>> logging which includes the original command as well as the result.
>> However, the afterCommand is not called if I returned a SKIP from the
>> beforeCommand which means, my log would miss this command unless I
>> duplicate the logging code in the beforeCommand also.
>
> Yeah, but the Ftplet has total control over what is happening. So, if
> you want to do logging after the command, break that into a method on
> it's own and call it from within the code in beforeCommand that sends
> the 5xx as well from afterCommand. The reason why I don't think it
> makes much sense to call afterCommand is that we (FtpServer) has no
> clue what the Ftplet has done, it knows way better than us when the
> command (or commands) is complete).
>
> /niklas
>
Re: Patch for FTPSERVER-222
Posted by Niklas Gustavsson <ni...@protocol7.com>.
On Thu, Nov 20, 2008 at 8:26 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> I attached the patch as a file attachment to the bug. Hope that is
> what you wanted me to do. If not let me know.
Yes, perfect.
> As far as calling afterCommand everytime, it is logical for an Ftplet
> to always expect a notification when a command has finished running
> (because that's what the method is intended for). For example, if I
> have an Ftplet that traps the beforeCommand method and checks to see
> if the user is authorized to run the specific command (for e.g. RETR,
> DELE etc). From this method, I send 5xx reply if the user is not
> authorized to execute the command. Now, from the same Ftplet, I
> implement the afterCommand method and use it to do some kind of
> logging which includes the original command as well as the result.
> However, the afterCommand is not called if I returned a SKIP from the
> beforeCommand which means, my log would miss this command unless I
> duplicate the logging code in the beforeCommand also.
Yeah, but the Ftplet has total control over what is happening. So, if
you want to do logging after the command, break that into a method on
it's own and call it from within the code in beforeCommand that sends
the 5xx as well from afterCommand. The reason why I don't think it
makes much sense to call afterCommand is that we (FtpServer) has no
clue what the Ftplet has done, it knows way better than us when the
command (or commands) is complete).
/niklas
Re: Patch for FTPSERVER-222
Posted by Sai Pullabhotla <sa...@jmethods.com>.
I attached the patch as a file attachment to the bug. Hope that is
what you wanted me to do. If not let me know.
As far as calling afterCommand everytime, it is logical for an Ftplet
to always expect a notification when a command has finished running
(because that's what the method is intended for). For example, if I
have an Ftplet that traps the beforeCommand method and checks to see
if the user is authorized to run the specific command (for e.g. RETR,
DELE etc). From this method, I send 5xx reply if the user is not
authorized to execute the command. Now, from the same Ftplet, I
implement the afterCommand method and use it to do some kind of
logging which includes the original command as well as the result.
However, the afterCommand is not called if I returned a SKIP from the
beforeCommand which means, my log would miss this command unless I
duplicate the logging code in the beforeCommand also.
Hope this makes sense.
Regards,
Sai Pullabhotla
Phone: (402) 408-5753
Fax: (402) 408-6861
www.jMethods.com
On Thu, Nov 20, 2008 at 12:49 PM, Niklas Gustavsson
<ni...@protocol7.com> wrote:
> On Thu, Nov 20, 2008 at 6:18 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> Attached is the patch for the Jira issue FTPSERVER-222 which calls
>> back the Ftplet.afterCommand method with the actual reply that was
>> sent to the client.
>
> Please attach this to the issue and make sure you tick the radiobutton
> on approving it for insertion into Apache software.
>
>> I've tested it reasonably and works as expected. However, there is an
>> issue when the Ftplet.beforeCommand sends a response other than the
>> DEFAULT, then afterCommand is not called any more. I know this was
>> recently changed to behave this way, but I think the afterCommand MUST
>> be called no matter what.
>
> There was some discussions on this, could you please describe why this
> would be necessary?
>
> /niklas
>
Re: Patch for FTPSERVER-222
Posted by Niklas Gustavsson <ni...@protocol7.com>.
On Thu, Nov 20, 2008 at 6:18 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> Attached is the patch for the Jira issue FTPSERVER-222 which calls
> back the Ftplet.afterCommand method with the actual reply that was
> sent to the client.
Please attach this to the issue and make sure you tick the radiobutton
on approving it for insertion into Apache software.
> I've tested it reasonably and works as expected. However, there is an
> issue when the Ftplet.beforeCommand sends a response other than the
> DEFAULT, then afterCommand is not called any more. I know this was
> recently changed to behave this way, but I think the afterCommand MUST
> be called no matter what.
There was some discussions on this, could you please describe why this
would be necessary?
/niklas