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

[Frank Yuan]

Hi Daniel


> -----Original Message-----
> From: Daniel Fuchs [mailto:daniel.fuchs at oracle.com]
> Subject: Re: RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests
> 
> 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:

I corrected, cleaned up some code during I addressed the major task, there are also some others besides you found, I even couldn't
recall all of them now. Sorry it confused you...

> ========================================================
> 
>
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/ptests/XMLReaderNSTableTest.java.fram
> es.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.fra
me
> s.html
> 
>
http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/NSTableFTGF.out.frames.h
t
> ml
> 
> 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?
> 

They are unrelated to sm enabling, XMLReaderNSTableTest was not added with @Test, so it was never run, after I added, I had to
correct the golden file to let it passed.
spf.setNamespaceAware(false); is redundant, I will replace with a comment line to state NamespaceAware is false by default.

> 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.
> 
> 

Yes, I will. Forgot yesterday...

> ========================================================
> 
> 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?
> 

The new destination directory which CLITest.java is moved to, contains same files. So don't need to move 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!
> 

Really thank you very much for having reviewed my changes and given me so much valuable comments!

Frank

> 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
> >