Logic error in SslEngineByteBufferInputStream causing exception and connection being aborted

[dimas]

This code

rabbitmq-java-client/src/main/java/com/rabbitmq/client/impl/nio/SslEngineByteBufferInputStream.java

Lines 79 to 83 in 732cade

	result = sslEngine.unwrap(cipherIn, plainIn);
	if (result.getStatus() != SSLEngineResult.Status.OK) {
	throw new SSLException("Unexpected result: " + result);
	}

is not prepared to receive anything but OK from SslEngine.unwrap but if there is still not enough data, even after additional read, another BUFFER_UNDERFLOW can be received. When this happens, exception is thrown and connection is killed.

I do not know how to reproduce it reliably but I have a test case which fail most of the time on my laptop. However it may require a slower machine (or a heavier loaded machine) to fail.

I am going to submit PR with the test and fixes, raising this issue first to have a link to reference to.

[dimas]

Okie, dokie, so the PR is #318

Couple of notes about it:

1. I introduced NioHelper.readWithRetry to simplify logic in SslEngineByteBufferInputStream. There are other places in client code where the same pattern of NioHelper.read+NioHelper.retryRead is used that could benefit from the new method but I did not change the existing code to mimimise the change but you may want to do a bigger change - ByteBufferInputStream.readFromNetworkIfNecessary and SslEngineHelper.unwrap can use it.
2. that whole NioHelper is a wrong thing because it defeats the purpose of NIO but I it is beyond the scope of this PR so I will raise it on the mailing list instead.
3. There is obviously no guarantee for the test to prove anything. With the old code it fails most of the time on my laptop with RabbitMQ running in vagrant VM on the same machine but it may not fail for you. Due to the nature of the problem - it all depends on timing if it happens or not.

[acogoluegnes]

Fixed by #318.

[michaelklishin]

@dimas great find, thank you so much!