RFR: 8294696 - BufferedInputStream.transferTo should drain buffer when mark set [v6]
Markus KARG
duke at openjdk.org
Wed Nov 29 22:53:22 UTC 2023
On Wed, 29 Nov 2023 05:55:03 GMT, Vladimir Sitnikov <vsitnikov at openjdk.org> wrote:
>> src/java.base/share/classes/java/io/BufferedInputStream.java line 612:
>>
>>> 610: if (avail > 0) {
>>> 611: // Prevent poisoning and leaking of buf
>>> 612: byte[] buffer = Arrays.copyOfRange(getBufIfOpen(), pos, count);
>>
>> @mkarg , could you please clarify why you added `Arrays.copyOfRange` here?
>> It seems to be an excessive copy that doesn't help much.
>>
>> `buf` is `protected` in `BufferedInputStream`, so if someone really wants to get hold of the actual buffer, they can subclass `BufferedInputStream` and expose the buffer directly.
>> What do you think of removing `copyOfRange`?
>
> Buffer copy was not there before, and defensive copy was never present in `ByteArrayInputStream` as well: https://github.com/openjdk/jdk/blob/9a6ca233c7e91ffa2ce9451568b3be88ccd04504/src/java.base/share/classes/java/io/ByteArrayInputStream.java#L213
Alan asked for this, and for good reason: While we implicitly trust subclasses as the buffer is "theirs" as part of the inheritance, we do not know where target stream comes from. It could be an injected verhicle to perform (at least) the following:
* Leaking data. The target stream could access data beyond the intended region.
* Poisoning. The target stream could write into the buffer.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/10525#discussion_r1409957433
More information about the core-libs-dev
mailing list