tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kkoli...@apache.org
Subject svn commit: r1201193 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/coyote/http11/filters/FlushableGZIPOutputStream.java webapps/docs/changelog.xml
Date Sat, 12 Nov 2011 05:26:42 GMT
Author: kkolinko
Date: Sat Nov 12 05:26:42 2011
New Revision: 1201193

URL: http://svn.apache.org/viewvc?rev=1201193&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52121
Fix possible output corruption when compression is enabled for a connector and the response
is flushed.

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/filters/FlushableGZIPOutputStream.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1201193&r1=1201192&r2=1201193&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Sat Nov 12 05:26:42 2011
@@ -64,13 +64,6 @@ PATCHES PROPOSED TO BACKPORT:
       - getStuckThreadIds() returns a list of ids. It might be useful to
         have a similar method that returns Thread.getName() names.
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52121
-  Fix possible output corruption when compression is
-  enabled for a connector and the response is flushed.
-  https://issues.apache.org/bugzilla/attachment.cgi?id=27924
-  +1: kkolinko, rjung, jfclere
-  -1:
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50570
   Apply FIPS mode patch from TC7:
   http://svn.apache.org/viewvc?rev=1199985&view=rev

Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/filters/FlushableGZIPOutputStream.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/filters/FlushableGZIPOutputStream.java?rev=1201193&r1=1201192&r2=1201193&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/filters/FlushableGZIPOutputStream.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/http11/filters/FlushableGZIPOutputStream.java
Sat Nov 12 05:26:42 2011
@@ -35,58 +35,93 @@ public class FlushableGZIPOutputStream e
         super(os);
     }
 
-    private static final byte[] EMPTYBYTEARRAY = new byte[0];
-    private boolean hasData = false;
-
     /**
-     * Here we make sure we have received data, so that the header has been for
-     * sure written to the output stream already.
+     * It is used to reserve one byte of real data so that it can be used when
+     * flushing the stream.
      */
+    private byte[] lastByte = new byte[1];
+    private boolean hasLastByte = false;
+
+    @Override
+    public void write(byte[] bytes) throws IOException {
+        write(bytes, 0, bytes.length);
+    }
+
     @Override
-    public synchronized void write(byte[] bytes, int i, int i1)
+    public synchronized void write(byte[] bytes, int offset, int length)
             throws IOException {
-        super.write(bytes, i, i1);
-        hasData = true;
+        if (length > 0) {
+            flushLastByte();
+            if (length > 1) {
+                super.write(bytes, offset, length - 1);
+            }
+            rememberLastByte(bytes[offset + length - 1]);
+        }
     }
 
     @Override
     public synchronized void write(int i) throws IOException {
-        super.write(i);
-        hasData = true;
+        flushLastByte();
+        rememberLastByte((byte) i);
     }
 
     @Override
-    public synchronized void write(byte[] bytes) throws IOException {
-        super.write(bytes);
-        hasData = true;
+    public synchronized void finish() throws IOException {
+        try {
+            flushLastByte();
+        } catch (IOException ignore) {
+            // If our write failed, then trailer write in finish() will fail
+            // with IOException as well, but it will leave Deflater in more
+            // consistent state.
+        }
+        super.finish();
     }
 
     @Override
-    public synchronized void flush() throws IOException {
-        if (!hasData) {
-            return; // do not allow the gzip header to be flushed on its own
+    public synchronized void close() throws IOException {
+        try {
+            flushLastByte();
+        } catch (IOException ignored) {
+            // Ignore. As OutputStream#close() says, the contract of close()
+            // is to close the stream. It does not matter much if the
+            // stream is not writable any more.
         }
+        super.close();
+    }
 
-        // trick the deflater to flush
-        /**
-         * Now this is tricky: We force the Deflater to flush its data by
-         * switching compression level. As yet, a perplexingly simple workaround
-         * for
-         * http://developer.java.sun.com/developer/bugParade/bugs/4255743.html
-         */
-        if (!def.finished()) {
-            def.setInput(EMPTYBYTEARRAY, 0, 0);
-
-            def.setLevel(Deflater.NO_COMPRESSION);
-            deflate();
-
-            def.setLevel(Deflater.DEFAULT_COMPRESSION);
-            deflate();
+    private void rememberLastByte(byte b) {
+        lastByte[0] = b;
+        hasLastByte = true;
+    }
 
-            out.flush();
+    private void flushLastByte() throws IOException {
+        if (hasLastByte) {
+            // Clear the flag first, because write() may fail
+            hasLastByte = false;
+            super.write(lastByte, 0, 1);
         }
+    }
 
-        hasData = false; // no more data to flush
+    @Override
+    public synchronized void flush() throws IOException {
+        if (hasLastByte) {
+            // - do not allow the gzip header to be flushed on its own
+            // - do not do anything if there is no data to send
+
+            // trick the deflater to flush
+            /**
+             * Now this is tricky: We force the Deflater to flush its data by
+             * switching compression level. As yet, a perplexingly simple workaround
+             * for
+             * http://developer.java.sun.com/developer/bugParade/bugs/4255743.html
+             */
+            if (!def.finished()) {
+                def.setLevel(Deflater.NO_COMPRESSION);
+                flushLastByte();
+                def.setLevel(Deflater.DEFAULT_COMPRESSION);
+            }
+        }
+        out.flush();
     }
 
     /*

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1201193&r1=1201192&r2=1201193&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Sat Nov 12 05:26:42 2011
@@ -160,6 +160,11 @@
         the socket from 30s to 10s. (kkolinko)
       </fix>
       <fix>
+        <bug>52121</bug>: Fix possible output corruption when compression is
+        enabled for a connector and the response is flushed. Test
+        case provided by David Marcks. (kkolinko)
+      </fix>
+      <fix>
         Replace unneeded call that iterated events queue in NioEndpoint.Poller.
         (kkolinko)
       </fix>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message