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

Reply via email to