RFR: JDK-8146975 - NullPointerException in IIOPInputStream.inputClassFields

[Roger Riggs]

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.

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
>