RFR JDK-8067870: Fix java.io.ObjectInputStream.PeekInputStream#skip

[roger riggs]

Hi Pavel,

The change looks ok.

But since you're written the test and used it to demonstrate it works, 
it is useful and
with the application of the normal test coding conventions should be 
added as the
regression test for the issue.

Thanks, Roger

On 12/18/2014 8:32 AM, Pavel Rappo wrote:
> Hi everyone,
>
> Could you please review my change for JDK-8067870?
>
> http://cr.openjdk.java.net/~prappo/8067870/webrev.00/
>
> -------------------------------------------------------------------------------
> I suppose it's the first time this method gets executed. Ever :) There's no need
> for a test to be included with the change but for those who want to see the
> error with their own eyes, you can run this:
>
> import java.io.ByteArrayInputStream;
> import java.io.IOException;
> import java.io.InputStream;
> import java.lang.reflect.Constructor;
>
> public class PeekInputStreamTest {
>
>      public static void main(String[] args) throws ReflectiveOperationException, IOException {
>
>          Class<? extends InputStream> clazz =
>                  Class.forName("java.io.ObjectInputStream$PeekInputStream")
>                          .asSubclass(InputStream.class);
>
>          Constructor<? extends InputStream> ctr =
>                  clazz.getDeclaredConstructor(InputStream.class);
>
>          ctr.setAccessible(true);
>
>          ByteArrayInputStream in = new ByteArrayInputStream(new byte[4]);
>
>          InputStream peek = ctr.newInstance(in);
>
>          peek.skip(1);
>      }
> }
>
> This should produce StackOverflowError.
> To be sure that the fix worked as intended, you can run this:
>
> import java.io.ByteArrayInputStream;
> import java.io.IOException;
> import java.io.InputStream;
> import java.lang.reflect.Constructor;
> import java.lang.reflect.Method;
>
> public class PeekInputStreamTest {
>
>      public static void main(String[] args) throws ReflectiveOperationException,
>              IOException {
>
>          InputStream pin = createPeekInputStream(
>                  new ByteArrayInputStream(new byte[]{1, 2, 3, 4}));
>          peek(pin);
>          if (pin.skip(1) != 1 || pin.read() != 2)
>              throw new AssertionError();
>
>          InputStream pin1 = createPeekInputStream(
>                  new ByteArrayInputStream(new byte[]{1, 2, 3, 4}));
>          if (pin1.skip(1) != 1 || pin1.read() != 2)
>              throw new AssertionError();
>
>          InputStream pin2 = createPeekInputStream(
>                  new ByteArrayInputStream(new byte[]{1, 2, 3, 4}));
>          if (pin2.skip(0) != 0 || pin2.read() != 1)
>              throw new AssertionError();
>
>          InputStream pin3 = createPeekInputStream(
>                  new ByteArrayInputStream(new byte[]{1, 2, 3, 4}));
>          if (pin3.skip(2) != 2 || pin3.read() != 3)
>              throw new AssertionError();
>
>          InputStream pin4 = createPeekInputStream(
>                  new ByteArrayInputStream(new byte[]{1, 2, 3, 4}));
>          if (pin4.skip(3) != 3 || pin4.read() != 4)
>              throw new AssertionError();
>
>          InputStream pin5 = createPeekInputStream(
>                  new ByteArrayInputStream(new byte[]{1, 2, 3, 4}));
>          if (pin5.skip(16) != 4 || pin5.read() != -1)
>              throw new AssertionError();
>      }
>
>      private static InputStream createPeekInputStream(InputStream underlying)
>              throws ReflectiveOperationException {
>          Class<? extends InputStream> clazz =
>                  Class.forName("java.io.ObjectInputStream$PeekInputStream")
>                          .asSubclass(InputStream.class);
>
>          Constructor<? extends InputStream> ctr =
>                  clazz.getDeclaredConstructor(InputStream.class);
>          ctr.setAccessible(true);
>          return ctr.newInstance(underlying);
>      }
>
>      private static void peek(InputStream pin)
>              throws ReflectiveOperationException {
>          Method p = pin.getClass().getDeclaredMethod("peek");
>          p.setAccessible(true);
>          p.invoke(pin);
>      }
> }
>
> The output should be clean.
>
> -Pavel