Hi Danushka,

I have glanced at your patches, ill try to take a proper look some
evening soon that I get time, but for now I would note the following:

In QPID-2726 you seem to have changed how the authentication is
performed in PlainSaslServer and yet havent modified the existing
PlainPasswordFilePrincipalDatabase at all - does it still actually
work after the patch is applied?

Im not sure that creating a QpidPasswordCallBack which isnt actually
used in the intended fashion of a PasswordCallBack (to retrieve a
password from the underlying implementation, rather than provide one
to it) is the correct way to go here. If you have a custom
PrincipalDatabase needing to implement the SASL/PLAIN profile then
perhaps it should also have a custom SaslServer(Factory) and
Initialiser to go with it that can be registered in the same fashion
as the existing Plain/Base64MD5 ones are and do the appropriate
comparisons for that database. This could eg be a subclass of
PlainSaslServer which differed only in how it compares the passwords
after doing the callbacks.

Also, I see Andrew has already posted some comments to QPID-2720 and
mentions commented out code, that seems to apply here as well. You
should also probably look at the following page regarding our
conventions for field naming etc:
https://cwiki.apache.org/confluence/display/qpid/Java+Coding+Standards

Robbie.

On 7 July 2010 10:53, Danushka Menikkumbura
<[email protected]> wrote:
> Hi devs,
>
> Could someone review and apply the patches for QPID-2720 and QPID-2726
> please?.
>
> Thanks,
> Danushka
>
> --
> Danushka Menikkumbura
> Apache Axis2 PMC Member
>
> Apache Qpid - World Domination through Advanced Message Queueing ;
> http://qpid.apache.org
>
> phone : +94 77 364 1754
> personal blog : http://danushka-menikkumbura.blogspot.com/
> <http://danushka-menikkumbura.blogspot.com/>technical blog :
> http://danushkastechythoughts.blogspot.com/
>  <http://danushkastechythoughts.blogspot.com/>twitter :
> http://twitter.com/danushkamenik
> <http://twitter.com/danushkamenik>linkedin :
> http://lk.linkedin.com/in/danushka
>

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:[email protected]

Reply via email to