You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by mick goulish <mi...@redhat.com> on 2012/11/02 07:57:49 UTC
Re: Review Request: Abstraction for SASL server role that is not tied to 0-10
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7565/#review13029
-----------------------------------------------------------
Ship it!
/trunk/qpid/cpp/src/qpid/NullSaslServer.cpp
<https://reviews.apache.org/r/7565/#comment28040>
I feel kind of funny about throwing for an error in an input to the system -- but definitely should log it. info? error?
Sorry it took me so long to get to this.
All I can respond to now is overall understandability.
This new code seems hugely more comprehensible / cleaner than what I remember.
Oh yes, please ship it.
- mick goulish
On Oct. 12, 2012, 4:47 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7565/
> -----------------------------------------------------------
>
> (Updated Oct. 12, 2012, 4:47 p.m.)
>
>
> Review request for qpid, michael goulish, Ted Ross, and mick goulish.
>
>
> Description
> -------
>
> The current broker::SaslAuthenticator mixes 0-10 protocol handshaking with the sasl interactions. This patch adds a cleaner abstraction of the server role in SASL along side that already in place for the client. This is then used in the 1.0 SASL implementation.
>
> (Note this patch assumes and partially includes https://reviews.apache.org/r/7562/ as I could not find an easy way to separate out commits in git into review-board compatible patches).
>
>
> This addresses bug QPID-4368.
> https://issues.apache.org/jira/browse/QPID-4368
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/CMakeLists.txt 1397295
> /trunk/qpid/cpp/src/Makefile.am 1397295
> /trunk/qpid/cpp/src/qpid/NullSaslServer.h PRE-CREATION
> /trunk/qpid/cpp/src/qpid/NullSaslServer.cpp PRE-CREATION
> /trunk/qpid/cpp/src/qpid/Sasl.h 1397295
> /trunk/qpid/cpp/src/qpid/SaslFactory.h 1397295
> /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1397295
> /trunk/qpid/cpp/src/qpid/SaslServer.h PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7565/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request: Abstraction for SASL server role that is not tied to 0-10
Posted by Gordon Sim <gs...@redhat.com>.
> On Nov. 2, 2012, 6:57 a.m., mick goulish wrote:
> > /trunk/qpid/cpp/src/qpid/NullSaslServer.cpp, line 41
> > <https://reviews.apache.org/r/7565/diff/1/?file=176223#file176223line41>
> >
> > I feel kind of funny about throwing for an error in an input to the system -- but definitely should log it. info? error?
Committed a minor adjustment to log this and return the failure code.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7565/#review13029
-----------------------------------------------------------
On Oct. 12, 2012, 4:47 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7565/
> -----------------------------------------------------------
>
> (Updated Oct. 12, 2012, 4:47 p.m.)
>
>
> Review request for qpid, michael goulish, Ted Ross, and mick goulish.
>
>
> Description
> -------
>
> The current broker::SaslAuthenticator mixes 0-10 protocol handshaking with the sasl interactions. This patch adds a cleaner abstraction of the server role in SASL along side that already in place for the client. This is then used in the 1.0 SASL implementation.
>
> (Note this patch assumes and partially includes https://reviews.apache.org/r/7562/ as I could not find an easy way to separate out commits in git into review-board compatible patches).
>
>
> This addresses bug QPID-4368.
> https://issues.apache.org/jira/browse/QPID-4368
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/CMakeLists.txt 1397295
> /trunk/qpid/cpp/src/Makefile.am 1397295
> /trunk/qpid/cpp/src/qpid/NullSaslServer.h PRE-CREATION
> /trunk/qpid/cpp/src/qpid/NullSaslServer.cpp PRE-CREATION
> /trunk/qpid/cpp/src/qpid/Sasl.h 1397295
> /trunk/qpid/cpp/src/qpid/SaslFactory.h 1397295
> /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1397295
> /trunk/qpid/cpp/src/qpid/SaslServer.h PRE-CREATION
>
> Diff: https://reviews.apache.org/r/7565/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gordon Sim
>
>