<html><body><div dir="ltr"><div style="">

    </div><div style=""><div>

        <div>If you return the short buffer (on IOException), does it need to cache the pending exception or can you just rely on the next read hitting that underlying exception again? (Is there an actual guarantee somewhere that the read position is not altered on IOExceptions or that you always get lending bytes before this report? (If so, does the native code do that as well?)</div><div dir="ltr"><br></div><div dir="ltr">Gruss</div><div dir="ltr">Bernd</div><div id="ms-outlook-mobile-signature"><div style="direction:ltr">-- </div><div style="direction:ltr">http://bernd.eckenfels.net</div></div>

    </div>

<div> </div><hr style="display:inline-block;width:98%" tabindex="-1"><div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif"><b>Von:</b> security-dev <security-dev-retn@openjdk.org> im Auftrag von Alan Bateman <alanb@openjdk.org><br><b>Gesendet:</b> Montag, Juli 24, 2023 3:21 PM<br><b>An:</b> core-libs-dev@openjdk.org <core-libs-dev@openjdk.org>; security-dev@openjdk.org <security-dev@openjdk.org><br><b>Betreff:</b> Re: RFR: 6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available<div> </div></font></div>On Mon, 24 Jul 2023 09:16:23 GMT, Vyom Tewari <vtewari@openjdk.org> wrote:

<br>

>> Limit native memory allocation and move write loop from the native layer into Java. This change should make the OOME reported in the issue much less likely.

<br>>

<br>> src/java.base/share/native/libjava/io_util.c line 99:

<br>> 

<br>>> 97:         return 0;

<br>>> 98:     } else if (len > BUF_SIZE) {

<br>>> 99:         if (len > MAX_MALLOC_SIZE)

<br>> 

<br>> Hi Brian if I am reading code correctly then with the current code change FIS.read(byte[] b, int off, int len) will always read (MAX_MALLOC_SIZE 2097152) bytes if len > MAX_MALLOC_SIZE. 

<br>> 

<br>> The java doc of read  as below

<br>> 

<br>> public int read(byte[] b,

<br>>                 int off,

<br>>                 int len)

<br>>          throws IOException

> Reads up to len bytes of data from the input stream into an array of bytes. An attempt is made to read as many as len bytes, but a smaller number may be read. The number of bytes actually read is returned as an integer.

<br>> 

> I think  if you are limiting the internal dynamic buffer to 2097152 byte then you have  to at least attempt to read as many as len bytes if possible before returning.

<br>> 

<br>> If I simply the run the following code 

<br>> 

<br>>      int size = 501 * 501 * 501 * 3;

<br>> 

<br>>             FileInputStream fis = new FileInputStream("/home/vyom1/test.img"); // Any file with size >= 501*501*501*2

<br>> 

<br>>             System.out.println("size: " + size);

<br>> 

<br>>             byte buf[] = new byte[size];

<br>> 

<br>>             System.out.println("buf ok");

<br>> 

<br>>             int bytesRead = fis.read(buf, 0, size);

<br>>             System.out.println("Bytes read " + bytesRead);

<br>> 

> It will always print “Bytes read 2097152” which is not as per Java specification of InputStream.read(byte[]b, int off, int len).

<br>

A short read is okay but changing long standing behavior could potentially break already broken code that calls the method with a large byte array and assumes it will read to EOF. So I think it's a forced move to read until there is no space remaining or EOF is reached. This is logic for the Java level, not the native code of course. If an I/O exception is on the second/subsequent reads then it will have to return the bytes that are read, not throw with bytes in the buffer.

<br>

<br>-------------

<br>

<br>PR Review Comment: https://git.openjdk.org/jdk/pull/14981#discussion_r1272236807

<br>

<br></div></div></body></html>