RFR: 8080272 Refactor I/O stream copying to use InputStream.transferTo/readAllBytes and Files.copy [v11]

[Sean Mullan]

On Fri, 19 Feb 2021 08:05:06 GMT, Andrey Turbanov <github.com+741251+turbanoff at 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));
> 
> ![изображение](https://user-images.githubusercontent.com/741251/108475587-f4f61080-72a1-11eb-91e0-ac2b98c7c490.png)
> 
> 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