-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes for SSL support and custom HTTP request headers #3
base: master
Are you sure you want to change the base?
Conversation
…quest headers. Added EventSource state updates for OPEN and CLOSED.
I tried your changes - unfortunately the SSL stuff appears not to work correctly, at least in my environment. I'm going to try to debug. |
I've fixed up a couple of small things to get it working well in my environment. Thanks very much for your work! |
Glad you got it working! I’d love to know what changes you had to make :-) On May 30, 2014, at 7:30 AM, Tom Mettam [email protected] wrote:
|
…dropped temporarily
… unresolved address condition
Looks like you've found them :) but these are the relevant commits: |
It seems that there's still an issue that causes it to not reliably reconnect in some conditions. (I'm running on Android), (Note that this isn't specific to SSL) I'm going to do some more test work and perhaps add some watches to ensure that we're always trying to connect (until closed). |
I notice the pull request and pulled them in. I did make one change to your Thanks again for working on it! On Fri, May 30, 2014 at 8:45 AM, Tom Mettam [email protected]
Daniel Noguerol, Owner |
I had noticed that happen occasionally and it seemed to involve the SSL On Fri, May 30, 2014 at 9:17 AM, Tom Mettam [email protected]
Daniel Noguerol, Owner |
I found the SSL reconnection issue. The issue is unfortunately with your design of allowing the client to provide the SSLEngine. When the SSLEngine has been used once, it cannot be initialised again - a new SSLEngine must be created. I'm going to work around this by adding an SSLEngineFactory class which the user can then overload to provide their own SSLEngine if needed. |
I've added some commits which repair the issue. |
Awesome! Thanks, Tom! I’ll take a look at what you did. ->Dan On May 31, 2014, at 9:57 AM, Tom Mettam [email protected] wrote:
|
…ine on reconnects. Thanks Tom for finding the bug!
Thanks again for troubleshooting this. I took a slightly different approach and created an SSLProvider interface so the client of event-source still has full control of creating the SSLEngine but otherwise I made the same changes you did. ->Dan On May 31, 2014, at 9:57 AM, Tom Mettam [email protected] wrote:
|
…alled previously)
Hi,
Thanks so much for making your Java EventSource implementation open source. I've made a few changes that I thought you might be interested in pulling into your source tree. Please ignore the changes in pom.xml -- those are specific to my build environment.
->Dan