Code review request, 8025415, Test SSLSocketImplThrowsWrongExceptions.java timed out
Hi Weijun, Another simple test stabilization fix. webrev: http://cr.openjdk.java.net/~xuelei/8025415/webrev.00/ This intermittent failure may caused by that server may also throw exception, which cannot be caught with current client-server test template. This fix is trying to use the new SSL socket test template (test/sun/security/ssl/templates/SSLSocketTemplate.java), which is more reliable. Not too much effort, just copy/past, and use the new test template. Thanks, Xuelei
Code Review request: 8028431: NullPointerException in DerValue.equals(DerValue)
Hi, please review this fix for 9: https://bugs.openjdk.java.net/browse/JDK-8028431 http://cr.openjdk.java.net/~asmotrak/8028431/webrev.00/ http://cr.openjdk.java.net/%7Easmotrak/8028431/webrev.00/ sun.security.util.DerValue.equals(DerValue) method does not check that null is passed. As a result, NullPointerException can occur. Artem
Re: Code Review request: 8028431: NullPointerException in DerValue.equals(DerValue)
Hello Artem, You fix looks good. You just need to fill in the missing portion of the copyright in the test. You could also adjust the copyright year range at the start of DerValue.java. Also I would add the test to the existing test/java/security/cert/X509Certificate/ directory rather than create a new one. Finally, I think the test should run fine without the jtreg tag for 'othervm'. Thanks. On 20/12/2013 12:51, Artem Smotrakov wrote: Hi, please review this fix for 9: https://bugs.openjdk.java.net/browse/JDK-8028431 http://cr.openjdk.java.net/~asmotrak/8028431/webrev.00/ http://cr.openjdk.java.net/%7Easmotrak/8028431/webrev.00/ sun.security.util.DerValue.equals(DerValue) method does not check that null is passed. As a result, NullPointerException can occur. Artem
Re: Code Review request: 8028431: NullPointerException in DerValue.equals(DerValue)
Hello Vincent, Thanks for your feedback. I heve updated the webrev with the following: - the test moved to the existing test/java/security/cert/X509Certificate/ directory - copyright in the test - copyright year at the start of DerValue.java There was no 'othervm' tag in the test. Did I miss something? Please take a look: http://cr.openjdk.java.net/~asmotrak/8028431/webrev.01/ http://cr.openjdk.java.net/%7Easmotrak/8028431/webrev.01/ Artem On 12/20/2013 05:19 PM, Vincent Ryan wrote: Hello Artem, You fix looks good. You just need to fill in the missing portion of the copyright in the test. You could also adjust the copyright year range at the start of DerValue.java. Also I would add the test to the existing test/java/security/cert/X509Certificate/ directory rather than create a new one. Finally, I think the test should run fine without the jtreg tag for 'othervm'. Thanks. On 20/12/2013 12:51, Artem Smotrakov wrote: Hi, please review this fix for 9: https://bugs.openjdk.java.net/browse/JDK-8028431 http://cr.openjdk.java.net/~asmotrak/8028431/webrev.00/ http://cr.openjdk.java.net/%7Easmotrak/8028431/webrev.00/ sun.security.util.DerValue.equals(DerValue) method does not check that null is passed. As a result, NullPointerException can occur. Artem
Re: Code Review request: 8028431: NullPointerException in DerValue.equals(DerValue)
All looks good now. Thanks. On 20/12/2013 14:05, Artem Smotrakov wrote: Hello Vincent, Thanks for your feedback. I heve updated the webrev with the following: - the test moved to the existing test/java/security/cert/X509Certificate/ directory - copyright in the test - copyright year at the start of DerValue.java There was no 'othervm' tag in the test. Did I miss something? Please take a look: http://cr.openjdk.java.net/~asmotrak/8028431/webrev.01/ http://cr.openjdk.java.net/%7Easmotrak/8028431/webrev.01/ Artem On 12/20/2013 05:19 PM, Vincent Ryan wrote: Hello Artem, You fix looks good. You just need to fill in the missing portion of the copyright in the test. You could also adjust the copyright year range at the start of DerValue.java. Also I would add the test to the existing test/java/security/cert/X509Certificate/ directory rather than create a new one. Finally, I think the test should run fine without the jtreg tag for 'othervm'. Thanks. On 20/12/2013 12:51, Artem Smotrakov wrote: Hi, please review this fix for 9: https://bugs.openjdk.java.net/browse/JDK-8028431 http://cr.openjdk.java.net/~asmotrak/8028431/webrev.00/ http://cr.openjdk.java.net/%7Easmotrak/8028431/webrev.00/ sun.security.util.DerValue.equals(DerValue) method does not check that null is passed. As a result, NullPointerException can occur. Artem
Re: Code Review request: 8028431: NullPointerException in DerValue.equals(DerValue)
A couple of other comments:
1. Add an @Override annotation to the equals method. While you are in
there, could you also add @Override to the toString and hashCode methods.
2. Move the == check and make it the first thing you check
3. Nit: don't include space between ! and (
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof DerValue)) {
return false;
}
DerValue other = (DerValue) o;
4. In the test, close the FileInputStream after you are done with it
(use try-with-resources)
5. Is the certificate used in the test a real certificate issued by a CA
or one that you created yourself? If it is a real certificate, we should
not include it in openJDK. You will need to move the test to the closed
repo, or create your own bad certificate with the symptoms.
Thanks,
Sean
On 12/20/2013 08:19 AM, Vincent Ryan wrote:
Hello Artem,
You fix looks good. You just need to fill in the missing portion
of the copyright in the test. You could also adjust the copyright
year range at the start of DerValue.java.
Also I would add the test to the existing
test/java/security/cert/X509Certificate/ directory rather than create a
new one.
Finally, I think the test should run fine without the jtreg
tag for 'othervm'.
Thanks.
On 20/12/2013 12:51, Artem Smotrakov wrote:
Hi,
please review this fix for 9:
https://bugs.openjdk.java.net/browse/JDK-8028431
http://cr.openjdk.java.net/~asmotrak/8028431/webrev.00/
http://cr.openjdk.java.net/%7Easmotrak/8028431/webrev.00/
sun.security.util.DerValue.equals(DerValue) method does not check that
null is passed. As a result, NullPointerException can occur.
Artem
hg: jdk8/tl/jdk: 8029955: AIOB in XMLEntityScanner.scanLiteral upon parsing literals with 100 LF chars
Changeset: 73473e9dfc46 Author:joehw Date: 2013-12-20 09:56 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/73473e9dfc46 8029955: AIOB in XMLEntityScanner.scanLiteral upon parsing literals with 100 LF chars Reviewed-by: dfuchs, lancea, ulfzibis + test/javax/xml/jaxp/parsers/8029955/EntityScannerTest.java
hg: jdk8/tl/jdk: 2 new changesets
Changeset: 7186275e6ef1 Author:rriggs Date: 2013-12-20 13:06 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7186275e6ef1 8030002: Enhance deserialization using readObject Reviewed-by: sherman, chegar, scolebourne ! src/share/classes/java/time/Duration.java ! src/share/classes/java/time/Instant.java ! src/share/classes/java/time/LocalDate.java ! src/share/classes/java/time/LocalDateTime.java ! src/share/classes/java/time/LocalTime.java ! src/share/classes/java/time/MonthDay.java ! src/share/classes/java/time/OffsetDateTime.java ! src/share/classes/java/time/OffsetTime.java ! src/share/classes/java/time/Period.java ! src/share/classes/java/time/Year.java ! src/share/classes/java/time/YearMonth.java ! src/share/classes/java/time/ZoneId.java ! src/share/classes/java/time/ZoneOffset.java ! src/share/classes/java/time/ZoneRegion.java ! src/share/classes/java/time/ZonedDateTime.java ! src/share/classes/java/time/chrono/AbstractChronology.java ! src/share/classes/java/time/chrono/ChronoLocalDateTimeImpl.java ! src/share/classes/java/time/chrono/ChronoPeriodImpl.java ! src/share/classes/java/time/chrono/ChronoZonedDateTimeImpl.java ! src/share/classes/java/time/chrono/HijrahChronology.java ! src/share/classes/java/time/chrono/HijrahDate.java ! src/share/classes/java/time/chrono/IsoChronology.java ! src/share/classes/java/time/chrono/JapaneseChronology.java ! src/share/classes/java/time/chrono/JapaneseDate.java ! src/share/classes/java/time/chrono/JapaneseEra.java ! src/share/classes/java/time/chrono/MinguoChronology.java ! src/share/classes/java/time/chrono/MinguoDate.java ! src/share/classes/java/time/chrono/ThaiBuddhistChronology.java ! src/share/classes/java/time/chrono/ThaiBuddhistDate.java ! src/share/classes/java/time/temporal/ValueRange.java ! src/share/classes/java/time/temporal/WeekFields.java ! src/share/classes/java/time/zone/ZoneOffsetTransition.java ! src/share/classes/java/time/zone/ZoneOffsetTransitionRule.java ! src/share/classes/java/time/zone/ZoneRules.java ! test/java/time/tck/java/time/AbstractTCKTest.java ! test/java/time/tck/java/time/chrono/serial/TCKChronoLocalDateSerialization.java ! test/java/time/tck/java/time/chrono/serial/TCKChronologySerialization.java ! test/java/time/tck/java/time/serial/TCKDurationSerialization.java ! test/java/time/tck/java/time/serial/TCKInstantSerialization.java ! test/java/time/tck/java/time/serial/TCKLocalDateSerialization.java ! test/java/time/tck/java/time/serial/TCKLocalDateTimeSerialization.java ! test/java/time/tck/java/time/serial/TCKLocalTimeSerialization.java ! test/java/time/tck/java/time/serial/TCKMonthDaySerialization.java ! test/java/time/tck/java/time/serial/TCKOffsetDateTimeSerialization.java ! test/java/time/tck/java/time/serial/TCKOffsetTimeSerialization.java ! test/java/time/tck/java/time/serial/TCKPeriodSerialization.java ! test/java/time/tck/java/time/serial/TCKYearMonthSerialization.java ! test/java/time/tck/java/time/serial/TCKYearSerialization.java ! test/java/time/tck/java/time/serial/TCKZoneOffsetSerialization.java ! test/java/time/tck/java/time/serial/TCKZonedDateTimeSerialization.java ! test/java/time/tck/java/time/temporal/serial/TCKValueRangeSerialization.java ! test/java/time/tck/java/time/temporal/serial/TCKWeekFieldsSerialization.java ! test/java/time/tck/java/time/zone/serial/TCKZoneOffsetTransitionRuleSerialization.java ! test/java/time/tck/java/time/zone/serial/TCKZoneOffsetTransitionSerialization.java ! test/java/time/tck/java/time/zone/serial/TCKZoneRulesSerialization.java Changeset: 39a02b18b386 Author:rriggs Date: 2013-12-20 13:06 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/39a02b18b386 8029909: Clarify equals/hashcode behavior for java.time types Summary: Document the behavior of equals and hashcode in java.time.chrono date types Reviewed-by: sherman, scolebourne ! src/share/classes/java/time/chrono/HijrahDate.java ! src/share/classes/java/time/chrono/JapaneseDate.java ! src/share/classes/java/time/chrono/MinguoDate.java ! src/share/classes/java/time/chrono/ThaiBuddhistDate.java
Fwd: SQE test CertPath/CertPathBuilderTest failed for java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
Hi Xuelei, Jason are vacation, Can you help me to check it. I think this code change will cause SQE test error. - checkCRLs(cert, pubKey, signFlag, true, + checkCRLs(cert, pubKey, null, signFlag, true, Can you give some suggestion about this change? Thanks Kevin Original Message Subject: SQE test CertPath/CertPathBuilderTest failed for java.lang.IndexOutOfBoundsException: Index: 0, Size: 0 Date: Fri, 20 Dec 2013 12:11:21 +0800 From: zaiyao liu zaiyao@oracle.com Organization: Oracle Corporation To: JASON.UH jason...@oracle.com Hi Jason, There are some sqe test CertPath/CertPathBuilderTest due to following error: [2013-12-19T06:34:55.17] java.lang.IndexOutOfBoundsException: Index: 0, Size: 0 [2013-12-19T06:34:55.17]at java.util.ArrayList.rangeCheck(ArrayList.java:638) [2013-12-19T06:34:55.17]at java.util.ArrayList.get(ArrayList.java:414) [2013-12-19T06:34:55.17]at java.util.Collections$UnmodifiableList.get(Collections.java:1369) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.buildToNewKey(RevocationChecker.java:1068) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.verifyWithSeparateSigningKey(RevocationChecker.java:904) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.checkCRLs(RevocationChecker.java:571) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.checkCRLs(RevocationChecker.java:459) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.check(RevocationChecker.java:361) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.check(RevocationChecker.java:337) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.ReverseBuilder.verifyCert(ReverseBuilder.java:443) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.SunCertPathBuilder.depthFirstSearchReverse(SunCertPathBuilder.java:687) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.SunCertPathBuilder.buildReverse(SunCertPathBuilder.java:261) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.SunCertPathBuilder.buildCertPath(SunCertPathBuilder.java:167) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.SunCertPathBuilder.build(SunCertPathBuilder.java:136) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.SunCertPathBuilder.engineBuild(SunCertPathBuilder.java:131) [2013-12-19T06:34:55.17]at java.security.cert.CertPathBuilder.build(CertPathBuilder.java:280) [2013-12-19T06:34:55.17]at BuildCertPath.doBuild(BuildCertPath.java:395) [2013-12-19T06:34:55.17]at BuildCertPath.main(BuildCertPath.java:137) [2013-12-19T06:34:55.49] FAIL : I checked this test failed since http://hg.openjdk.java.net/jdk8/tl/jdk/rev/d6c4ae56c079 submitted, Can you help to check whether I should change SQE test to meet JDK changed, or this is a JDK bug? I have attached the SQE test, Please tell me if you need more information. Thanks Kevin /* * @(#)BuildCertPath.java 1.1 11/22/00 * * Copyright (c) 1998-2000 Sun Microsystems, Inc. All Rights Reserved. * * This software is the confidential and proprietary information of Sun * Microsystems, Inc. (Confidential Information). You shall not * disclose such Confidential Information and shall use it only in * accordance with the terms of the license agreement you entered into * with Sun. * * SUN MAKES NO REPRESENTATIONS OR WARRANTIES ABOUT THE SUITABILITY OF THE * SOFTWARE, EITHER EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE * IMPLIED WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR * PURPOSE, OR NON-INFRINGEMENT. SUN SHALL NOT BE LIABLE FOR ANY DAMAGES * SUFFERED BY LICENSEE AS A RESULT OF USING, MODIFYING OR DISTRIBUTING * THIS SOFTWARE OR ITS DERIVATIVES. * * CopyrightVersion 1.0_beta * */ import sun.security.util.Debug; import java.io.FileInputStream; import java.security.GeneralSecurityException; import java.security.KeyStore; import java.security.Provider; import java.security.PublicKey; import java.security.Security; import java.security.cert.CertificateException; import java.security.cert.CollectionCertStoreParameters; import java.security.cert.TrustAnchor; import java.security.cert.X509Certificate; import java.security.cert.X509CertSelector; import java.text.DateFormat; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Set; import java.util.Vector; import java.security.cert.CertPathBuilder; import java.security.cert.CertPathBuilderException; import java.security.cert.CertPathValidator; import java.security.cert.CertStore; import java.security.cert.LDAPCertStoreParameters; import java.security.cert.PKIXCertPathChecker; import java.security.cert.PKIXBuilderParameters; import
Re: Fwd: SQE test CertPath/CertPathBuilderTest failed for java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
Please file a JDK bug. Xuelei On 12/21/2013 11:10 AM, zaiyao liu wrote: Hi Xuelei, Jason are vacation, Can you help me to check it. I think this code change will cause SQE test error. - checkCRLs(cert, pubKey, signFlag, true, + checkCRLs(cert, pubKey, null, signFlag, true, Can you give some suggestion about this change? Thanks Kevin Original Message Subject:SQE test CertPath/CertPathBuilderTest failed for java.lang.IndexOutOfBoundsException: Index: 0, Size: 0 Date: Fri, 20 Dec 2013 12:11:21 +0800 From: zaiyao liu zaiyao@oracle.com Organization: Oracle Corporation To: JASON.UH jason...@oracle.com Hi Jason, There are some sqe test CertPath/CertPathBuilderTest due to following error: [2013-12-19T06:34:55.17] java.lang.IndexOutOfBoundsException: Index: 0, Size: 0 [2013-12-19T06:34:55.17]at java.util.ArrayList.rangeCheck(ArrayList.java:638) [2013-12-19T06:34:55.17]at java.util.ArrayList.get(ArrayList.java:414) [2013-12-19T06:34:55.17]at java.util.Collections$UnmodifiableList.get(Collections.java:1369) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.buildToNewKey(RevocationChecker.java:1068) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.verifyWithSeparateSigningKey(RevocationChecker.java:904) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.checkCRLs(RevocationChecker.java:571) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.checkCRLs(RevocationChecker.java:459) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.check(RevocationChecker.java:361) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.RevocationChecker.check(RevocationChecker.java:337) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.ReverseBuilder.verifyCert(ReverseBuilder.java:443) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.SunCertPathBuilder.depthFirstSearchReverse(SunCertPathBuilder.java:687) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.SunCertPathBuilder.buildReverse(SunCertPathBuilder.java:261) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.SunCertPathBuilder.buildCertPath(SunCertPathBuilder.java:167) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.SunCertPathBuilder.build(SunCertPathBuilder.java:136) [2013-12-19T06:34:55.17]at sun.security.provider.certpath.SunCertPathBuilder.engineBuild(SunCertPathBuilder.java:131) [2013-12-19T06:34:55.17]at java.security.cert.CertPathBuilder.build(CertPathBuilder.java:280) [2013-12-19T06:34:55.17]at BuildCertPath.doBuild(BuildCertPath.java:395) [2013-12-19T06:34:55.17]at BuildCertPath.main(BuildCertPath.java:137) [2013-12-19T06:34:55.49] FAIL : I checked this test failed sincehttp://hg.openjdk.java.net/jdk8/tl/jdk/rev/d6c4ae56c079 submitted, Can you help to check whether I should change SQE test to meet JDK changed, or this is a JDK bug? I have attached the SQE test, Please tell me if you need more information. Thanks Kevin
