Bug 51475 - GzipInterceptor#decompress is not completly implemented
Summary: GzipInterceptor#decompress is not completly implemented
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Cluster (show other bugs)
Version: 6.0.29
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-05 09:58 UTC by Christian Stöber
Modified: 2011-07-21 17:09 UTC (History)
1 user (show)



Attachments
Patch file für GzipInterceptor (5.41 KB, patch)
2011-07-05 09:58 UTC, Christian Stöber
Details | Diff
Patch file for GzipInterceptor, TestGzipInterceptor and TribesTestSuite (5.42 KB, text/plain)
2011-07-05 12:08 UTC, Christian Stöber
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Stöber 2011-07-05 09:58:12 UTC
Created attachment 27254 [details]
Patch file für GzipInterceptor

While testing the GzipInterceptor I was wondering about the strange errors: "Unable to decompress byte contents". At first I thougt, I was using the interceptor in wrong order. But taking a look the source shows a TODO at method decompress: "Fix to create an automatically growing buffer."

Because we have session properties which are compressed larger than 2048 bytes, the interceptor didn't work, in our case.

I've created a patch. See my attachement.
Comment 1 Mark Thomas 2011-07-05 10:29:53 UTC
There are a number of issues with the patch as currently proposed.
- It uses tabs rather than spaces
- In includes whitespace changes that add noise
- Tests should be provided as JUnit tests in the separate tests directory in the source tree
Comment 2 Christian Stöber 2011-07-05 12:08:55 UTC
Created attachment 27256 [details]
Patch file for GzipInterceptor, TestGzipInterceptor and TribesTestSuite

Okay. I've checked out Tomcat 6.0.x from subversion in my ecplise and created a new patch through eclipse. For me the patch looks better now.
I've also removed the test code from GzipInterceptor and have written a small JUnit test case.
Comment 3 Mark Thomas 2011-07-05 16:48:34 UTC
Thanks for the updated patch. There are still some unnecessary changes in that patch that distract from the changes that fix the problem. Also, new files need to include a license header.

I have fixed 7.0.x based on your patch and the fix will be included in 7.0.18 onwards.

I have also proposed the fix for 6.0.x.
Comment 4 Christian Stöber 2011-07-15 09:55:47 UTC
Mark,

because it's not desirable and efficient to compress small packages, I've modified the code that it compress only packages with a size greater than 2048 bytes.

Should I file a new bug? Or should I send you an new patch?
Comment 5 Mark Thomas 2011-07-15 10:16:52 UTC
New bug please. This should be user configurable. I'd suggest an attribute name of minCompressionSize
Comment 6 Christian Stöber 2011-07-15 10:25:18 UTC
Ok, I'll will file a bug.

It is user configurable. Based on the compression option in org.apache.coyote.http11.Http11Protocoll I called it compressionMinSize. ;)
Comment 7 Mark Thomas 2011-07-21 17:09:14 UTC
Fixed in 6.0.x and will be included in 6.0.33 onwards.