Created attachment 29238 [details] Please see description for purpose of attachment. FlushableGZIPOutputStream gives corrupt output in rare circumstances. Tomcat 7 encounters this bug when compression is turned on. Please see the attachment: Compile and run ErrorCase.java which uses FlushableGZIPOutputStream to GZIP the contents of data.bin (included in the attachment) and then uses GZIPInputStream to gunzip the output of FlushableGZIPOutputStream, resulting in java.io.IOException: Corrupt GZIP trailer.
Thanks for the test case. It made fixing this a whole lot easier than it would otherwise have been. The fix has been applied to trunk and 7.0.x and will be included in 7.0.30 onwards. I haven't included the test case in the unit tests due to the size of the file required to trigger this issue. (It would significantly increase the size of the source distribution.)
Good catch. I proposed backport of this fix (r1377342) to Tomcat 6.0.
Created attachment 29286 [details] Demonstrates the same problem post-fix. Please unzip to obtain the data.bin which causes the issue.
I have attached a new data.bin which demonstrates that the problem remains post-fix.
Note that, not surprisingly, using Java 7's GZIPOutputStream with syncFlush=true resolves the issue.
That is an option for Tomcat 8 (which will require Java 7) but not for Tomcat 7 and earlier.
Very different failure mode (a whole chunk of data is missing from the end). Probably a different bug. I'll start digging.
Easy solution implemented for trunk (Tomcat 8). Still trying to find a fix for Tomcat 7.
Created attachment 29297 [details] 2012-08-29_tc6_53725.patch Thank you for the test! Fixed in 7.0 with r1378378. Will be in 7.0.30. Here is patch for Tomcat 6 (r1377343 + r1378378).
That test (In reply to comment #9) > Created attachment 29297 [details] > 2012-08-29_tc6_53725.patch > > Thank you for the test! > > Fixed in 7.0 with r1378378. Will be in 7.0.30. > > Here is patch for Tomcat 6 (r1377343 + r1378378). While that patch fixed the test for the second test case, it re-broke it for the first. After a lot of trial and error I have a patch that works for both test cases. I'm not at all clear as to the root cause of what is happening as the critical code is native code. The latest patch may still fail for some edge cases.
After some time with dealing with such issues with our own servlet filter for compression and trying various solutions tried in Tomcat, I finally gave up and went with jzlib for pre-Java-7 cases and with the built-in sync-flush functionality in Java 7 cases. I got tired of someone always coming up with some new case where trying to allow flushing with the pre-Java-7 GZipOutputStream caused an issue.
I reverted the fix in r1378402 and implemented a different one in r1378403 + r1378408 (will be in 7.0.30) The FlushableGZIPOutputStream#deflate() method is not called by this class directly, but by its parent class, DeflaterOutputStream. Such calls are always in a loop, while (!def.needsInput()) { deflate(); } while (!def.finished()) { deflate(); } so I there should not be a need to add (!def.needsInput()) condition to the deflate() method itself.
Fixed in 6.0.x and will be included in 6.0.36 onwards.