Merge pull request #943 from da-als/master

[dig-animati]

Description

This pull request includes a fix developed by @doyledavidson where any data located after the end of a SSL handshake is saved to be returned on the next read operation. This way, the data is not lost and connections from clients that append data at the end of the handshake do not have trouble communicating with Java-WebSocket.

Related Issue

Fixes #665
Fixes #940

Motivation and Context

Some clients, when connecting to a WSS server, use TLS False Start, which causes initial data to be appended to the SSL handshake. That data gets discarded by the Java-WebSocket server which often causes communication problems, even causing the connection to become unresponsive for the client as it waits for a reply from the server.

How Has This Been Tested?

I use the Java-WebSocket library on an application that acts as a local server on hundreds of clients that access this server functionality from a regular web browser. Clients that are using browsers that use TLS False Start often required multiple connection attempts to work around the issue, and even that, in some cases, was not enough. I have deployed my fork of this library with a new beta of my application on about a dozen clients for testing; they are not running into the issue anymore (connections work on the first try) and no regressions have been reported.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

Once again, the code was added by @doyledavidson, not myself. I am not familiar with the process for running the tests, but since it affects new connections' handshakes and their first read operations I would assume the changes would be fully covered by existing tests.

[PhilipRoman]

Looks good to me, although I wasn't able to test this.

[marci4]

@PhilipRoman thx, I will try it when I have some time on my hands.

[marci4]

I had some problems running this.
The autobahn testsuite failed a few times for me.
Anyone else tried it?

[dig-animati]

I had some problems running this.
The autobahn testsuite failed a few times for me.
Anyone else tried it?

After your report I tried running all the tests in the project. The only test that failed for me was "Issue256Test", but this one also fails without the changes from the merge (it fails on the master branch of the main project) so it has no relation to this pull request.

[marci4]

@da-als I was running it against the big websocket autobahn test suite, which is not part of the normal unit test.

Currently, really busy, so I did not have more time to take a look at this sorry..

[jiangjianping]

@da-als
thank you very much. This almost reosolved my issue #958

[marci4]

I havent forgotten this PR, currently just very busy...

[marci4]

Test look good

[marci4]

@da-als thx a lot for your contribution!

[dig-animati]

@marci4 Happy to help! Glad to see the fix merged.

[doyledavidson]

Thank you everyone for getting that pulled in!

[doyledavidson]

Any plan to push a new version (1.4.1?) to Maven repo?

[marci4]

@doyledavidson a snapshot should be available