From 01862b8f23ac47ae14d464c6005262a3f43d3915 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 30 Jun 2016 13:26:21 -0700
Subject: [PATCH] Detect the MIME type of large files by examining the first
 chunk

Summary:
Fixes T11242. See that task for detailed discussion.

Previously, it didn't particularly matter that we don't MIME detect chunked files since they were all just big blobs of junk (PSDs, zips/tarballs, whatever) that we handled uniformly.

However, videos are large and the MIME type also matters.

  - Detect the overall mime type by detecitng the MIME type of the first chunk. This appears to work properly, at least for video.
  - Skip mime type detection on other chunks, which we were performing and ignoring. This makes uploading chunked files a little faster since we don't need to write stuff to disk.

Test Plan:
Uploaded a 50MB video locally, saw it as chunks with a "video/mp4" mime type, played it in the browser in Phabricator as an embedded HTML 5 video.

{F1706837}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11242

Differential Revision: https://secure.phabricator.com/D16204
---
 .../FileUploadChunkConduitAPIMethod.php       | 25 ++++++++++++++++++-
 .../files/storage/PhabricatorFile.php         |  6 ++---
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/applications/files/conduit/FileUploadChunkConduitAPIMethod.php b/src/applications/files/conduit/FileUploadChunkConduitAPIMethod.php
index e3444d05e2..949bdde28a 100644
--- a/src/applications/files/conduit/FileUploadChunkConduitAPIMethod.php
+++ b/src/applications/files/conduit/FileUploadChunkConduitAPIMethod.php
@@ -51,6 +51,16 @@ final class FileUploadChunkConduitAPIMethod
       $start,
       $start + $length);
 
+    // If this is the initial chunk, leave the MIME type unset so we detect
+    // it and can update the parent file. If this is any other chunk, it has
+    // no meaningful MIME type. Provide a default type so we can avoid writing
+    // it to disk to perform MIME type detection.
+    if (!$start) {
+      $mime_type = null;
+    } else {
+      $mime_type = 'application/octet-stream';
+    }
+
     // NOTE: These files have a view policy which prevents normal access. They
     // are only accessed through the storage engine.
     $chunk_data = PhabricatorFile::newFromFileData(
@@ -58,13 +68,26 @@ final class FileUploadChunkConduitAPIMethod
       array(
         'name' => $file->getMonogram().'.chunk-'.$chunk->getID(),
         'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
+        'mime-type' => $mime_type,
       ));
 
     $chunk->setDataFilePHID($chunk_data->getPHID())->save();
 
+    $needs_update = false;
+
     $missing = $this->loadAnyMissingChunk($viewer, $file);
     if (!$missing) {
-      $file->setIsPartial(0)->save();
+      $file->setIsPartial(0);
+      $needs_update = true;
+    }
+
+    if (!$start) {
+      $file->setMimeType($chunk_data->getMimeType());
+      $needs_update = true;
+    }
+
+    if ($needs_update) {
+      $file->save();
     }
 
     return null;
diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php
index 4e43e3dd18..7e8f4b18fc 100644
--- a/src/applications/files/storage/PhabricatorFile.php
+++ b/src/applications/files/storage/PhabricatorFile.php
@@ -270,10 +270,8 @@ final class PhabricatorFile extends PhabricatorFileDAO
 
     $file->setByteSize($length);
 
-    // TODO: We might be able to test the first chunk in order to figure
-    // this out more reliably, since MIME detection usually examines headers.
-    // However, enormous files are probably always either actually raw data
-    // or reasonable to treat like raw data.
+    // NOTE: Once we receive the first chunk, we'll detect its MIME type and
+    // update the parent file. This matters for large media files like video.
     $file->setMimeType('application/octet-stream');
 
     $chunked_hash = idx($params, 'chunkedHash');