Re: Code review request for 7172149 ArrayIndexOutOfBoundsException from Signature.verify
Hello Xuelei, What do you think of the updated patch? any comments? http://cr.openjdk.java.net/~luchsh/7172149_2/ Thanks - Jonathan On 05/30/2012 07:26 AM, Xuelei Fan wrote: On 5/29/2012 3:11 PM, Jonathan Lu wrote: Hi Xuelei, Thanks for review! On 05/29/2012 02:45 PM, Xuelei Fan wrote: That's an interesting topic. From my understand, the length of an array is of type "int". So normally, the (offset + length) should not be great than integer.max_value. Of course, Hostile or improper code are not of the case. What's interesting to me is that may be when we do additive operation for two "int" values, we may have to convert it to "long" in case of any overflow strictly. We are luck here because we have "long" type. But what about the additive operation for two "long" values I think this issue is special, since it is about index value of Java arrays, which is limited to smaller than Integer.MAX_VALUE according to Java language specification, not other general conditions of comparing integer or long values. Jonathan, do you run into the problem in real world? For now I am not quiet sure of whether it is from a real world problem, but this problem does exhibit some weakness or behavior differences, right? Yes, it is an improvement. Would you please add a comment about why convert it to "long", and update the copyright year to 2012? Otherwise, looks fine to me for JDK 8. Thanks& Regards, Xuelei Thanks& regards -Jonathan Thanks& Regards, Xuelei On 5/29/2012 1:53 PM, Jonathan Lu wrote: Hi Security-dev, Here's a patch for bug7172149, could anybody please help to take a look? http://cr.openjdk.java.net/~luchsh/7172149/ The problem is that the range check in Signature.verify(byte[], int, int) uses integer value to check whether (offset + length) is greater than signature.length, but if (offset + length) overflows the check will fail and ArrayIndexOutOfBoundsException will be thrown instead of IllegalArgumentException.My proposed solution is to make a conversion to long in the if block. Thanks! - Jonathan
hg: jdk8/tl/jdk: 6924259: Remove offset and count fields from java.lang.String
Changeset: 2c773daa825d Author:mduigou Date: 2012-05-17 10:06 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2c773daa825d 6924259: Remove offset and count fields from java.lang.String Summary: Removes the use of shared character array buffers by String along with the two fields needed to support the use of shared buffers. Reviewed-by: alanb, mduigou, forax, briangoetz Contributed-by: brian.dohe...@oracle.com ! src/share/classes/java/lang/Integer.java ! src/share/classes/java/lang/Long.java ! src/share/classes/java/lang/String.java ! src/share/classes/java/lang/StringCoding.java
hg: jdk8/tl/langtools: 7159016: Static import of member in processor-generated class fails in JDK 7
Changeset: 844478076c25 Author:jjh Date: 2012-05-31 15:07 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/844478076c25 7159016: Static import of member in processor-generated class fails in JDK 7 Reviewed-by: jjg ! src/share/classes/com/sun/tools/javac/comp/MemberEnter.java + test/tools/javac/T7159016.java
hg: jdk8/tl/jdk: 7126277: Alternative String hashing implementation
Changeset: 43bd5ee0205e Author:mduigou Date: 2012-05-30 22:18 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/43bd5ee0205e 7126277: Alternative String hashing implementation Summary: All of the hashing based Map implementations: HashMap, Hashtable, LinkedHashMap, WeakHashMap and ConcurrentHashMap are modified to use an enhanced hashing algorithm for string keys when the capacity of the hash table has ever grown beyond 512 entries. The enhanced hashing implementation uses the murmur3 hashing algorithm along with random hash seeds and index masks. These enhancements mitigate cases where colliding String hash values could result in a performance bottleneck. Reviewed-by: alanb, forax, dl ! make/java/java/FILES_java.gmk ! src/share/classes/java/lang/String.java ! src/share/classes/java/util/HashMap.java ! src/share/classes/java/util/Hashtable.java ! src/share/classes/java/util/LinkedHashMap.java ! src/share/classes/java/util/WeakHashMap.java ! src/share/classes/java/util/concurrent/ConcurrentHashMap.java + src/share/classes/sun/misc/Hashing.java ! test/java/util/Collection/BiggernYours.java ! test/java/util/Hashtable/HashCode.java ! test/java/util/Hashtable/SimpleSerialization.java + test/java/util/Map/Collisions.java ! test/java/util/Map/Get.java + test/sun/misc/Hashing.java
hg: jdk8/tl/jdk: 3 new changesets
Changeset: 0c6830e7241f Author:mullan Date: 2012-05-30 17:19 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/0c6830e7241f 6854712: Revocation checking enhancements (JEP-124) 6637288: Add OCSP support to PKIX CertPathBuilder implementation 7126011: ReverseBuilder.getMatchingCACerts may throws NPE Reviewed-by: xuelei ! src/share/classes/java/security/cert/CertPathBuilder.java ! src/share/classes/java/security/cert/CertPathBuilderSpi.java + src/share/classes/java/security/cert/CertPathChecker.java ! src/share/classes/java/security/cert/CertPathValidator.java ! src/share/classes/java/security/cert/CertPathValidatorSpi.java ! src/share/classes/java/security/cert/PKIXCertPathChecker.java + src/share/classes/java/security/cert/PKIXRevocationChecker.java ! src/share/classes/java/security/cert/package.html ! src/share/classes/sun/security/provider/certpath/AdjacencyList.java ! src/share/classes/sun/security/provider/certpath/BasicChecker.java ! src/share/classes/sun/security/provider/certpath/BuildStep.java ! src/share/classes/sun/security/provider/certpath/Builder.java ! src/share/classes/sun/security/provider/certpath/CertStoreHelper.java ! src/share/classes/sun/security/provider/certpath/CollectionCertStore.java ! src/share/classes/sun/security/provider/certpath/ConstraintsChecker.java - src/share/classes/sun/security/provider/certpath/CrlRevocationChecker.java ! src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java ! src/share/classes/sun/security/provider/certpath/ForwardBuilder.java ! src/share/classes/sun/security/provider/certpath/ForwardState.java ! src/share/classes/sun/security/provider/certpath/IndexedCollectionCertStore.java ! src/share/classes/sun/security/provider/certpath/KeyChecker.java ! src/share/classes/sun/security/provider/certpath/OCSP.java - src/share/classes/sun/security/provider/certpath/OCSPChecker.java ! src/share/classes/sun/security/provider/certpath/OCSPRequest.java ! src/share/classes/sun/security/provider/certpath/OCSPResponse.java + src/share/classes/sun/security/provider/certpath/PKIX.java ! src/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java ! src/share/classes/sun/security/provider/certpath/PKIXMasterCertPathValidator.java ! src/share/classes/sun/security/provider/certpath/PolicyChecker.java ! src/share/classes/sun/security/provider/certpath/PolicyNodeImpl.java ! src/share/classes/sun/security/provider/certpath/ReverseBuilder.java ! src/share/classes/sun/security/provider/certpath/ReverseState.java + src/share/classes/sun/security/provider/certpath/RevocationChecker.java ! src/share/classes/sun/security/provider/certpath/SunCertPathBuilder.java ! src/share/classes/sun/security/provider/certpath/SunCertPathBuilderParameters.java ! src/share/classes/sun/security/provider/certpath/URICertStore.java ! src/share/classes/sun/security/provider/certpath/Vertex.java ! src/share/classes/sun/security/provider/certpath/X509CertPath.java ! src/share/classes/sun/security/provider/certpath/X509CertificatePair.java ! src/share/classes/sun/security/x509/X509CRLEntryImpl.java + test/java/security/cert/PKIXRevocationChecker/UnitTest.java Changeset: 3192e73394fe Author:mullan Date: 2012-05-31 17:07 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3192e73394fe Merge - src/share/classes/sun/security/provider/certpath/CrlRevocationChecker.java ! src/share/classes/sun/security/provider/certpath/ForwardBuilder.java ! src/share/classes/sun/security/provider/certpath/ForwardState.java - src/share/classes/sun/security/provider/certpath/OCSPChecker.java ! src/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java ! src/share/classes/sun/security/provider/certpath/ReverseBuilder.java ! src/share/classes/sun/security/provider/certpath/ReverseState.java ! src/share/classes/sun/security/provider/certpath/SunCertPathBuilder.java Changeset: 48dfc0df61d0 Author:mullan Date: 2012-05-31 17:10 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/48dfc0df61d0 Merge
hg: jdk8/tl/langtools: 2 new changesets
Changeset: af6a4c24f4e3 Author:mcimadamore Date: 2012-05-31 17:42 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/af6a4c24f4e3 7166552: Inference: cleanup usage of Type.ForAll Summary: Remove hack to callback into type-inference from assignment context Reviewed-by: dlsmith, jjg ! src/share/classes/com/sun/tools/javac/code/Type.java ! src/share/classes/com/sun/tools/javac/comp/Attr.java ! src/share/classes/com/sun/tools/javac/comp/AttrContext.java ! src/share/classes/com/sun/tools/javac/comp/Check.java ! src/share/classes/com/sun/tools/javac/comp/Infer.java ! src/share/classes/com/sun/tools/javac/comp/Resolve.java ! src/share/classes/com/sun/tools/javac/resources/compiler.properties ! test/tools/javac/6758789/T6758789b.out ! test/tools/javac/diags/examples.not-yet.txt ! test/tools/javac/diags/examples/ApplicableMethodFound1.java ! test/tools/javac/diags/examples/CantApplyDiamond1.java - test/tools/javac/diags/examples/FullInstSig.java ! test/tools/javac/diags/examples/IncompatibleTypes1.java ! test/tools/javac/diags/examples/InferredDoNotConformToLower.java - test/tools/javac/diags/examples/InvalidInferredTypes.java + test/tools/javac/diags/examples/NoUniqueMaximalInstance.java - test/tools/javac/diags/examples/UndeterminedType1.java ! test/tools/javac/diags/examples/WhereFreshTvar.java ! test/tools/javac/generics/7015430/T7015430.out ! test/tools/javac/generics/7151802/T7151802.out ! test/tools/javac/generics/inference/6315770/T6315770.out ! test/tools/javac/generics/inference/6638712/T6638712b.out ! test/tools/javac/generics/inference/6638712/T6638712e.out ! test/tools/javac/generics/inference/6650759/T6650759m.out ! test/tools/javac/generics/inference/7154127/T7154127.out ! test/tools/javac/varargs/6313164/T6313164.out Changeset: 37dc15c68760 Author:mcimadamore Date: 2012-05-31 17:44 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/37dc15c68760 7160084: javac fails to compile an apparently valid class/interface combination Summary: javac generates wrong syntetized trees for nested enum constants Reviewed-by: dlsmith, jjg ! src/share/classes/com/sun/tools/javac/comp/Attr.java ! src/share/classes/com/sun/tools/javac/comp/MemberEnter.java ! src/share/classes/com/sun/tools/javac/tree/TreeInfo.java + test/tools/javac/enum/7160084/T7160084a.java + test/tools/javac/enum/7160084/T7160084b.java