qpid-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gordon Sim <g...@redhat.com>
Subject Re: Inter-broker link with SSL
Date Tue, 05 Aug 2014 16:51:25 GMT
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: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org


Mime
View raw message