RFR (JAXP) JDK-8067170: Enable security manager on JAXP unit tests
Frank Yuan
frank.yuan at oracle.com
Wed Aug 3 03:21:23 UTC 2016
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
> >
More information about the core-libs-dev
mailing list