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


Reply via email to