Hi Daniel
> -----Original Message----- > From: Daniel Fuchs [mailto:daniel.fu...@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 > >
