RFR: 8155039: Simplify code to setup SSLContextImpl and TrustManagerFactoryImpl

Mandy Chung mandy.chung at oracle.com
Fri Jun 10 02:32:40 UTC 2016


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 ?

Mandy

> On Jun 9, 2016, at 5:31 PM, Claes Redestad <claes.redestad at 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
>>>> 
>> 




More information about the security-dev mailing list