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

…se of mod_deflate.

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

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) {