Re: [9] RFR: 8075297: Tests for RFEs 4515853 and 4745056
>> > I think it it enough to add a "max_retries = 1" to [libdefaults] of > krb5.conf, and left default timeout value for the test. Please take a look at > updated webrev: > > http://cr.openjdk.java.net/~asmotrak/8075297/webrev.02/ Can you also add the same line to the krb5.conf for the other test? Thanks Max
Re: [9] RFR: 8075297: Tests for RFEs 4515853 and 4745056
Hi Max, On 07/16/2015 06:19 PM, Weijun Wang wrote: One final point. You have the timeout set to 5 minutes in + * @run main/othervm/timeout=300 BogusKDC Did you observe the test spending a lot of time? A kerberos client is designed to timeout after 30 seconds if there is no reply from a KDC but sometimes it could be very fast if the KDC does not exist (i.e. not listening on the port at all). Sometimes it could be even slower if the client believes the KDC is there and it will retry for 3 times. This can also be platform dependent, for example, on Mac there is no PortUnreachableException. Yes, I noticed that sometimes a client may do 3 attempts, and default timeout is 30 seconds. That's why I increased timeout for this test. I suggest you add a "max_retries = 1" to [libdefaults] of krb5.conf in case the worst thing happens. You can also add "kdc_timeout = 10s". Don't make it too short, some embedded Linux are very slow. I think it it enough to add a "max_retries = 1" to [libdefaults] of krb5.conf, and left default timeout value for the test. Please take a look at updated webrev: http://cr.openjdk.java.net/~asmotrak/8075297/webrev.02/ I am trying to drag the review process longer so you can push it yourself. :-) It looks like my account has been updated, so probably I can push it by myself now :-) http://openjdk.java.net/census#asmotrak Artem Thanks Max On 07/17/2015 05:05 AM, Artem Smotrakov wrote: Hi Max, Good point, please see an updated webrev: http://cr.openjdk.java.net/~asmotrak/8075297/webrev.01/ Artem
Re: RfR JDK-8051626 Rework security restrictions of Java Access Bridge and related Utilities
>From Mandy: - remove unused imports - add @run main/othervm http://cr.openjdk.java.net/~ptbrunet/JDK-8051626/webrev.02/ On 7/15/15 4:42 PM, Pete Brunet wrote: > An update is available and mostly changes only the test case, > Bug8151626.java. The other change is to remove jtreg.security.policy. > > http://cr.openjdk.java.net/~ptbrunet/JDK-8051626/webrev.01/ > > Changes: > > From Sean > - The jtreg @run statement was removed; don't specify security manager > or security policy. > - Remove jtreg.security.policy > - Add System.setSecurityManager(new SecurityManager()); to the beginning > of the code. > > From Sergey > - Wrap test in invokeAndWait > - Add frame.dispose in finally > - Remove System.exit > > I also removed the Thread.sleep. It doesn't appear to be needed. > > Pete > > On 7/14/15 5:00 AM, Sergey Bylokhov wrote: >> Hi, Pete. >> The fix looks fine, but you should tweak the test a little bit. >> - You access the swing components on non-EDT thread. >> - You should not use System.exit in the test. >> - The JFrame should be disposed before the end of the test. >> >> On 14.07.15 1:34, Pete Brunet wrote: >>> Please review the webrev for >>> https://bugs.openjdk.java.net/browse/JDK-8051626 >>> >>> http://cr.openjdk.java.net/~ptbrunet/JDK-8051626/webrev.00/ >>> >>> This is so the the Java Accessibility Utilities package, >>> com.sun.java.accessibility.util, can be run with the security manager >>> active but the non-public accessibility packages won't, i.e. >>> com.sun.java.accessibility.internal and >>> com.sun.java.accessibility.util.internal. >>> >>> Running the regression test proves that there will be a security >>> exception when using a method of com.sun.java.accessibility.util before >>> the fix but not after. >>> >>> Pete >>
Re: RfR JDK-8051626 Rework security restrictions of Java Access Bridge and related Utilities
The fix looks fine. On 16.07.15 0:42, Pete Brunet wrote: An update is available and mostly changes only the test case, Bug8151626.java. The other change is to remove jtreg.security.policy. http://cr.openjdk.java.net/~ptbrunet/JDK-8051626/webrev.01/ Changes: From Sean - The jtreg @run statement was removed; don't specify security manager or security policy. - Remove jtreg.security.policy - Add System.setSecurityManager(new SecurityManager()); to the beginning of the code. From Sergey - Wrap test in invokeAndWait - Add frame.dispose in finally - Remove System.exit I also removed the Thread.sleep. It doesn't appear to be needed. Pete On 7/14/15 5:00 AM, Sergey Bylokhov wrote: Hi, Pete. The fix looks fine, but you should tweak the test a little bit. - You access the swing components on non-EDT thread. - You should not use System.exit in the test. - The JFrame should be disposed before the end of the test. On 14.07.15 1:34, Pete Brunet wrote: Please review the webrev for https://bugs.openjdk.java.net/browse/JDK-8051626 http://cr.openjdk.java.net/~ptbrunet/JDK-8051626/webrev.00/ This is so the the Java Accessibility Utilities package, com.sun.java.accessibility.util, can be run with the security manager active but the non-public accessibility packages won't, i.e. com.sun.java.accessibility.internal and com.sun.java.accessibility.util.internal. Running the regression test proves that there will be a security exception when using a method of com.sun.java.accessibility.util before the fix but not after. Pete -- Best regards, Sergey.
Re: [9] RFR: 8075297: Tests for RFEs 4515853 and 4745056
One final point. You have the timeout set to 5 minutes in + * @run main/othervm/timeout=300 BogusKDC Did you observe the test spending a lot of time? A kerberos client is designed to timeout after 30 seconds if there is no reply from a KDC but sometimes it could be very fast if the KDC does not exist (i.e. not listening on the port at all). Sometimes it could be even slower if the client believes the KDC is there and it will retry for 3 times. This can also be platform dependent, for example, on Mac there is no PortUnreachableException. I suggest you add a "max_retries = 1" to [libdefaults] of krb5.conf in case the worst thing happens. You can also add "kdc_timeout = 10s". Don't make it too short, some embedded Linux are very slow. I am trying to drag the review process longer so you can push it yourself. :-) Thanks Max On 07/17/2015 05:05 AM, Artem Smotrakov wrote: Hi Max, Good point, please see an updated webrev: http://cr.openjdk.java.net/~asmotrak/8075297/webrev.01/ Artem
Re: RFR: (XS) : 8131665: Bad exception message in HandshakeHash.getFinishedHash
Looks fine to me.
Thanks,
Xuelei
On 7/17/2015 1:02 AM, Seán Coffey wrote:
> Bug report : https://bugs.openjdk.java.net/browse/JDK-8131665
>
> Simple fix to improve the Error message for this scenario.
>
> diff --git
> a/src/java.base/share/classes/sun/security/ssl/HandshakeHash.java
> b/src/java.base/share/classes/sun/security/ssl/HandshakeHash.java
> --- a/src/java.base/share/classes/sun/security/ssl/HandshakeHash.java
> +++ b/src/java.base/share/classes/sun/security/ssl/HandshakeHash.java
> @@ -355,7 +355,7 @@
> try {
> return cloneDigest(finMD).digest();
> } catch (Exception e) {
> -throw new Error("BAD");
> +throw new Error("Error during hash calculation", e);
> }
> }
> }
>
Re: [9] RFR: 8075297: Tests for RFEs 4515853 and 4745056
Hi Max, Good point, please see an updated webrev: http://cr.openjdk.java.net/~asmotrak/8075297/webrev.01/ Artem On 07/16/2015 12:57 AM, Weijun Wang wrote: The logic in RefreshKrb5Config.java is very good. Why not also try NotRefreshable again after writing the correct krb5.conf in BogusKDC.java? Thanks Max On 07/16/2015 01:38 PM, Artem Smotrakov wrote: Hello, Please review a couple of new tests that check that: - Kerberos client tries slave KDC if master KDC is not responding - if refreshKrb5Config is set to true for Krb5LoginModule, then configuration will be refreshed before login() method is called KDC.java registers a name service which returns 127.0.0.1 for all hosts. The name service was updated to throw UnknownHostException for "not.existing.host". Webrev: http://cr.openjdk.java.net/~asmotrak/8075297/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8075297 Artem
Re: [9] RFR: 8129306: Some new tests developed for JDK-8085979 fail in jdk9/cpu
Looks fine to me. > On 16 Jul 2015, at 19:08, Konstantin Shefov > wrote: > > Hello, > > Please review a test bug fix. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8129306 > Webrev: http://cr.openjdk.java.net/~kshefov/8129306/webrev.00/ > > -Konstantin
[9] RFR: 8129306: Some new tests developed for JDK-8085979 fail in jdk9/cpu
Hello, Please review a test bug fix. Bug: https://bugs.openjdk.java.net/browse/JDK-8129306 Webrev: http://cr.openjdk.java.net/~kshefov/8129306/webrev.00/ -Konstantin
RFR: (XS) : 8131665: Bad exception message in HandshakeHash.getFinishedHash
Bug report : https://bugs.openjdk.java.net/browse/JDK-8131665
Simple fix to improve the Error message for this scenario.
diff --git
a/src/java.base/share/classes/sun/security/ssl/HandshakeHash.java
b/src/java.base/share/classes/sun/security/ssl/HandshakeHash.java
--- a/src/java.base/share/classes/sun/security/ssl/HandshakeHash.java
+++ b/src/java.base/share/classes/sun/security/ssl/HandshakeHash.java
@@ -355,7 +355,7 @@
try {
return cloneDigest(finMD).digest();
} catch (Exception e) {
-throw new Error("BAD");
+throw new Error("Error during hash calculation", e);
}
}
}
--
Regards,
Sean.
Re: [9] RFR: 8074784: Additional tests for XML DSig API
Hi Artem, The updated test looks good. Thanks, Sean On 06/26/2015 01:57 PM, Artem Smotrakov wrote: Hi Sean, I added new test cases to GenerationTests, please take a look: http://cr.openjdk.java.net/~asmotrak/8074784/webrev.01/ Artem On 06/16/2015 07:23 PM, Sean Mullan wrote: On 05/20/2015 04:16 PM, Artem Smotrakov wrote: Hi Sean, Yes, at first, I thought about updating the existing tests in test/javax/xml/crypto/dsig directory. But then I noticed that both GenerationTests and ValidationTests has ~30 test cases. And new Detached.java test contains >30 test cases. If one of test cases fails, JTREG will show that full test failed. As a result, it may hide failures of other test cases (an engineer should look at logs carefully). You could always change that to continue on error and run all tests before reporting a failure. That was just originally the way the test was created, it isn't set in stone. Also it may be better to split tests if possible when some tools for automated failures analysis is used (for example, Java SQE uses such a tool). That was the main reason why I added a separate test. Not sure if performance may be an issue, I have not done any measurement. Yes, but there is still a lot of common infrastructure in this test and ValidationTests that could be combined. You have made use of lambdas and streams to create a nice way to structure and run the different variants of tests. But a detached signature is really just another variant, and I think your test could be easily adapted to also test enveloped and enveloping signatures, as well as the other cases in ValidationTests. So, it would be nice to combine these tests. I would be ok with filing a follow-on issue to do that, though. I noticed a few typos: 151: s/fot/for 332: s/transfrom/transform/ 418: s/validaition/validation/ I also would recommend reordering the imports in alphabetical order. Looks fine otherwise. Thanks, Sean Artem On 05/20/2015 10:52 PM, Sean Mullan wrote: Hi Artem, Is there a reason this needs to be a separate test? It seems like it would be better to fold it into the existing GenerationTests and ValidationTests in the test/javax/xml/crypto/dsig directory, so you could reuse common code. Thanks, Sean On 05/12/2015 11:32 AM, Artem Smotrakov wrote: Hello, Please review a new test for generating and validation of detached XML digital signatures. Bug: https://bugs.openjdk.java.net/browse/JDK-8074784 Webrev: http://cr.openjdk.java.net/~asmotrak/8074784/webrev.00/ Artem
RFR 8131350: policytool can directly reference permission classes
Hi All Please take a look at http://cr.openjdk.java.net/~weijun/8131350/webrev.00/ Policytool contains a lot of hard coded strings for permission class names, which can be changed to actual Class types. That allows checking at compile-time rather than failing at run-time if any name had a typo. Noreg-cleanup. Thanks Max
Re: [9] RFR: 8075297: Tests for RFEs 4515853 and 4745056
The logic in RefreshKrb5Config.java is very good. Why not also try NotRefreshable again after writing the correct krb5.conf in BogusKDC.java? Thanks Max On 07/16/2015 01:38 PM, Artem Smotrakov wrote: Hello, Please review a couple of new tests that check that: - Kerberos client tries slave KDC if master KDC is not responding - if refreshKrb5Config is set to true for Krb5LoginModule, then configuration will be refreshed before login() method is called KDC.java registers a name service which returns 127.0.0.1 for all hosts. The name service was updated to throw UnknownHostException for "not.existing.host". Webrev: http://cr.openjdk.java.net/~asmotrak/8075297/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8075297 Artem
