[GitHub] [incubator-dubbo] kezhenxu94 commented on issue #3420: DO NOT USE org.apache.dubbo.common.utils.IOUtils.write() for your own project

["kezhenxu94 (GitHub)" <gi...@apache.org>]

> * [x]  I have searched the [issues](https://github.com/apache/incubator-dubbo/issues) of this repository and believe that this is not a duplicate.
> * [x]  I have checked the [FAQ](https://github.com/apache/incubator-dubbo/blob/master/FAQ.md) of this repository and believe that this is not a duplicate.
> 
> ### Environment
> * Dubbo version: 2.7.1-SNAPSHOT
> 
> `org.apache.dubbo.common.utils.IOUtils` it seems that this is a class for common use but there is a pitfall in it. DO NOT USE IT IN YOUR OWN PROJECT!
> 
> ### Problem Description
> There is a method called `write()` in the class：
> 
> ```
> public static long write(InputStream is, OutputStream os, int bufferSize) throws IOException {
>     int read;
>     long total = 0;
>     byte[] buff = new byte[bufferSize];
>     while (is.available() > 0) {   // !!!
>         read = is.read(buff, 0, buff.length);
>         if (read > 0) {
>             os.write(buff, 0, read);
>             total += read;
>         }
>     }
>     return total;
> }
> ```
> which uses the returned value of `InputStream.available()` as the break condition of the while loop, in network environment, it returns 0 immediately in case of network latency and thus is not applicable to work as the EOF mark of the stream. Test code as follows:
> 
> ```
> @Test
> public void testWriteURL() throws Exception {
>     String url = "https://pan.baidu.com/static/images/16new/bg2.jpg";
>     long expectedSize = 77_949L;
> 
>     InputStream inputStream = new URL(url).openStream();
>     ByteArrayOutputStream bos = new ByteArrayOutputStream();
>     long writeSize = org.apache.dubbo.common.utils.IOUtils.write(inputStream, bos);
>     assertThat(writeSize, equalTo(expectedSize)); // org.hamcrest.MatcherAssert
> }
> ```
> The test will usually fail, method `write()` returns after reading part of the content and the loop breaks.
> 
> for a correct implementation please refer to `org.apache.commons.io.IOUtils#copy(java.io.InputStream, java.io.OutputStream)`, and corresponding test code is:
> 
> ```
> public static void main(String[] args) throws IOException {
>     String url = "https://pan.baidu.com/static/images/16new/bg2.jpg";
>     long expectedSize = 77_949L;
> 
>     InputStream inputStream = new URL(url).openStream();
>     ByteArrayOutputStream bos = new ByteArrayOutputStream();
>     long writeSize = org.apache.commons.io.IOUtils.copy(inputStream, bos);
>     assertThat(writeSize, equalTo(expectedSize)); // org.hamcrest.MatcherAssert
> }
> ```
> the test above passes
> 
> why this issue didn't expose up to now? Because it is only used once in `org.apache.dubbo.common.io.Bytes#unzip()` within the whole project, and is only used to deal with data in memory. If we use it to read data from network in the future in some places, problem occurs.
@yiding-he khanimteyaz

[ Full content available at: https://github.com/apache/incubator-dubbo/issues/3420 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org