On Fri, 19 Feb 2021 08:05:06 GMT, Andrey Turbanov
<github.com+741251+turban...@openjdk.org> wrote:
>> src/java.base/share/classes/sun/security/provider/certpath/X509CertPath.java
>> line 228:
>>
>>> 226: try {
>>> 227: if (is.markSupported() == false) {
>>> 228: // Copy the entire input stream into an InputStream
>>> that does
>>
>> I don't think you should remove lines 228-232. These methods are called by
>> methods of CertificateFactory that take InputStream (which may contain a
>> stream of security data) and they are designed such that they try to read
>> one Certificate, CRL, or CertPath from the InputStream and leave the
>> InputStream ready to parse the next structure instead of consuming all of
>> the bytes. Thus they check if the InputStream supports mark in order to try
>> to preserve that behavior. If mark is not supported, then it's ok to use
>> InputStream.readAllBytes, otherwise, leave the stream as-is.
>
> 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
-------------
PR: https://git.openjdk.java.net/jdk/pull/1853
