RFR: JDK-8146975 - NullPointerException in IIOPInputStream.inputClassFields

[Chris Hegarty]

> On 9 Jun 2016, at 15:18, Roger Riggs <Roger.Riggs at oracle.com> wrote:
> 
> Hi Mark,
> 
> IIOPInputStream.java:
> 
> - Does adding doPriv overhead to every field access have a noticeable impact on performance?
>  If most of the fields are accessible or can easily be checked as being in the base module,
>  I could see trying the access directly and catching  the security exception to retry with doPriv.

Maybe something like what is done in 
 sun.security.action.GetPropertyAction.privilegedGetProperty(String)

-Chris.

> 
> Roger
> 
> ps.
> 
> There is a newer version of webrev that provides convenient next and previous links.
> 
> http://hg.openjdk.java.net/code-tools/webrev/raw-file/d26c194772db/webrev.ksh
> 
> 
> 
> On 6/9/2016 8:57 AM, Seán Coffey wrote:
>> Hey Mark,
>> 
>> Looks fine. A few comments.
>> 
>>> 2291                     Class<?> declaredFieldClass = declaredClassField.getType();
>>> 2292
>>> 2293                     if (declaredFieldClass == null) {
>>> 2294                             continue;
>>> 2295                     }
>> I'm not sure if this check is required. Can getType() return null ?
>> 
>> ==
>> 
>>> 2833                         Field classField;
>>> 2834                         classField = c
>>> 2835 .getDeclaredField(fieldName);
>>> 2836                         return classField;
>> Could we fold that code down to 'return c.getDeclaredField(fieldName);' ?
>> 
>> For the tests :
>> ==
>> ConcurrentHashMapTest.java :
>> you might want to remove the old rmiServerProcess.destroy(), orbdProcess.destroyForcibly() comments.
>> ==
>> HelloClient.java
>> line 157 : s/1000/ONE_SECOND
>> 
>> The use of static ports could be one to watch. If we encounter port issues, the test might have to be revisited.
>> 
>> Regards,
>> Sean.
>> 
>> On 08/06/2016 23:18, Mark Sheppard wrote:
>>> 
>>> Hi
>>>   please oblige and review the following changes
>>> http://cr.openjdk.java.net/~msheppar/8146975/jdk9/webrev/
>>> 
>>> http://cr.openjdk.java.net/~msheppar/8146975/jdk9/test/webrev/
>>> 
>>> which address the issue raised in
>>> https://bugs.openjdk.java.net/browse/JDK-8146975
>>> 
>>> the type checking in inputClassFields and other places failed to fully allowing for
>>> the processing of return ValueTypes, and hence the getDeclaredField fails as
>>> "application code" exist  on the call stack restricting access. This leads to a security exception,
>>> which in turn leads to an IllegalArgumentExcetption, the processing of which failed to allow
>>> for a null object value in the stream.
>>> This has now been rectified, with the getDeclaredField wrapped in a doPrivileged call.
>>> 
>>> regards
>>> Mark
>> 
>