Hi,
Please find below a fix for:
8231449: HttpClient’s client ssl certificate authentication seems
to be broken.
https://bugs.openjdk.java.net/browse/JDK-8231449
webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8231449/webrev.00/
The root of the issue is that the HttpClient's SSLFlowDelegate
had a mechanism to try & limit the actual size of its readBuffer,
which contains encrypted bytes before decryption. The idea was
that if the readBuffer went over a certain limit (16K) then the
SSLFlowDelegate::Reader would not request more data from the
upstream publisher (the SocketTube) until the data was consumed.
The issue occurs when this mechanism gets triggered during the
handshake. In that case, we may not have demand from downstream
yet, and because of that, the SocketTube might deregister its
read event with the selector, until it receive more demand.
But because we're still in the handshake, such demand will never
be forthcoming.
In other words, whether to request more data from upstream is a
combination of the handshake status, the demand from downstream,
and the presence of decrypted data in the output queue.
The issue is 100% reproducible if the server side
presents a certificate that overflows the readBuffer's soft
limit of 16K.
This fix is reworking the way that data is requested from upstream.
The reason why the overflow mechanism was put in place was because
updateUpstreamWindow() was called at the wrong place: it was called
both after delivering data to be decrypted, and after data had been
decrypted. That failed to capture the proper place where to request
more data - which should have been done in the Reader's scheduler
loop. As a result - more data than actually required could be
delivered, which introduced the need for trying to limit the
size of the readBuffer, which in turn caused the issue
observed when that happened during the handshake.
The fix now handles all upstream requests from the Reader's scheduler
loop, which removes the need from watching over the readBuffer, as
upstream will no longer deliver data before it's actually needed.
A note on the tests:
LargeHandshakeTest.java: is the actual regression test for
this fix - which will fail without
the fix.
It contains a certificate large enough to trigger the old
overflow mechansim during the handshake.
LargeResponseTest.java: is a test that triggers the old
overflow mechanism while handling
response data. This never failed for
me, but it's good to have a regression
test for that too.
HttpSlowServerTest.java: this is just an additional test which
is actually orthogonal to this fix.
I have also verified that this fixes the issue as described
in the bug description, by accessing the external site mentioned.
best regards,
-- daniel
