RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests

Daniel Fuchs daniel.fuchs at oracle.com
Tue Aug 2 12:30:24 UTC 2016


Hi Frank,

I am intrigued by these change which do not seem
to have anything to do with the rest. I mean, I'm not opposed
as long as they are intended and that Joe validates them:
========================================================

http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/ptests/XMLReaderNSTableTest.java.frames.html

118         spf.setNamespaceAware(false);

http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/EntityResolverGF.out.frames.html

http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/NSTableFTGF.out.frames.html

http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/publish.xml.frames.html
This looks like a desirable change - but what does it have to do
with enabling security manager?

Also:
========================================================

http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/unittest/transform/XSLTFunctionsTest.java.frames.html

   70 
tf.setFeature("http://www.oracle.com/xml/jaxp/properties/enableExtensionFunctions", 
true);

Is this needed only when there is a security manager?

========================================================

http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java.html


  119         /*
  120         addPermission(new RuntimePermission("getClassLoader"));
  121         addPermission(new RuntimePermission("createClassLoader"));
  122         addPermission(new RuntimePermission("createSecurityManager"));
  123         addPermission(new RuntimePermission("modifyThread"));
  124         addPermission(new PropertyPermission("*", "read,write"));
  125
  126         addPermission(new RuntimePermission("setIO"));
  127         addPermission(new RuntimePermission("setContextClassLoader"));
  128         addPermission(new 
RuntimePermission("accessDeclaredMembers"));*/


Please remove before pushing.


========================================================

test/javax/xml/jaxp/internaltest/javax/xml/transform/cli/tigertest-in.xml
test/javax/xml/jaxp/internaltest/javax/xml/transform/cli/tigertest.xsl

It seems these two files have been removed, but shouldn't they have
been moved to test/javax/xml/jaxp/internaltest/javax/xml/transform
instead?
I see they are used by 
test/javax/xml/jaxp/internaltest/javax/xml/transform/CLITest.java

Did you forget to hg add them?

========================================================


Otherwise the new JAXPPolicyManager & its Policy implementation
look good. This is much simpler and better than the first
iteration :-)

This was a *very* long patch - so congratulations for seeing
this through!

cheers,

-- daniel


On 02/08/16 11:20, Frank Yuan wrote:
> Hi Joe and Daniel
>
> I have finished the rework as your comments, please check http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/
>
> JAXP tests use Policy classes, as well as 3 other patterns provided by JAXPTestUtilities:
> 1. runWithAllPerm methods, are only used for user setup code, never run jaxp code with them.
> 2. tryRunWithPolicyManager method, is used to run some test methods with security manager and run the others without security
> manager in single test class
> 3. runWithTmpPermission methods, which may run jaxp code with some limited permissions, those permissions belong to user permissions
> and jaxp code won't doPrivileged for them. E.g. PropertyPermission("MyInputFactory", "read") or
> FilePermission("/tmp/this/does/not/exist/but/that/is/ok", "read")
>
>
> Btw, some tests or test methods are disabled or commented sm support temporarily for some known jaxp security bugs.
>
> Thanks
> Frank
>



More information about the core-libs-dev mailing list