RFR: 8294696 - BufferedInputStream.transferTo should drain buffer when mark set [v2]

Alan Bateman alanb at openjdk.org
Sun Oct 23 06:24:40 UTC 2022


On Wed, 19 Oct 2022 08:31:07 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Markus KARG has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Checking explicitly -1 instead of < 0
>
>> * Alternative B: Instead of double-buffering I drop the original buffer and use a same-size replacement each time the buffer was drained inside of `transferTo`. This is a rather cheap solution and rather perfectly prevents OOME, as I drop _first_ before reallocating.
> 
> That is a good idea to try. It wouldn't work when BIS is extended but transferTo already checks this.
> 
> Note that you will need to zero the elements from 0 to pos, and count to buf.length, but I think you know that.

> @AlanBateman I have implemented Alternative B as proposed with one single change: I do not zero-out anything as in our particular case (count == pos and markpos == -1) it is cheaper to simply set count and pos both to 0 (fill() did the same trick already). Due to that any subsequent read() invocation will in turn fill(), which in turn writes valid bytes into the new buffer. So this solution should safe us from possible OOME _and_ poisoned buffers. WDYT?

It helps a bit but it still leaks the bytes in 0 to pos, and count buf.length. I think we have to assume that Dr. Evil's output stream will throw an exception so the code to replace the buffer won't run. This means replaces buf before handing out the original buffer. The transferTo method transfers all bytes to EOF so you may be able to get away with just allocating a 0 or tiny buffer, it can grow if needed with subsequent reads or transfers.

-------------

PR: https://git.openjdk.org/jdk/pull/10525


More information about the core-libs-dev mailing list