hengsokchamroeun / javapns

Automatically exported from code.google.com/p/javapns
0 stars 0 forks source link

Proxy settings impacting other running processes in JVM #91

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1.Set up JavaPNS to use proxy

What is the expected output? What do you see instead?
I would have hoped that the requesting the use of the proxy would only impact 
the JavaPNS process.  Because you are setting the system properties for 
https.proxyHost and https.proxyPort, that is in effect for other processes that 
are run in the JVM as well.

What version of the product are you using? On what operating system?
v2.1 on Windows XP.

Please provide any additional information below.
There are numerous ways this could be handled.  A couple that come immediately 
to mind are: 
1) Keep the setting of the proxy host and proxy port internal to JavaPNS by 
setting internal variables with the value and not system properties.
2) Set JavaPNS-specific system properties that only JavaPNS will retrieve (e.g. 
"javapns.https.proxyHost").

Original issue reported on code.google.com by petend...@gmail.com on 7 Dec 2011 at 4:37

GoogleCodeExporter commented 8 years ago

Original comment by sype...@gmail.com on 7 Dec 2011 at 4:44

GoogleCodeExporter commented 8 years ago
Fixed in r342:  proxy can now be set on AppleNotificationServer and 
AppleFeedbackServer directly to avoid impacting other JVM processes.

Original comment by sype...@gmail.com on 13 Dec 2011 at 2:48

GoogleCodeExporter commented 8 years ago
I was just reviewing the changes that you committed for the setting of the 
proxy.  Thanks for accommodating this request.  

One final suggestion related to this (or perhaps you have another suggestion I 
can use instead - ;)).  The proxy setting is not available via the Push static 
methods.  I can see the code that you have there for, say, Push.payloads().  
Currently, the only way that I can figure to do the work that method does while 
also setting a proxy is to copy your code into my class and add the line to 
call setProxy() in the appropriate place.

That's not a huge deal, I can certainly do that.  Is that your recommendation?  
Another option might be to overload or add setting the proxy to the required 
Push methods.

Thanks again for seeing the value in my original request.

Original comment by petend...@gmail.com on 15 Dec 2011 at 4:19

GoogleCodeExporter commented 8 years ago
Indeed, upon reflection, the solution isn't perfect yet.  Maybe after all that 
the PushNotificationManager.setProxy method should be un-deprecated and 
modified to set library-specific properties instead of the JVM-wide ones, as 
you suggested initially.  I'll do the modifications shortly...

Re-opening issue because fix should be improved.

Original comment by sype...@gmail.com on 15 Dec 2011 at 4:43

GoogleCodeExporter commented 8 years ago
I have just committed r344 which includes a new (and I believe more intuitive) 
way of configuring proxies.  To set a proxy for the entire library, invoke 
javapns.communication.ProxyManager.setProxy(host, port);   A proxy can still be 
configured for specific AppleServers, or for the entire JVM, but the preferred 
way would be to set it for the entire library with this method.

Could you let me know if it meets your needs and expectations, so this issue 
can be closed?

Thank you!

Original comment by sype...@gmail.com on 16 Dec 2011 at 4:36

GoogleCodeExporter commented 8 years ago
Hi Sylvain,

I just reviewed the changes and I agree that it is much more intuitive.  I like 
all 3 of the options provided (JavaPNS, JVM and server), and the order that you 
are checking them is, I believe, appropriate.  I'm at home today so I won't be 
able to make the changes to my program and test them until Monday, but at first 
glance, I like it.  :)

Assuming everything runs fine in my test, I'll provide you a final comment on 
Monday.

Thanks.

Original comment by petend...@gmail.com on 16 Dec 2011 at 8:56

GoogleCodeExporter commented 8 years ago
I just tested out the changes related to ProxyManager and they are working 
great and not impacting other processes in the JVM.  Thanks again for the quick 
turnaround.

Original comment by petend...@gmail.com on 20 Dec 2011 at 3:26

GoogleCodeExporter commented 8 years ago
Awesome!  Thank you for the quick feedback!

Closing as fixed.

Original comment by sype...@gmail.com on 20 Dec 2011 at 3:42