Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Don't be too strict on client Content-Length, it could be wrong becau…

…se of mod_deflate.
  • Loading branch information...
commit e795d484ffda4dabb35608f5da2cc0ae03a90fe5 1 parent e973b93
@FooBarWidget FooBarWidget authored
Showing with 20 additions and 33 deletions.
  1. +7 −0 NEWS
  2. +13 −33 ext/apache2/Hooks.cpp
View
7 NEWS
@@ -10,6 +10,13 @@ Release 2.2.15
...after which Phusion Passenger breaks because that directory is
necessary for it to function properly. The cause of this problem
has been found and has been fixed.
+ * [Apache] Fixed some upload handling problems
+ Previous versions of Phusion Passenger check whether the size of
+ the received upload data matches the contents of the Content-Length
+ header as received by the client. It turns out that there could
+ be a mismatch e.g. because of mod_deflate input compression, so
+ we can't trust Content-Length anyway and we're being too strict.
+ The check has now been removed.
* [Nginx] Fixed compilation issues with Nginx 0.7.66
Thanks to Potamianos Gregory for reporting this issue. Issue #500.
* [Nginx] Default Nginx version changed to 0.7.66
View
46 ext/apache2/Hooks.cpp
@@ -506,17 +506,23 @@ class Hooks {
*/
if (expectingUploadData) {
if (contentLength == NULL || atol(contentLength) > LARGE_UPLOAD_THRESHOLD) {
- uploadDataFile = receiveRequestBody(r, contentLength);
+ uploadDataFile = receiveRequestBody(r);
} else {
receiveRequestBody(r, contentLength, uploadDataMemory);
}
}
- if (expectingUploadData && contentLength == NULL) {
- /* In case of "chunked" transfer encoding, we'll set the
- * Content-Length header to the length of the received upload
- * data. Rails requires this header for its HTTP upload data
- * multipart parsing process.
+ if (expectingUploadData) {
+ /* We'll set the Content-Length header to the length of the
+ * received upload data. Rails requires this header for its
+ * HTTP upload data multipart parsing process.
+ * There are two reasons why we don't rely on the Content-Length
+ * header as sent by the client:
+ * - The client doesn't always send Content-Length, e.g. in case
+ * of "chunked" transfer encoding.
+ * - mod_deflate input compression can make it so that the
+ * amount of upload we receive doesn't match Content-Length:
+ * http://httpd.apache.org/docs/2.0/mod/mod_deflate.html#enable
*/
if (uploadDataFile != NULL) {
apr_table_set(r->headers_in, "Content-Length",
@@ -1088,15 +1094,11 @@ class Hooks {
* Receive the HTTP upload data and buffer it into a BufferedUpload temp file.
*
* @param r The request.
- * @param contentLength The value of the HTTP Content-Length header. This is used
- * to check whether the HTTP client has sent complete upload
- * data. NULL indicates that there is no Content-Length header,
- * i.e. that the HTTP client used chunked transfer encoding.
* @throws RuntimeException
* @throws SystemException
* @throws IOException
*/
- shared_ptr<BufferedUpload> receiveRequestBody(request_rec *r, const char *contentLength) {
+ shared_ptr<BufferedUpload> receiveRequestBody(request_rec *r) {
TRACE_POINT();
DirConfig *config = getDirConfig(r);
shared_ptr<BufferedUpload> tempFile;
@@ -1121,17 +1123,6 @@ class Hooks {
} while (written < (size_t) len);
total_written += written;
}
-
- if (contentLength != NULL && ftell(tempFile->handle) != atol(contentLength)) {
- string message = "It looks like the browser did not finish the file upload: "
- "it said it will upload ";
- message.append(contentLength);
- message.append(" bytes, but it closed the connection after sending ");
- message.append(toString(ftell(tempFile->handle)));
- message.append(" bytes. The user probably clicked Stop in the browser "
- "or his Internet connection stalled.");
- throw IOException(message);
- }
return tempFile;
}
@@ -1162,17 +1153,6 @@ class Hooks {
while ((len = readRequestBodyFromApache(r, buf, sizeof(buf))) > 0) {
buffer.append(buf, len);
}
-
- if (contentLength != NULL && buffer.size() != l_contentLength) {
- string message = "It looks like the browser did not finish the file upload: "
- "it said it will upload ";
- message.append(contentLength);
- message.append(" bytes, but it closed the connection after sending ");
- message.append(toString(buffer.size()));
- message.append(" bytes. The user probably clicked Stop in the browser "
- "or his Internet connection stalled.");
- throw IOException(message);
- }
}
void sendRequestBody(request_rec *r, Application::SessionPtr &session, shared_ptr<BufferedUpload> &uploadData) {
Please sign in to comment.
Something went wrong with that request. Please try again.