On Fri, 19 Feb 2021 15:03:11 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> As I can see only ByteArrayInputStream is actually passed in `InputStream` >> in current JDK code: >> >> PKCS7 pkcs7 = new PKCS7(is.readAllBytes()); >> private static List<X509Certificate> parsePKCS7(InputStream is) >> certs = parsePKCS7(is); >> public X509CertPath(InputStream is, String encoding) >> return new X509CertPath(new ByteArrayInputStream(data), >> encoding); >> >> PKCS7 pkcs7 = new PKCS7(is.readAllBytes()); >> private static List<X509Certificate> parsePKCS7(InputStream is) >> certs = parsePKCS7(is); >> public X509CertPath(InputStream is, String encoding) >> this(is, PKIPATH_ENCODING); >> public X509CertPath(InputStream is) throws >> CertificateException { >> return new X509CertPath(new >> ByteArrayInputStream(encoding)); >> >>  >> >> Perhaps original marking approach was lost during refactoring? > > You are right, the code was refactored (way back in 2010) [1] to read one > block at a time, so this check on mark can be removed. So, in this case, I > think it is probably safe to just pass the InputStream as-is to > PKCS7(InputStream), but maybe you can add a comment that says this should > always be a ByteArrayInputStream. We can look at refactoring this code and > clean it up a bit more later. > > [1] https://hg.openjdk.java.net/jdk/jdk/rev/337ae296b6d6 I find implementation of `sun.security.pkcs.PKCS7#PKCS7(java.io.InputStream)` a bit confusing (or even buggy). It uses only `InputStream.available()` to parse block. So I would prefer to not use it. ------------- PR: https://git.openjdk.java.net/jdk/pull/1853