Bug 22104 - mod_deflate does not change Content-Length, breaks DAV file upload via PUT
Summary: mod_deflate does not change Content-Length, breaks DAV file upload via PUT
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_dav (show other bugs)
Version: 2.0.47
Hardware: All other
: P3 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
Depends on:
Reported: 2003-08-04 10:57 UTC by Tim Robbins
Modified: 2004-11-16 19:05 UTC (History)
0 users

Patch to make mod_dav use bucket brigades when handling PUT requests (2.98 KB, patch)
2003-08-04 13:22 UTC, Tim Robbins
Details | Diff
Call apr_brigade_cleanup() to avoid running out of buffer space (3.01 KB, patch)
2003-08-04 13:39 UTC, Tim Robbins
Details | Diff
clean patch I wrote in the meantime :) (4.29 KB, patch)
2003-08-04 14:09 UTC, André Malo
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Robbins 2003-08-04 10:57:46 UTC
Although it is well documented that mod_deflate does not modify the Content-
Length header when decoding a gzip-compressed message body, the incorrect value 
seems to confuse mod_dav -- when I try to upload a compressed message body with 
the PUT method, mod_dav returns a 400 Bad Request error, and this message 
appears in the log:

[Mon Aug 04 19:49:25 2003] [error] [client] An error occurred 
while reading the request body.  [400, #0]

To test, I'm PUT'ing the output of "echo test | gzip -9c". What happens is that 
dav_method_put()'s first call to ap_get_client_block() returns 5 (strlen
("test\n")), but then it calls ap_get_client_block() again, and it returns -1 
to signal an error. I believe this is because it's expecting more data.

I'm not sure how to solve this problem. It's probably not a great idea to 
buffer the whole body in memory then fix up the Content-Length header after 
decompressing it all, so perhaps mod_deflate could fake up a "chunked"-encoded 
body, putting each block of data returned from zlib into a new chunk, and 
signalling EOF with a 0-length chunk. I'm not familiar with the internals of 
the Apache httpd, so I'm not sure.

I'm filing this as "Enhancement" because although the problem is documented, it 
would be really nice to be able to use PUT requests w/ gzip-compressed bodies.
Comment 1 André Malo 2003-08-04 11:19:37 UTC
mod_dav should use bucket brigades when reading PUT data, then all should be fine.

So changing component to mod_dav and accept this as a bug.

Thanks for the report.
Comment 2 Tim Robbins 2003-08-04 13:22:16 UTC
Created attachment 7641 [details]
Patch to make mod_dav use bucket brigades when handling PUT requests
Comment 3 Tim Robbins 2003-08-04 13:28:17 UTC
Thanks for the suggestion to use bucket brigades -- I've got compressed uploads 
at least partly working now, but it seems to choke on large requests:
[Mon Aug 04 23:21:13 2003] [error] [client] (55)No buffer space 
available: Could not get brigade.  [500, #0]

There are other things I'm not quite sure about in the patch, but I thought I'd 
upload it just to get the ball rolling.
Comment 4 Tim Robbins 2003-08-04 13:39:53 UTC
Created attachment 7642 [details]
Call apr_brigade_cleanup() to avoid running out of buffer space
Comment 5 André Malo 2003-08-04 14:09:48 UTC
Created attachment 7643 [details]
clean patch I wrote in the meantime :)
Comment 6 André Malo 2003-08-04 14:11:50 UTC
Yeah, it was probably the missing cleanup. Can you try my patch nevertheless?

If it works, I'm going to commit it in 2.1 and propose it for backport.
Comment 7 Tim Robbins 2003-08-05 10:04:48 UTC
This seems to work, thanks! I haven't performed exhaustive testing; I uploaded 
a 15MB file, both with and without gzip compression, and verified that the 
files were identical and had the same md5sum as the original file.
Comment 8 André Malo 2003-08-06 14:23:19 UTC
It's essentially the same patch as yours :)

Thanks for testing so far, I'm going to commit now.
Comment 9 Jeff Trawick 2003-11-21 22:32:38 UTC
fix already committed to Apache 2.1-dev...  sounds like we need to consider it
for merge to 2.0.next
Comment 10 André Malo 2003-11-21 22:36:18 UTC
It's considered already, but the review you know ... :-))
Comment 11 Joe Orton 2004-01-08 12:58:49 UTC
Backport now in 2.0 tree courtesy of Justin...