On Wed, 29 Nov 2023 05:55:03 GMT, Vladimir Sitnikov <vsitni...@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
