Skip to content

Conversation

@whizzosoftware
Copy link

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

@Lokomojo
Copy link

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.

@Lokomojo
Copy link

I've fixed up a couple of small things to get it working well in my environment. Thanks very much for your work!

@whizzosoftware
Copy link
Author

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 notifications@github.com wrote:

I've fixed up a couple of small things to get it working well in my environment. Thanks very much for your work!


Reply to this email directly or view it on GitHub.

@Lokomojo
Copy link

Looks like you've found them :) but these are the relevant commits:

Lokomojo@0105269

Lokomojo@4d19073

Lokomojo@f3f7dc7

@Lokomojo
Copy link

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).

@whizzosoftware
Copy link
Author

I notice the pull request and pulled them in. I did make one change to your
code -- I pulled the port number (80 vs. 443) check into a method so it
wasn't duplicated in two places.

Thanks again for working on it!

On Fri, May 30, 2014 at 8:45 AM, Tom Mettam notifications@github.com
wrote:

Looks like you've found them :) but these are the relevant commits:

Lokomojo/eventsource-java@0105269
Lokomojo@0105269

Lokomojo/eventsource-java@4d19073
Lokomojo@4d19073

Lokomojo/eventsource-java@f3f7dc7
Lokomojo@f3f7dc7


Reply to this email directly or view it on GitHub
#3 (comment)
.

Daniel Noguerol, Owner
Whizzo Software LLC http://www.whizzosoftware.com/
Office: (888) 212-4941
Mobile: (303) 570-3113

@whizzosoftware
Copy link
Author

I had noticed that happen occasionally and it seemed to involve the SSL
piece (from what I saw). I was going to look into next week.

On Fri, May 30, 2014 at 9:17 AM, Tom Mettam notifications@github.com
wrote:

It seems that there's still an issue that causes it to not reliably
reconnect in some conditions. (I'm running on Android),

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).


Reply to this email directly or view it on GitHub
#3 (comment)
.

Daniel Noguerol, Owner
Whizzo Software LLC http://www.whizzosoftware.com/
Office: (888) 212-4941
Mobile: (303) 570-3113

@Lokomojo
Copy link

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.

@Lokomojo
Copy link

I've added some commits which repair the issue.

@whizzosoftware
Copy link
Author

Awesome! Thanks, Tom! I’ll take a look at what you did.

->Dan

On May 31, 2014, at 9:57 AM, Tom Mettam notifications@github.com wrote:

I've added some commits which repair the issue.


Reply to this email directly or view it on GitHub.

@whizzosoftware
Copy link
Author

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 notifications@github.com wrote:

I've added some commits which repair the issue.


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants