You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by Nathan Mittler <na...@gmail.com> on 2006/11/11 23:40:22 UTC

code review for transport patch

Hey guys,
I took a crack at a fix which sends a stomp error frame back to the client
in the event of a failure during authentication or authorization.  I haven't
been through much of this code before, so I'd like to get some feedback on
my changes.

Part of my concern is that some of the tweaks will affect all protocols (not
only stomp) so I want to make sure I get it right.  In particular, a
connection error is returned back to the client if an exception is caught by
the TransportConnection.onCommand.  In the case of stomp, this ends up as a
stomp error frame.

Here's the patch against the latest trunk:

http://people.apache.org/~nmittler/patch.txt

I also have a tarball if anyone wants to check it out:

http://people.apache.org/~nmittler/
apache-activemq-4.2-incubator-SNAPSHOT.tar.gz<http://people.apache.org/%7Enmittler/apache-activemq-4.2-incubator-SNAPSHOT.tar.gz>

Thanks,
Nate

Re: code review for transport patch

Posted by Nathan Mittler <na...@gmail.com>.
One more thing ... looks like all the unit tests in the broker still pass
with these changes

On 11/11/06, Nathan Mittler <na...@gmail.com> wrote:
>
> Hey guys,
> I took a crack at a fix which sends a stomp error frame back to the client
> in the event of a failure during authentication or authorization.  I haven't
> been through much of this code before, so I'd like to get some feedback on
> my changes.
>
> Part of my concern is that some of the tweaks will affect all protocols
> (not only stomp) so I want to make sure I get it right.  In particular, a
> connection error is returned back to the client if an exception is caught by
> the TransportConnection.onCommand.  In the case of stomp, this ends up as
> a stomp error frame.
>
> Here's the patch against the latest trunk:
>
> http://people.apache.org/~nmittler/patch.txt<http://people.apache.org/%7Enmittler/patch.txt>
>
> I also have a tarball if anyone wants to check it out:
>
> http://people.apache.org/~nmittler/<http://people.apache.org/%7Enmittler/>apache-activemq-4.2-incubator-SNAPSHOT.tar.gz
>
> <http://people.apache.org/%7Enmittler/apache-activemq-4.2-incubator-SNAPSHOT.tar.gz>
>
> Thanks,
> Nate
>

Re: code review for transport patch

Posted by Nathan Mittler <na...@gmail.com>.
oops - You're right,  My mistake!

On 11/11/06, Jason Dillon <ja...@planet57.com> wrote:
>
> I think this belongs on the ActiveMQ dev list...
> --jason
>
>
> On Nov 11, 2006, at 2:40 PM, Nathan Mittler wrote:
>
> Hey guys,
> I took a crack at a fix which sends a stomp error frame back to the client
> in the event of a failure during authentication or authorization.  I haven't
> been through much of this code before, so I'd like to get some feedback on
> my changes.
>
> Part of my concern is that some of the tweaks will affect all protocols
> (not only stomp) so I want to make sure I get it right.  In particular, a
> connection error is returned back to the client if an exception is caught by
> the TransportConnection.onCommand.  In the case of stomp, this ends up as
> a stomp error frame.
>
> Here's the patch against the latest trunk:
>
> http://people.apache.org/~nmittler/patch.txt<http://people.apache.org/%7Enmittler/patch.txt>
>
> I also have a tarball if anyone wants to check it out:
>
> http://people.apache.org/~nmittler/<http://people.apache.org/%7Enmittler/>apache-activemq-4.2-incubator-SNAPSHOT.tar.gz
>
> <http://people.apache.org/%7Enmittler/apache-activemq-4.2-incubator-SNAPSHOT.tar.gz>
>
> Thanks,
> Nate
>
>
>

Re: code review for transport patch

Posted by Jason Dillon <ja...@planet57.com>.
I think this belongs on the ActiveMQ dev list...

--jason


On Nov 11, 2006, at 2:40 PM, Nathan Mittler wrote:

> Hey guys,
> I took a crack at a fix which sends a stomp error frame back to the  
> client in the event of a failure during authentication or  
> authorization.  I haven't been through much of this code before, so  
> I'd like to get some feedback on my changes.
>
> Part of my concern is that some of the tweaks will affect all  
> protocols (not only stomp) so I want to make sure I get it right.   
> In particular, a connection error is returned back to the client if  
> an exception is caught by the TransportConnection.onCommand.  In  
> the case of stomp, this ends up as a stomp error frame.
>
> Here's the patch against the latest trunk:
>
> http://people.apache.org/~nmittler/patch.txt
>
> I also have a tarball if anyone wants to check it out:
>
> http://people.apache.org/~nmittler/apache-activemq-4.2-incubator- 
> SNAPSHOT.tar.gz
>
> Thanks,
> Nate


Re: code review for transport patch

Posted by Jason Dillon <ja...@planet57.com>.
I think this belongs on the ActiveMQ dev list...

--jason


On Nov 11, 2006, at 2:40 PM, Nathan Mittler wrote:

> Hey guys,
> I took a crack at a fix which sends a stomp error frame back to the  
> client in the event of a failure during authentication or  
> authorization.  I haven't been through much of this code before, so  
> I'd like to get some feedback on my changes.
>
> Part of my concern is that some of the tweaks will affect all  
> protocols (not only stomp) so I want to make sure I get it right.   
> In particular, a connection error is returned back to the client if  
> an exception is caught by the TransportConnection.onCommand.  In  
> the case of stomp, this ends up as a stomp error frame.
>
> Here's the patch against the latest trunk:
>
> http://people.apache.org/~nmittler/patch.txt
>
> I also have a tarball if anyone wants to check it out:
>
> http://people.apache.org/~nmittler/apache-activemq-4.2-incubator- 
> SNAPSHOT.tar.gz
>
> Thanks,
> Nate