Reminder RFR: 8245036: DataInputStream.readFully(byte[], int, int) does not throw expected IndexOutOfBoundsExceptions

[Raffaello Giulietti]

Hi,

a gentle reminder to review this almost trivial patch.


Greetings
Raffaello



On 2020-07-29 19:44, Raffaello Giulietti wrote:
> Hi Tagir,
> 
> here's yesterday's patch with added unit tests.
> 
> 
> Greetings
> Raffaello
> 
> 
> 
> On 2020-07-29 05:26, Tagir Valeev wrote:
>> Hello!
>>
>> Having a unit-test would be nice!
>>
>> With best regards,
>> Tagir Valeev.
>>
>> On Tue, Jul 28, 2020 at 9:04 PM Raffaello Giulietti
>> <raffaello.giulietti at gmail.com> wrote:
>>>
>>> Hello,
>>>
>>> here's a very simple patch to address issue 8245036.
>>> HTH
>>>
>>>
>>> Greetings
>>> Raffaello
> 
> 
> 
> 
> ----
> 
> # HG changeset patch
> # User lello
> # Date 1596043874 -7200
> #      Wed Jul 29 19:31:14 2020 +0200
> # Node ID b9d417f53ddaae3801585b285f6eb8ba1606af8b
> # Parent  89fe9e02a522b57c356f0dd5279085a075c2945b
> 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,15 +22,36 @@
>    */
> 
>   /* @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 {
> +
> +    private static final void testNegativeOffset() throws Exception {
> +        byte[] buffer = new byte[100];
> +        File file = new File(System.getProperty("test.src"),
> +                "ReadFully.java");
> +        FileInputStream in = new FileInputStream(file);
> +        DataInputStream dis = new DataInputStream(in);
> +
> +        boolean caughtException = false;
> +        try {
> +            dis.readFully(buffer, -1, buffer.length);
> +        } catch (IndexOutOfBoundsException ie) {
> +            caughtException = true;
> +        } finally {
> +            dis.close();
> +            if (!caughtException)
> +                throw new RuntimeException("Test testNegativeOffset() 
> failed");
> +        }
> +    }
> +
> +    private static final void testNegativeLength() throws Exception {
>           byte[] buffer = new byte[100];
>           File file = new File(System.getProperty("test.src"),
>                   "ReadFully.java");
> @@ -45,7 +66,53 @@
>           } finally {
>               dis.close();
>               if (!caughtException)
> -                throw new RuntimeException("Test failed");
> +                throw new RuntimeException("Test testNegativeLength() 
> failed");
> +        }
> +    }
> +
> +    private static final void testBigOffsetLength1() throws Exception {
> +        byte[] buffer = new byte[100];
> +        File file = new File(System.getProperty("test.src"),
> +                "ReadFully.java");
> +        FileInputStream in = new FileInputStream(file);
> +        DataInputStream dis = new DataInputStream(in);
> +
> +        boolean caughtException = false;
> +        try {
> +            dis.readFully(buffer, 0, buffer.length + 1);
> +        } catch (IndexOutOfBoundsException ie) {
> +            caughtException = true;
> +        } finally {
> +            dis.close();
> +            if (!caughtException)
> +                throw new RuntimeException("Test testBigOffsetLength1() 
> failed");
>           }
>       }
> +
> +    private static final void testBigOffsetLength2() throws Exception {
> +        byte[] buffer = new byte[100];
> +        File file = new File(System.getProperty("test.src"),
> +                "ReadFully.java");
> +        FileInputStream in = new FileInputStream(file);
> +        DataInputStream dis = new DataInputStream(in);
> +
> +        boolean caughtException = false;
> +        try {
> +            dis.readFully(buffer, buffer.length, 1);
> +        } catch (IndexOutOfBoundsException ie) {
> +            caughtException = true;
> +        } finally {
> +            dis.close();
> +            if (!caughtException)
> +                throw new RuntimeException("Test testBigOffsetLength2() 
> failed");
> +        }
> +    }
> +
> +    public static final void main(String[] args) throws Exception {
> +        testNegativeOffset();
> +        testNegativeLength();
> +        testBigOffsetLength1();
> +        testBigOffsetLength2();
> +    }
> +
>   }
>