RFR: 8245036: DataInputStream.readFully(byte[], int, int) does not throw expected IndexOutOfBoundsExceptions
Raffaello Giulietti
raffaello.giulietti at gmail.com
Fri Aug 7 08:03:00 UTC 2020
On 2020-08-06 23:42, Brian Burkhalter wrote:
> Hi Raffaello,
>
> I think the source change is correct, it’s just that for a deterministic
> issue, the test generally will fail before the source change and pass
> with the source change applied. I did not investigate any further, either.
>
> Thanks,
>
> Brian
>
>> On Aug 6, 2020, at 2:36 PM, Raffaello Giulietti
>> <raffaello.giulietti at gmail.com <mailto:raffaello.giulietti at gmail.com>>
>> wrote:
>>
>> the body of readFully() seems to silently assume that the other
>> arguments are correct (e.g., off >= 0 and so on).
>>
>> In the (still) current implementation of
>> DataInputStream.readFully(byte[], int, int), when off < 0 and len > 0,
>> in the loop it then invokes the underlying in.read(byte[], int, int)
>> with a negative offset off + n during the initial iterations.
>>
>> In the specific case of the tests on underlying FileInputStreams, it
>> thus seems that read(byte[], int, int) does not, in turn, validate its
>> arguments. I didn't investigate further, although I will in the next
>> days if deemed necessary.
>
Hi Brian,
my previous explanation is (luckily) wrong.
The reason the new tests pass on the old source is because the
underlying input stream correctly throws IOOBE which is catched by the
tests.
There's one corner case that would not pass, however, so I added
testNegativeOffsetZeroLength() to exercise that. It passes on the new
source but fails on the old.
I also added two additional corner cases for too large combinations of
offset and length.
----
# HG changeset patch
# User lello
# Date 1596786685 -7200
# Fri Aug 07 09:51:25 2020 +0200
# Node ID 1537910a63fbcd517607779c72b02ead2c77bf08
# Parent b01985b4f88f554f97901e53e1ba314681dd9c19
Patch to fix JDK-8245036
8245036: DataInputStream.readFully(byte[], int, int) does not throw
expected IndexOutOfBoundsExceptions
Reviewed-by: TBD
Contributed-by: Raffaello Giulietti <raffaello.giulietti at gmail.com>
diff --git a/src/java.base/share/classes/java/io/DataInputStream.java
b/src/java.base/share/classes/java/io/DataInputStream.java
--- a/src/java.base/share/classes/java/io/DataInputStream.java
+++ b/src/java.base/share/classes/java/io/DataInputStream.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1994, 2019, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 1994, 2020, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -25,6 +25,8 @@
package java.io;
+import java.util.Objects;
+
/**
* A data input stream lets an application read primitive Java data
* types from an underlying input stream in a machine-independent
@@ -192,8 +194,7 @@
* @see java.io.FilterInputStream#in
*/
public final void readFully(byte b[], int off, int len) throws
IOException {
- if (len < 0)
- throw new IndexOutOfBoundsException();
+ Objects.checkFromIndexSize(off, len, b.length);
int n = 0;
while (n < len) {
int count = in.read(b, off + n, len - n);
diff --git a/test/jdk/java/io/DataInputStream/ReadFully.java
b/test/jdk/java/io/DataInputStream/ReadFully.java
--- a/test/jdk/java/io/DataInputStream/ReadFully.java
+++ b/test/jdk/java/io/DataInputStream/ReadFully.java
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1999, 2010, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 1999, 2020, Oracle and/or its affiliates. All rights
reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -22,30 +22,108 @@
*/
/* @test
- * @bug 4214513
- * @summary Passing a negative length argument for readFully must throw
- * IndexOutOfBoundsException.
+ * @bug 4214513 8245036
+ * @summary Passing a negative offset or length,
+ * or passing a combination of offset and length too big
+ * for readFully must throw IndexOutOfBoundsException.
*/
+import java.io.*;
-import java.io.*;
public class ReadFully {
- public static final void main(String[] args) throws Exception {
- byte[] buffer = new byte[100];
+
+ private static final void testNegativeOffset() throws Exception {
+ File file = new File(System.getProperty("test.src"),
+ "ReadFully.java");
+ try (FileInputStream in = new FileInputStream(file);
+ DataInputStream dis = new DataInputStream(in);) {
+ byte[] buffer = new byte[100];
+ dis.readFully(buffer, -1, buffer.length);
+ throw new RuntimeException("Test testNegativeOffset() failed");
+ } catch (IndexOutOfBoundsException ignore) {
+ }
+ }
+
+ private static final void testNegativeLength() throws Exception {
+ File file = new File(System.getProperty("test.src"),
+ "ReadFully.java");
+ try (FileInputStream in = new FileInputStream(file);
+ DataInputStream dis = new DataInputStream(in);) {
+ byte[] buffer = new byte[100];
+ dis.readFully(buffer, 0, -1);
+ throw new RuntimeException("Test testNegativeLength() failed");
+ } catch (IndexOutOfBoundsException ignore) {
+ }
+ }
+
+ private static final void testNegativeOffsetZeroLength() throws
Exception {
+ File file = new File(System.getProperty("test.src"),
+ "ReadFully.java");
+ try (FileInputStream in = new FileInputStream(file);
+ DataInputStream dis = new DataInputStream(in);) {
+ byte[] buffer = new byte[100];
+ dis.readFully(buffer, -1, 0);
+ throw new RuntimeException("Test
testNegativeOffsetZeroLength() failed");
+ } catch (IndexOutOfBoundsException ignore) {
+ }
+ }
+
+ private static final void testBigOffsetLength1() throws Exception {
File file = new File(System.getProperty("test.src"),
"ReadFully.java");
- FileInputStream in = new FileInputStream(file);
- DataInputStream dis = new DataInputStream(in);
+ try (FileInputStream in = new FileInputStream(file);
+ DataInputStream dis = new DataInputStream(in);) {
+ byte[] buffer = new byte[100];
+ dis.readFully(buffer, 0, buffer.length + 1);
+ throw new RuntimeException("Test testBigOffsetLength1()
failed");
+ } catch (IndexOutOfBoundsException ignore) {
+ }
+ }
- boolean caughtException = false;
- try {
- dis.readFully(buffer, 0, -20);
- } catch (IndexOutOfBoundsException ie) {
- caughtException = true;
- } finally {
- dis.close();
- if (!caughtException)
- throw new RuntimeException("Test failed");
+ private static final void testBigOffsetLength2() throws Exception {
+ File file = new File(System.getProperty("test.src"),
+ "ReadFully.java");
+ try (FileInputStream in = new FileInputStream(file);
+ DataInputStream dis = new DataInputStream(in);) {
+ byte[] buffer = new byte[100];
+ dis.readFully(buffer, 1, buffer.length);
+ throw new RuntimeException("Test testBigOffsetLength2()
failed");
+ } catch (IndexOutOfBoundsException ignore) {
}
}
+
+ private static final void testBigOffsetLength3() throws Exception {
+ File file = new File(System.getProperty("test.src"),
+ "ReadFully.java");
+ try (FileInputStream in = new FileInputStream(file);
+ DataInputStream dis = new DataInputStream(in);) {
+ byte[] buffer = new byte[100];
+ dis.readFully(buffer, buffer.length, 1);
+ throw new RuntimeException("Test testBigOffsetLength3()
failed");
+ } catch (IndexOutOfBoundsException ignore) {
+ }
+ }
+
+ private static final void testBigOffsetLength4() throws Exception {
+ File file = new File(System.getProperty("test.src"),
+ "ReadFully.java");
+ try (FileInputStream in = new FileInputStream(file);
+ DataInputStream dis = new DataInputStream(in);) {
+ byte[] buffer = new byte[100];
+ dis.readFully(buffer, buffer.length + 1, 0);
+ throw new RuntimeException("Test testBigOffsetLength4()
failed");
+ } catch (IndexOutOfBoundsException ignore) {
+ }
+ }
+
+ public static final void main(String[] args) throws Exception {
+ testNegativeOffset();
+ testNegativeLength();
+ testNegativeOffsetZeroLength();
+ testBigOffsetLength1();
+ testBigOffsetLength2();
+ testBigOffsetLength3();
+ testBigOffsetLength4();
+ }
+
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: JDK-8245036.patch
Type: text/x-patch
Size: 7289 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20200807/b01f24e2/JDK-8245036-0001.patch>
More information about the core-libs-dev
mailing list