From 842710608e49693a059f37a1d76197ac77c424b4 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 13 Dec 2016 14:02:04 -0800
Subject: [PATCH] Don't combine automatic output compression with
 "Content-Length"

Summary:
Fixes T12013. Send either "Content-Length" or enable output compression, but not both.

Prefer compression for static resources (CSS, JS, etc).

Test Plan: Ran `curl -v ...`, no longer saw responses with both compression and `Content-Length`.

Reviewers: chad, avivey

Reviewed By: avivey

Subscribers: avivey

Maniphest Tasks: T12013

Differential Revision: https://secure.phabricator.com/D17045
---
 src/aphront/response/AphrontFileResponse.php   | 18 +++++++++++++++++-
 src/aphront/response/AphrontResponse.php       | 15 +++++++++++++++
 src/aphront/sink/AphrontHTTPSink.php           |  2 ++
 .../controller/CelerityResourceController.php  |  7 ++++---
 support/PhabricatorStartup.php                 |  5 -----
 5 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/src/aphront/response/AphrontFileResponse.php b/src/aphront/response/AphrontFileResponse.php
index 4688e3bc8f..a8f673d218 100644
--- a/src/aphront/response/AphrontFileResponse.php
+++ b/src/aphront/response/AphrontFileResponse.php
@@ -5,6 +5,7 @@ final class AphrontFileResponse extends AphrontResponse {
   private $content;
   private $contentIterator;
   private $contentLength;
+  private $compressResponse;
 
   private $mimeType;
   private $download;
@@ -69,6 +70,15 @@ final class AphrontFileResponse extends AphrontResponse {
     return $this->contentLength;
   }
 
+  public function setCompressResponse($compress_response) {
+    $this->compressResponse = $compress_response;
+    return $this;
+  }
+
+  public function getCompressResponse() {
+    return $this->compressResponse;
+  }
+
   public function setRange($min, $max) {
     $this->rangeMin = $min;
     $this->rangeMax = $max;
@@ -94,7 +104,9 @@ final class AphrontFileResponse extends AphrontResponse {
       $content_len = $this->getContentLength();
     }
 
-    $headers[] = array('Content-Length', $this->getContentLength());
+    if (!$this->shouldCompressResponse()) {
+      $headers[] = array('Content-Length', $this->getContentLength());
+    }
 
     if (strlen($this->getDownload())) {
       $headers[] = array('X-Download-Options', 'noopen');
@@ -118,4 +130,8 @@ final class AphrontFileResponse extends AphrontResponse {
     return $headers;
   }
 
+  protected function shouldCompressResponse() {
+    return $this->getCompressResponse();
+  }
+
 }
diff --git a/src/aphront/response/AphrontResponse.php b/src/aphront/response/AphrontResponse.php
index b213244f1c..6c52e417e3 100644
--- a/src/aphront/response/AphrontResponse.php
+++ b/src/aphront/response/AphrontResponse.php
@@ -242,6 +242,21 @@ abstract class AphrontResponse extends Phobject {
     return gmdate('D, d M Y H:i:s', $epoch_timestamp).' GMT';
   }
 
+  protected function shouldCompressResponse() {
+    return true;
+  }
+
+  public function willBeginWrite() {
+    if ($this->shouldCompressResponse()) {
+      // Enable automatic compression here. Webservers sometimes do this for
+      // us, but we now detect the absence of compression and warn users about
+      // it so try to cover our bases more thoroughly.
+      ini_set('zlib.output_compression', 1);
+    } else {
+      ini_set('zlib.output_compression', 0);
+    }
+  }
+
   public function didCompleteWrite($aborted) {
     return;
   }
diff --git a/src/aphront/sink/AphrontHTTPSink.php b/src/aphront/sink/AphrontHTTPSink.php
index 4a729da16e..11c6466cf1 100644
--- a/src/aphront/sink/AphrontHTTPSink.php
+++ b/src/aphront/sink/AphrontHTTPSink.php
@@ -96,6 +96,8 @@ abstract class AphrontHTTPSink extends Phobject {
    * @return void
    */
   final public function writeResponse(AphrontResponse $response) {
+    $response->willBeginWrite();
+
     // Build the content iterator first, in case it throws. Ideally, we'd
     // prefer to handle exceptions before we emit the response status or any
     // HTTP headers.
diff --git a/src/applications/celerity/controller/CelerityResourceController.php b/src/applications/celerity/controller/CelerityResourceController.php
index 95d674c641..0f1478ec5c 100644
--- a/src/applications/celerity/controller/CelerityResourceController.php
+++ b/src/applications/celerity/controller/CelerityResourceController.php
@@ -103,9 +103,10 @@ abstract class CelerityResourceController extends PhabricatorController {
       }
     }
 
-    $response = new AphrontFileResponse();
-    $response->setContent($data);
-    $response->setMimeType($type_map[$type]);
+    $response = id(new AphrontFileResponse())
+      ->setContent($data)
+      ->setMimeType($type_map[$type])
+      ->setCompressResponse(true);
 
     // NOTE: This is a piece of magic required to make WOFF fonts work in
     // Firefox and IE. Possibly we should generalize this more.
diff --git a/support/PhabricatorStartup.php b/support/PhabricatorStartup.php
index 26c6b26a9c..0a27a616e3 100644
--- a/support/PhabricatorStartup.php
+++ b/support/PhabricatorStartup.php
@@ -395,11 +395,6 @@ final class PhabricatorStartup {
     if (function_exists('libxml_disable_entity_loader')) {
       libxml_disable_entity_loader(true);
     }
-
-    // Enable automatic compression here. Webservers sometimes do this for
-    // us, but we now detect the absence of compression and warn users about
-    // it so try to cover our bases more thoroughly.
-    ini_set('zlib.output_compression', 1);
   }