> On Oct. 12, 2015, 2:57 p.m., Kenneth Giusti wrote: > > trunk/qpid/cpp/src/qpid/broker/amqp/Domain.cpp, line 237 > > <https://reviews.apache.org/r/39173/diff/2/?file=1094232#file1094232line237> > > > > Hrmmm... not sure I follow. > > > > There are 3 potential sources for the sasl service name. In priority: > > > > 1) passed via the domain create command option --sasl-service-name > > 2) lacking that, use the value configured via the broker wide > > configuration option --sasl-service-name > > 3) lacking that, default to 'amqp' > > > > If we remove the conditional, the code would ignore the broker > > configuration setting.
yes, that is correct - the previous code never tried to use the command line option, so why should this change? It was getting the configured property to use from somewhere and it will still do that. Given that the command line option is the sasl service name to use when *listening*, I don't think it is correct to be using that name to connect outbound. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39173/#review102254 ----------------------------------------------------------- On Oct. 9, 2015, 8:33 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39173/ > ----------------------------------------------------------- > > (Updated Oct. 9, 2015, 8:33 p.m.) > > > Review request for qpid, Andrew Stitcher and Gordon Sim. > > > Bugs: QPID-6783 > https://issues.apache.org/jira/browse/QPID-6783 > > > Repository: qpid > > > Description > ------- > > See related JIRA > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1706484 > trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1706484 > trunk/qpid/cpp/src/qpid/broker/amqp/Domain.cpp 1706484 > trunk/qpid/cpp/src/qpid/broker/amqp/ProtocolPlugin.cpp 1706484 > trunk/qpid/cpp/src/qpid/messaging/ConnectionOptions.cpp 1706484 > > Diff: https://reviews.apache.org/r/39173/diff/ > > > Testing > ------- > > Unit tests. > > > Thanks, > > Kenneth Giusti > >
