On 08/05/2014 05:18 PM, Chris Richardson wrote:
I think I've tracked this one down.
Good job! Thanks for chasing this down!
My theory is that the qpid::amqp_1_10::Connection::decode method, which
contains the code
if (isClient && !initialized) {
//read in protocol header
framing::ProtocolInitiation pi;
if (pi.decode(in)) {
if(!(pi==version))
throw Exception(QPID_MSG("Unsupported version: " << pi
<< " supported version " <<
version));
QPID_LOG(trace, "RECV [" << identifier << "]: INIT(" << pi <<
")");
}
initialized = true;
}
can incorrectly set the connection status to "initialised" when the
pi.decode() method fails (which it does if <8 bytes are available). This
does not happen in practice without SSL, but when SSL is switched on I have
seen zero, 1 and 4 bytes turn up in the input buffer on first invocation.
Moving the "initialized = true" line to inside the "if (pi.decode(int) {"
block seems to fix the issue (this is because subsequent calls to the
Connection::decode() method (when sufficient data is available in the input
buffer) will result in the pi.decode() method correctly consuming the
"AMQP..." header).
That sounds like the right fix. Thank you once again!
(The description reminded me of
https://issues.apache.org/jira/browse/QPID-5488, but though related this
is a slightly different issue that only affects 'outgoing' connections).
A further optimisation might be to skip the decode() method body if
insufficient data is available thereby avoiding the overhead of the various
objects that are created and destroyed and the exceptions that are
thrown... but that's extra lines of code and "less is more", as they say ;)
The first line of the decode is in any case exactly such a check so I
don't think anything is gained by this.
Could a qpid dev please confirm this fix and, if approved, could we be
informed of when a fix will be released?
I'll get this in for the upcoming 0.30 release.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
