From fbc4967154ba1f647cb31a0745a1d52518749955 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 27 Apr 2016 06:42:32 -0700
Subject: [PATCH] Improve cache behaviors for font files and other nonstandard
 resource types

Summary:
Ref T10843. There are actually two separate notions of cacheability here:

  - Is this cacheable by the browser (e.g., should we emit "Expires: long in the future")?
  - Is this cacheable locally (e.g., should we stick it in APC, or just read it off disk every time)?

These got a little mixed up by D15775, so we aren't currently emitting proper "Expires" headers on font files and a few other resource types.

Straighten this out so that we "Expires" these unusual resources correctly.

Test Plan: Verified that `.woff` files get a proper "Expires" header now, not just CSS/JS.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10843

Differential Revision: https://secure.phabricator.com/D15807
---
 .../celerity/controller/CelerityResourceController.php    | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/applications/celerity/controller/CelerityResourceController.php b/src/applications/celerity/controller/CelerityResourceController.php
index debb37c1ca..c1ea652084 100644
--- a/src/applications/celerity/controller/CelerityResourceController.php
+++ b/src/applications/celerity/controller/CelerityResourceController.php
@@ -50,8 +50,8 @@ abstract class CelerityResourceController extends PhabricatorController {
     // is not, refuse to cache this resource. This avoids poisoning caches
     // and CDNs if we're getting a request for a new resource to an old node
     // shortly after a push.
-    $is_cacheable = ($hash === $expect_hash) &&
-                    $this->isCacheableResourceType($type);
+    $is_cacheable = ($hash === $expect_hash);
+    $is_locally_cacheable = $this->isLocallyCacheableResourceType($type);
     if (AphrontRequest::getHTTPHeader('If-Modified-Since') && $is_cacheable) {
       // Return a "304 Not Modified". We don't care about the value of this
       // field since we never change what resource is served by a given URI.
@@ -60,7 +60,7 @@ abstract class CelerityResourceController extends PhabricatorController {
 
     $cache = null;
     $data = null;
-    if ($is_cacheable && !$dev_mode) {
+    if ($is_cacheable && $is_locally_cacheable && !$dev_mode) {
       $cache = PhabricatorCaches::getImmutableCache();
 
       $request_path = $this->getRequest()->getPath();
@@ -168,7 +168,7 @@ abstract class CelerityResourceController extends PhabricatorController {
    * @param string  Resource type.
    * @return bool   True to enable caching.
    */
-  private function isCacheableResourceType($type) {
+  private function isLocallyCacheableResourceType($type) {
     $types = array(
       'js' => true,
       'css' => true,