On 06/09/2016 10:32 PM, Mandy Chung wrote:
Hi Claes,
I don’t like the PropertiesWrapper idea. The caller should be
cautious in storing any sensitive information. For the system
properties, these callsites use it in the local scope that I don’t
see any reason and benefit to introduce a wrapper. I didn’t follow
this discussion closely and I may miss some reason ?
The original code used multiple calls to System.getProperty wrapped in a
doPrivileged. Claes' first iteration of the fix changed this to use a
GetPropertyAction.privilegedGetProperties method that returned a
Properties object. I expressed a concern that this was now exposing an
object that, if accidentally leaked to untrusted code could cause much
more damage than the original code (since the code would be able to
set/get/remove *any* system property). Hence the current fix which uses
a wrapper class which is not exported.
--Sean
Mandy
On Jun 9, 2016, at 5:31 PM, Claes Redestad
<claes.redes...@oracle.com> wrote:
Hi,
by using a non-exported type, PropertiesWrapper, to encapsulate
Properties this change makes it impossible for a JDK developer to
accidentally leak system properties outside of java.base without
breaking encapsulation. I believe that was yours and Sean's main
concern about the API changes to GetPropertyAction that this is
hopefully a final amendment to.
Generally the changes to streamline system property access bring
about minor improvements to startup and footprint by reducing the
number of classes generated and loaded as well as the number of
doPriv calls done.
/Claes
On 2016-06-08 03:24, Xuelei Fan wrote:
Hi Claes,
What's the necessary to get all system properties for just one
specific one? Can you explain more about the necessary to make
the change?
Thanks, Xuelei
On 6/8/2016 3:44 AM, Claes Redestad wrote:
Hi,
there is some lingering concern that this and related changes
make it that much easier to accidentally leak the system
Properties object outside of core modules. By wrapping access
to the system Properties object in a class residing in a
non-exported package we disallow this at little to no cost:
http://cr.openjdk.java.net/~redestad/8155039/webrev.02/
/Claes
On 2016-05-12 15:37, Claes Redestad wrote:
Hi,
the API this improvement depends on was updated to reflect
more clearly that this is taking a privileged action:
https://bugs.openjdk.java.net/browse/JDK-8155775
Here's the updated webrev:
http://cr.openjdk.java.net/~redestad/8155039/webrev.01/
Thanks!
/Claes
On 2016-04-25 19:28, Claes Redestad wrote:
Hi,
SSLContextImpl and TrustManagerFactoryImpl has some setup
code that could be simplified, getting rid of a couple of
anonymous classes in the process.
Webrev:
http://cr.openjdk.java.net/~redestad/8155039/webrev.00 Bug:
https://bugs.openjdk.java.net/browse/JDK-8155039
Alternatively we could remove OpenFileInputStreamAction
instead since these two uses introduced here are actually
the only active uses of this old convenience class.
Thanks!
/Claes
