From 17fc447503af072e713cf96e96c710e6bec749cf Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 19 Jun 2017 14:41:32 -0700 Subject: [PATCH] Don't compute MIME type of noninitial chunks from `diffusion.filecontentquery` Summary: Ref T12857. This is generally fairly fuzzy for now, but here's something concrete: when we build a large file with `diffusion.filecontentquery`, we compute the MIME type of all chunks, not just the initial chunk. Instead, pass a dummy MIME type to non-initial chunks so we don't try to compute them. This mirrors logic elsewhere, in `file.uploadchunk`. This should perhaps be centralized at some point, but it's a bit tricky since the file doesn't know that it's a chunk until later. Also, clean up the `TempFile` immediately -- this shouldn't actually affect anything, but we don't need it to live any longer than this. Test Plan: - Made `hashFileContent()` return `null` to skip the chunk cache. - Added `phlog()` to the MIME type computation. - Loaded a 12MB file in Diffusion. - Before patch: Saw 3x MIME type computations, one for each 4MB chunk. - After patch: Saw 1x MIME type computation, for initial chunk only. Reviewers: chad, amckinley Reviewed By: chad Maniphest Tasks: T12857 Differential Revision: https://secure.phabricator.com/D18138 --- .../files/storage/PhabricatorFile.php | 1 + .../PhabricatorFileUploadSource.php | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index 9fdae4445a..19f0bed90a 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -393,6 +393,7 @@ final class PhabricatorFile extends PhabricatorFileDAO $tmp = new TempFile(); Filesystem::writeFile($tmp, $data); $file->setMimeType(Filesystem::getMimeType($tmp)); + unset($tmp); } try { diff --git a/src/applications/files/uploadsource/PhabricatorFileUploadSource.php b/src/applications/files/uploadsource/PhabricatorFileUploadSource.php index 5e72156091..e3242af329 100644 --- a/src/applications/files/uploadsource/PhabricatorFileUploadSource.php +++ b/src/applications/files/uploadsource/PhabricatorFileUploadSource.php @@ -186,12 +186,18 @@ abstract class PhabricatorFileUploadSource $actual_length = strlen($data); $rope->removeBytesFromHead($actual_length); - $chunk_data = PhabricatorFile::newFromFileData( - $data, - array( - 'name' => $file->getMonogram().'.chunk-'.$offset, - 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, - )); + $params = array( + 'name' => $file->getMonogram().'.chunk-'.$offset, + 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, + ); + + // If this isn't the initial chunk, provide a dummy MIME type so we do not + // try to detect it. See T12857. + if ($offset > 0) { + $params['mime-type'] = 'application/octet-stream'; + } + + $chunk_data = PhabricatorFile::newFromFileData($data, $params); $chunk = PhabricatorFileChunk::initializeNewChunk( $file->getStorageHandle(), -- 2.51.2