eventsource-java icon indicating copy to clipboard operation
eventsource-java copied to clipboard

Changes for SSL support and custom HTTP request headers

Open whizzosoftware opened this issue 11 years ago • 11 comments

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

whizzosoftware avatar May 15 '14 15:05 whizzosoftware

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 avatar May 30 '14 12:05 Lokomojo

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

Lokomojo avatar May 30 '14 13:05 Lokomojo

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:

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.

whizzosoftware avatar May 30 '14 14:05 whizzosoftware

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

https://github.com/TomMettam/eventsource-java/commit/0105269c772ddf732e37605ea6da8ad2ee6d1b77

https://github.com/TomMettam/eventsource-java/commit/4d1907316ee0440622b3624a67b34f7589757f56

https://github.com/TomMettam/eventsource-java/commit/f3f7dc7c54f47af47ad1959ef0f7813c1fc003e5

Lokomojo avatar May 30 '14 14:05 Lokomojo

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

Lokomojo avatar May 30 '14 15:05 Lokomojo

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 [email protected] wrote:

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

TomMettam@0105269 https://github.com/TomMettam/eventsource-java/commit/0105269c772ddf732e37605ea6da8ad2ee6d1b77

TomMettam@4d19073 https://github.com/TomMettam/eventsource-java/commit/4d1907316ee0440622b3624a67b34f7589757f56

TomMettam@f3f7dc7 https://github.com/TomMettam/eventsource-java/commit/f3f7dc7c54f47af47ad1959ef0f7813c1fc003e5

— Reply to this email directly or view it on GitHub https://github.com/aslakhellesoy/eventsource-java/pull/3#issuecomment-44658789 .

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

whizzosoftware avatar May 30 '14 18:05 whizzosoftware

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 [email protected] 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 https://github.com/aslakhellesoy/eventsource-java/pull/3#issuecomment-44662504 .

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

whizzosoftware avatar May 30 '14 18:05 whizzosoftware

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 avatar May 31 '14 15:05 Lokomojo

I've added some commits which repair the issue.

Lokomojo avatar May 31 '14 15:05 Lokomojo

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:

I've added some commits which repair the issue.

— Reply to this email directly or view it on GitHub.

whizzosoftware avatar Jun 01 '14 00:06 whizzosoftware

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:

I've added some commits which repair the issue.

— Reply to this email directly or view it on GitHub.

whizzosoftware avatar Jun 01 '14 01:06 whizzosoftware