----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1608/#review1767 -----------------------------------------------------------
/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java <https://reviews.apache.org/r/1608/#comment4018> I believe you forgot to add ClientConnectionDelegate ? I can't seem to find this in the current source tree. Perhaps some of the missing code from ClientDelegate is moved to this class? /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java <https://reviews.apache.org/r/1608/#comment4012> Why is this code (related to GSSAPI) is removed ? I don't see this code moved elsewhere either? This will break existing functionality :( /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java <https://reviews.apache.org/r/1608/#comment4013> Removing this without a suitable replacement will break existing functionality. Is this check performed else where? Could you pls help me located it? This was in place to ensure the client to throw an exception if it's not configured to support any of the mechanisms supported by the broker. It used to be that we just ignored SASL all together if no matching mechs were found. I'd argue that this is an important check. Could you please explain your reasons behind the removal (if the same check is not performed elsewhere)? /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java <https://reviews.apache.org/r/1608/#comment4014> Lines 138 to 141 were required for SASL encryption support. Why is this removed ? Is this taken care of elsewhere? If so my apologies (but could you point me to the relevant code?). /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java <https://reviews.apache.org/r/1608/#comment4015> The user identity when using GSSAPI and External are crucial for ACL support. We have existing customers relying on this feature. Why was this removed? (Again is there a replacement for this some where?) /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java <https://reviews.apache.org/r/1608/#comment4016> Again this method was used to retrieved the kerberos identity of the user. /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionSettings.java <https://reviews.apache.org/r/1608/#comment4017> I would think it's better to default to PLAIN as that would be the one that will be universally supported. However I believe you now retrieve the default from the sasl config file ? - rajith On 2011-08-22 08:58:27, Keith Wall wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1608/ > ----------------------------------------------------------- > > (Updated 2011-08-22 08:58:27) > > > Review request for qpid and rajith attapattu. > > > Summary > ------- > > This patch changes the 0-10 code path to create the SASL callback handler > using the CallbackHandlerRegistry. This allows the 0-10 code path to > support SASL mechanisms requiring other callback handlers, such as > CRAM-MD5-HASHED. Support for the sasl_mechs client connection option has > been retained and now applies to the 0-8..0-9-1 code paths too. > > If the user *specifies* a sasl_mechs client connection option the behaviour > of the code is unchanged from the previous version: it restricts the list of > SASL mechanisms in use. > > If the user *does not specify* a sasl_mechs client connection option, the old > code used a hardcoded PLAIN default. This is no longer the case. Now the > client will use the first SASL mechanism from the list > CallbackHandlerRegistry.properties that is also available on the server. > > Removed dead code and strengthen unit tests. > > > > This addresses bug QPID-3415. > https://issues.apache.org/jira/browse/QPID-3415 > > > Diffs > ----- > > > /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java > 1160136 > > /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionStartMethodHandler.java > 1160136 > > /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/CallbackHandlerRegistry.java > 1160136 > > /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/CallbackHandlerRegistry.properties > 1160136 > > /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ClientDelegate.java > 1160136 > > /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java > 1160136 > > /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionSettings.java > 1160136 > > /trunk/qpid/java/common/src/test/java/org/apache/qpid/transport/ConnectionTest.java > 1160136 > > /trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/client/connection/ConnectionTest.java > 1160136 > > /trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/message/UTF8Test.java > 1160136 > > Diff: https://reviews.apache.org/r/1608/diff > > > Testing > ------- > > Improved unit testing. Run java, cpp and cpp.ssl profiles. I am not able to > test GSSAPI locally. > > > Thanks, > > Keith > >
