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

[Joe Wang]

other than that previously discussed,

in
test/javax/xml/jaxp/functional/javax/xml/parsers/ptests/DocumentBuilderFactoryTest.java
the comments related to the removed code can be removed as well

test/javax/xml/jaxp/functional/javax/xml/parsers/ptests/SAXParserTest.java

@Test(expectedExceptions = SAXException.class, dataProvider = 
"parser-provider", enabled = false) //disabled for 8162848
     -- just a reminder, as we discussed, the test needs to be enabled, 
and 8162848 closed (also one in 
test/javax/xml/jaxp/unittest/common/Sources.java)

what are runWithSecurityManager and runWithoutSecurityManager in some of 
the tests? I thought the whole test suite will be run with and without 
security manager, isn't that the plan?

Once again, great work and nice to clean up the old security-testing code.

Best,
Joe

On 8/2/16, 2:56 PM, Joe Wang wrote:
>
>
> On 8/2/16, 5:30 AM, Daniel Fuchs wrote:
>> 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);
>
> Not sure why this was changed.
>>
>> http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/EntityResolverGF.out.frames.html 
>>
>
> The test itself wasn't very clear on the content it tests. If it only 
> verifies whether a resolver is used as is said, then this and related 
> changes in ResolverTest and publish.xml are fine. To the extent it 
> verifies, it didn't have to output to a file.
>>
>> http://cr.openjdk.java.net/~fyuan/8067170/webrev.02/test/javax/xml/jaxp/functional/org/xml/sax/xmlfiles/out/NSTableFTGF.out.frames.html 
>>
>
> We have to let Frank explain why namespace was turned off for these 
> tests :-)  In this case, XMLReaderNSTableTest. In general, I would say 
> enabling security shouldn't change tests or gold files in this case.
>
>>
>> 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?
>
> Probably to remove the reference to that particular server?  Seems to 
> be fine as a cleanup. Again, it (the original test) only verifies a 
> resolve is used, it didn't even need to write out a file to prove that.
>>
>> 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?
>
> Yes, when Security Manager is present, this feature is turned off by 
> default.
>>
>> ========================================================
>>
>> 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!
>
> Very long patch, indeed! Thanks for the great effort!  Very well done 
> with the unified Policy/Permission management.
>
> Best,
> Joe
>
>>
>> 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
>>>
>>