Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8180899

GZIPOutputStream should not write header in constructor

    XMLWordPrintable

Details

    Description

      FULL PRODUCT VERSION :
      java version "1.8.0_121"
      Java(TM) SE Runtime Environment (build 1.8.0_121-b13)
      Java HotSpot(TM) 64-Bit Server VM (build 25.121-b13, mixed mode)

      ADDITIONAL OS VERSION INFORMATION :
      Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64

      EXTRA RELEVANT SYSTEM CONFIGURATION :
      Not relevant

      A DESCRIPTION OF THE PROBLEM :
      GZIPOutputStream writes out the header in the constructor.

      If that goes wrong, the underlying stream is not closed in try-with-resources, if it was not assigned a separate variable.

      This mistake was even made here: http://www.oracle.com/technetwork/articles/java/trywithresources-401775.html

      It is suggested there to re-write

      try(
                 FileInputStream fin = new FileInputStream(input);
                 FileOutputStream fout = new FileOutputStream(output);
                 GZIPOutputStream out = new GZIPOutputStream(fout)
             ) { ... }

      into

      try(
                 FileInputStream fin = new FileInputStream(input);
                 GZIPOutputStream out = new GZIPOutputStream(new FileOutputStream(output))
             ) { ... }

      which then would not close the FileOutputStream.

      I think this goes against the assumptions of developers that declaring resources in try-with-resources as shown in the second example is safe.

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      Declare a GZIPOutputStream like this:

      try(
                 GZIPOutputStream out = new GZIPOutputStream(new XXXOutputStream())
             ) { ... }

      with your own implementation of OutputStream that throws an IOException upon write.

      The GZIPOutputStream will then in its constructor try to write into your stream, that will fail, and since it's happening at construction time, there is no reference for the try-with-resources block that could call close() on the GZIPOutputStream (which in turn means, the underlying stream is not closed).

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      The header is written after construction time, but before the first actual write, in order to make sure that the underlying output stream is closed in the case that stream is created anonymously in try-with-resources like shown above.
      ACTUAL -
      An IOException is thrown, and the underlying stream is not closed when created anonymously.

      ERROR MESSAGES/STACK TRACES THAT OCCUR :
      java.io.IOException
      at java.util.zip.GZIPOutputStream.writeHeader(GZIPOutputStream.java:182)
      at java.util.zip.GZIPOutputStream.<init>(GZIPOutputStream.java:94)
      at java.util.zip.GZIPOutputStream.<init>(GZIPOutputStream.java:109)

      REPRODUCIBILITY :
      This bug can be reproduced always.

      ---------- BEGIN SOURCE ----------
      import java.io.IOException;
      import java.io.OutputStream;
      import java.util.zip.GZIPOutputStream;

      public class ExceptionTest {

          public static void main(final String[] args) throws IOException {
              try (GZIPOutputStream out = new GZIPOutputStream(new ThrowingOutputStream())) {

              } finally {
                  System.out.println("Stream closed? " + ThrowingOutputStream.isClosed());
              }
          }

          public static class ThrowingOutputStream extends OutputStream {
              private static boolean closed = false;

              public static boolean isClosed() {
                  return closed;
              }

              @Override
              public void write(final int b) throws IOException {
                  throw new IOException();
              }

              @Override
              public void close() throws IOException {
                  closed = true;
              }
          }

      }

      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      Declaring in multiple statements:

              try (OutputStream out1 = new ThrowingOutputStream();
                   GZIPOutputStream out2 = new GZIPOutputStream(out1)) {

      This yields "Stream closed? true", but I assume many developers will not know they need to do this (proof - even Oracle doesn't, see http://www.oracle.com/technetwork/articles/java/trywithresources-401775.html )

      Attachments

        Activity

          People

            smarks Stuart Marks
            webbuggrp Webbug Group
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: