amzn / amazon-pay-sdk-java

Amazon Pay Java SDK
https://pay.amazon.com/documentation
Apache License 2.0
58 stars 51 forks source link

Using PayConfig.setProxyHost() leads to corruption of JVM property "http.proxyPort" #32

Open mjantscher-netconomy opened 5 years ago

mjantscher-netconomy commented 5 years ago

Amazon Pay Java SDK version: 3.5.1

Looking at Util.java - line 186 we can see that the JVM property http.proxyPort is set everytime a) httpSendRequest(String method, ....., PayConfig config) is called and b) config.getProxyHost() on line 182 returns a reference to an object.

Let's take a closer look at line 186:

  1. config.getProxyPort() returns the port number as a value of the primitive type int.
  2. systemSettings.put() is called for setting the property http.proxyPort. According to the Oracle Java 8 Docs doing so "is strongly discouraged as they allow the caller to insert entries whose keys or values are not Strings. The setProperty method should be used instead."

The undesired consequences of using put() in combination with non-String values become clearly visible if we have a look at an actual implementation of Java's Properties.getProperty() method (e.g. the OpenJDK 8 implementation). We can see on line 970 that every property value that is not a String instance will become null. On line 971 we see that a default value might be returned instead of null in case defaults provides one.

Its obvious now that Properties.getProperty("http.proxyPort") for the whole JVM will return an undesired value of null (or some default value) if we set the proxy port number as an int value instead of a String.

Quick (but dirty) fix proposal

As a quick workaround we rewrote Util.java - line 186 to the following: systemSettings.put("http.proxyPort", Integer.valueOf(config.getProxyPort()).toString());

The benefit of this fix is that it introduces no API changes. The downside of this fix is that we still don't comply with Oracle's recommendation regarding setting properties.

Other fix proposals

bjguillot commented 5 years ago

Thanks for bringing this to our attention. We'll evaluate the different approaches outlined and see what makes the most sense. Are you currently making use of the proxy functionality of the SDK?

mjantscher-netconomy commented 5 years ago

After applying the quick fix, yes.

Using the proxy functionality before the fix prevented connections from our application to other HTTP-based services via proxy from being established.