Iterate over ranges correctly for encryped files

Summary:
Fixes T12079. Currently, when a file is encrypted and a request has "Content-Range", we apply the range first, //then// decrypt the result. This doesn't work since you can't start decrypting something from somewhere in the middle (at least, not with our cipher selection).

Instead: decrypt the result, //then// apply the range.

Test Plan: Added failing unit tests, made them pass

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12079

Differential Revision: https://secure.phabricator.com/D17623

src/applications/files/engine/PhabricatorChunkedFileStorageEngine.php

@@ -174,7 +174,16 @@ final class PhabricatorChunkedFileStorageEngine
     return (4 * 1024 * 1024);
   }
 
-  public function getRawFileDataIterator(PhabricatorFile $file, $begin, $end) {
+  public function getRawFileDataIterator(
+    PhabricatorFile $file,
+    $begin,
+    $end,
+    PhabricatorFileStorageFormat $format) {
+
+    // NOTE: It is currently impossible for files stored with the chunk
+    // engine to have their own formatting (instead, the individual chunks
+    // are formatted), so we ignore the format object.
+
     $chunks = id(new PhabricatorFileChunkQuery())
       ->setViewer(PhabricatorUser::getOmnipotentUser())
       ->withChunkHandles(array($file->getStorageHandle()))

src/applications/files/engine/PhabricatorFileStorageEngine.php

@@ -325,10 +325,20 @@ abstract class PhabricatorFileStorageEngine extends Phobject {
     return $engine->getChunkSize();
   }
 
-  public function getRawFileDataIterator(PhabricatorFile $file, $begin, $end) {
+  public function getRawFileDataIterator(
-    // The default implementation is trivial and just loads the entire file
+    PhabricatorFile $file,
-    // upfront.
+    $begin,
-    $data = $this->readFile($file->getStorageHandle());
+    $end,
+    PhabricatorFileStorageFormat $format) {
+
+    $formatted_data = $this->readFile($file->getStorageHandle());
+    $formatted_data = array($formatted_data);
+
+    $data = '';
+    $format_iterator = $format->newReadIterator($formatted_data);
+    foreach ($format_iterator as $raw_chunk) {
+      $data .= $raw_chunk;
+    }
 
     if ($begin !== null && $end !== null) {
       $data = substr($data, $begin, ($end - $begin));

src/applications/files/format/__tests__/PhabricatorFileStorageFormatTestCase.php

@@ -33,6 +33,15 @@ final class PhabricatorFileStorageFormatTestCase extends PhabricatorTestCase {
     // The actual raw data in the storage engine should be encoded.
     $raw_data = $engine->readFile($file->getStorageHandle());
     $this->assertEqual($expect, $raw_data);
+
+    // If we generate an iterator over a slice of the file, it should return
+    // the decrypted file.
+    $iterator = $file->getFileDataIterator(4, 14);
+    $raw_data = '';
+    foreach ($iterator as $data_chunk) {
+      $raw_data .= $data_chunk;
+    }
+    $this->assertEqual('cow jumped', $raw_data);
   }
 
   public function testAES256Storage() {
@@ -73,5 +82,14 @@ final class PhabricatorFileStorageFormatTestCase extends PhabricatorTestCase {
     // input data.
     $raw_data = $engine->readFile($file->getStorageHandle());
     $this->assertTrue($data !== $raw_data);
+
+    // If we generate an iterator over a slice of the file, it should return
+    // the decrypted file.
+    $iterator = $file->getFileDataIterator(4, 14);
+    $raw_data = '';
+    foreach ($iterator as $data_chunk) {
+      $raw_data .= $data_chunk;
+    }
+    $this->assertEqual('cow jumped', $raw_data);
   }
 }

src/applications/files/storage/PhabricatorFile.php

@@ -746,14 +746,18 @@ final class PhabricatorFile extends PhabricatorFileDAO
    */
   public function getFileDataIterator($begin = null, $end = null) {
     $engine = $this->instantiateStorageEngine();
-    $raw_iterator = $engine->getRawFileDataIterator($this, $begin, $end);
 
     $key = $this->getStorageFormat();
-
     $format = id(clone PhabricatorFileStorageFormat::requireFormat($key))
       ->setFile($this);
 
-    return $format->newReadIterator($raw_iterator);
+    $iterator = $engine->getRawFileDataIterator(
+      $this,
+      $begin,
+      $end,
+      $format);
+
+    return $iterator;
   }
 
   public function getURI() {