From b479941ceb6e2034ae8d981886937916884acc06 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 18 Apr 2017 13:17:39 -0700 Subject: [PATCH] Make "Range" HTTP header work for Celerity static resource requests Summary: Ref T7567. In T8266 I fixed a bunch of obscure "Range" issues, but only for file downloads -- not for Celerity. Extend all that stuff to Celerity, which is fortunately much easier. I believe this will fix Conpherence sounds in Safari. Test Plan: - Wrote out an HTTP request in a text file with `Range: bytes=0-1` and similar, piped it to localhost with `cat request.txt | nc localhost 80`, saw server return appropriate range responses consistent with file behavior after T8266, which all seems to work. - Also did that for files to try to make sure I wasn't breaking anything. Reviewers: chad, amckinley Reviewed By: chad Maniphest Tasks: T7567 Differential Revision: https://secure.phabricator.com/D17724 --- src/aphront/response/AphrontFileResponse.php | 25 +++++++++++++++++ .../controller/CelerityResourceController.php | 27 ++++++++++++++++--- .../PhabricatorFileDataController.php | 20 ++------------ 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/src/aphront/response/AphrontFileResponse.php b/src/aphront/response/AphrontFileResponse.php index 9699c49ad4..6bae4c808f 100644 --- a/src/aphront/response/AphrontFileResponse.php +++ b/src/aphront/response/AphrontFileResponse.php @@ -139,4 +139,29 @@ final class AphrontFileResponse extends AphrontResponse { return $this->getCompressResponse(); } + public function parseHTTPRange($range) { + $begin = null; + $end = null; + + $matches = null; + if (preg_match('/^bytes=(\d+)-(\d*)$/', $range, $matches)) { + // Note that the "Range" header specifies bytes differently than + // we do internally: the range 0-1 has 2 bytes (byte 0 and byte 1). + $begin = (int)$matches[1]; + + // The "Range" may be "200-299" or "200-", meaning "until end of file". + if (strlen($matches[2])) { + $range_end = (int)$matches[2]; + $end = $range_end + 1; + } else { + $range_end = null; + } + + $this->setHTTPResponseCode(206); + $this->setRange($begin, $range_end); + } + + return array($begin, $end); + } + } diff --git a/src/applications/celerity/controller/CelerityResourceController.php b/src/applications/celerity/controller/CelerityResourceController.php index 0f1478ec5c..730e5ddc90 100644 --- a/src/applications/celerity/controller/CelerityResourceController.php +++ b/src/applications/celerity/controller/CelerityResourceController.php @@ -104,9 +104,30 @@ abstract class CelerityResourceController extends PhabricatorController { } $response = id(new AphrontFileResponse()) - ->setContent($data) - ->setMimeType($type_map[$type]) - ->setCompressResponse(true); + ->setMimeType($type_map[$type]); + + $range = AphrontRequest::getHTTPHeader('Range'); + + if (strlen($range)) { + $response->setContentLength(strlen($data)); + + list($range_begin, $range_end) = $response->parseHTTPRange($range); + + if ($range_begin !== null) { + if ($range_end !== null) { + $data = substr($data, $range_begin, ($range_end - $range_begin)); + } else { + $data = substr($data, $range_begin); + } + } + + $response->setContentIterator(array($data)); + } else { + $response + ->setContent($data) + ->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/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php index c98c05e27b..c8bfcc488a 100644 --- a/src/applications/files/controller/PhabricatorFileDataController.php +++ b/src/applications/files/controller/PhabricatorFileDataController.php @@ -62,24 +62,8 @@ final class PhabricatorFileDataController extends PhabricatorFileController { // an initial request for bytes 0-1 of the audio file, and things go south // if we can't respond with a 206 Partial Content. $range = $request->getHTTPHeader('range'); - if ($range) { - $matches = null; - if (preg_match('/^bytes=(\d+)-(\d*)$/', $range, $matches)) { - // Note that the "Range" header specifies bytes differently than - // we do internally: the range 0-1 has 2 bytes (byte 0 and byte 1). - $begin = (int)$matches[1]; - - // The "Range" may be "200-299" or "200-", meaning "until end of file". - if (strlen($matches[2])) { - $range_end = (int)$matches[2]; - $end = $range_end + 1; - } else { - $range_end = null; - } - - $response->setHTTPResponseCode(206); - $response->setRange($begin, $range_end); - } + if (strlen($range)) { + list($begin, $end) = $response->parseHTTPRange($range); } $is_viewable = $file->isViewableInBrowser();