From a24f001b08effffee0848f0f1413b85f3d265a40 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 18 Mar 2016 06:22:37 -0700 Subject: [PATCH] Support pushing data into Git LFS Summary: Ref T7789. Ref T10604. This implements the `upload` action, which streams file data into Files. This makes Git LFS actually work, at least roughly. Test Plan: - Tracked files in an LFS repository. - Pushed LFS data (`git lfs track '*.png'; git add something.png; git commit -m ...; git push`). - Pulled LFS data (`git checkout master^; rm -rf .git/lfs; git checkout master; open something.png`). - Verified LFS refs show up in the gitlfsref table. Reviewers: chad Reviewed By: chad Maniphest Tasks: T7789, T10604 Differential Revision: https://secure.phabricator.com/D15492 --- src/__phutil_library_map__.php | 2 + .../controller/DiffusionServeController.php | 103 +++++++++++++++++- .../PhabricatorFileUploadSource.php | 4 +- .../PhabricatorIteratorFileUploadSource.php | 25 +++++ 4 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 src/applications/files/uploadsource/PhabricatorIteratorFileUploadSource.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 38f9505b24..34ec91844d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2501,6 +2501,7 @@ phutil_register_library_map(array( 'PhabricatorInvalidConfigSetupCheck' => 'applications/config/check/PhabricatorInvalidConfigSetupCheck.php', 'PhabricatorIteratedMD5PasswordHasher' => 'infrastructure/util/password/PhabricatorIteratedMD5PasswordHasher.php', 'PhabricatorIteratedMD5PasswordHasherTestCase' => 'infrastructure/util/password/__tests__/PhabricatorIteratedMD5PasswordHasherTestCase.php', + 'PhabricatorIteratorFileUploadSource' => 'applications/files/uploadsource/PhabricatorIteratorFileUploadSource.php', 'PhabricatorJIRAAuthProvider' => 'applications/auth/provider/PhabricatorJIRAAuthProvider.php', 'PhabricatorJavelinLinter' => 'infrastructure/lint/linter/PhabricatorJavelinLinter.php', 'PhabricatorJiraIssueHasObjectEdgeType' => 'applications/doorkeeper/edge/PhabricatorJiraIssueHasObjectEdgeType.php', @@ -6947,6 +6948,7 @@ phutil_register_library_map(array( 'PhabricatorInvalidConfigSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorIteratedMD5PasswordHasher' => 'PhabricatorPasswordHasher', 'PhabricatorIteratedMD5PasswordHasherTestCase' => 'PhabricatorTestCase', + 'PhabricatorIteratorFileUploadSource' => 'PhabricatorFileUploadSource', 'PhabricatorJIRAAuthProvider' => 'PhabricatorOAuth1AuthProvider', 'PhabricatorJavelinLinter' => 'ArcanistLinter', 'PhabricatorJiraIssueHasObjectEdgeType' => 'PhabricatorEdgeType', diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php index 9fca803008..31bcb2cbed 100644 --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -891,7 +891,12 @@ final class DiffusionServeController extends DiffusionController { } $path = $this->getGitLFSRequestPath($repository); - if ($path == 'objects/batch') { + $matches = null; + + if (preg_match('(^upload/(.*)\z)', $path, $matches)) { + $oid = $matches[1]; + return $this->serveGitLFSUploadRequest($repository, $viewer, $oid); + } else if ($path == 'objects/batch') { return $this->serveGitLFSBatchRequest($repository, $viewer); } else { return DiffusionGitLFSResponse::newErrorResponse( @@ -947,7 +952,7 @@ final class DiffusionServeController extends DiffusionController { if ($file_phids) { $files = id(new PhabricatorFileQuery()) ->setViewer($viewer) - ->withPHIDs(array($file_phids)) + ->withPHIDs($file_phids) ->execute(); $files = mpull($files, null, 'getPHID'); } else { @@ -960,6 +965,7 @@ final class DiffusionServeController extends DiffusionController { $oid = idx($object, 'oid'); $size = idx($object, 'size'); $ref = idx($refs, $oid); + $error = null; // NOTE: If we already have a ref for this object, we only emit a // "download" action. The client should not upload the file again. @@ -968,9 +974,26 @@ final class DiffusionServeController extends DiffusionController { if ($ref) { $file = idx($files, $ref->getFilePHID()); if ($file) { + // Git LFS may prompt users for authentication if the action does + // not provide an "Authorization" header and does not have a query + // parameter named "token". See here for discussion: + // + $no_authorization = 'Basic '.base64_encode('none'); + $get_uri = $file->getCDNURIWithToken(); $actions['download'] = array( 'href' => $get_uri, + 'header' => array( + 'Authorization' => $no_authorization, + ), + ); + } else { + $error = array( + 'code' => 404, + 'message' => pht( + 'Object "%s" was previously uploaded, but no longer exists '. + 'on this server.', + $oid), ); } } else if ($want_upload) { @@ -995,11 +1018,20 @@ final class DiffusionServeController extends DiffusionController { ); } - $output[] = array( + $object = array( 'oid' => $oid, 'size' => $size, - 'actions' => $actions, ); + + if ($actions) { + $object['actions'] = $actions; + } + + if ($error) { + $object['error'] = $error; + } + + $output[] = $object; } $output = array( @@ -1010,6 +1042,69 @@ final class DiffusionServeController extends DiffusionController { ->setContent($output); } + private function serveGitLFSUploadRequest( + PhabricatorRepository $repository, + PhabricatorUser $viewer, + $oid) { + + $ref = id(new PhabricatorRepositoryGitLFSRefQuery()) + ->setViewer($viewer) + ->withRepositoryPHIDs(array($repository->getPHID())) + ->withObjectHashes(array($oid)) + ->executeOne(); + if ($ref) { + return DiffusionGitLFSResponse::newErrorResponse( + 405, + pht( + 'Content for object "%s" is already known to this server. It can '. + 'not be uploaded again.', + $oid)); + } + + $request_stream = new AphrontRequestStream(); + $request_iterator = $request_stream->getIterator(); + $hashing_iterator = id(new PhutilHashingIterator($request_iterator)) + ->setAlgorithm('sha256'); + + $source = id(new PhabricatorIteratorFileUploadSource()) + ->setName('lfs-'.$oid) + ->setViewPolicy(PhabricatorPolicies::POLICY_NOONE) + ->setIterator($hashing_iterator); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $file = $source->uploadFile(); + unset($unguarded); + + $hash = $hashing_iterator->getHash(); + if ($hash !== $oid) { + return DiffusionGitLFSResponse::newErrorResponse( + 400, + pht( + 'Uploaded data is corrupt or invalid. Expected hash "%s", actual '. + 'hash "%s".', + $oid, + $hash)); + } + + $ref = id(new PhabricatorRepositoryGitLFSRef()) + ->setRepositoryPHID($repository->getPHID()) + ->setObjectHash($hash) + ->setByteSize($file->getByteSize()) + ->setAuthorPHID($viewer->getPHID()) + ->setFilePHID($file->getPHID()); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + // Attach the file to the repository to give users permission + // to access it. + $file->attachToObject($repository->getPHID()); + $ref->save(); + unset($unguarded); + + // This is just a plain HTTP 200 with no content, which is what `git lfs` + // expects. + return new DiffusionGitLFSResponse(); + } + private function newGitLFSHTTPAuthorization( PhabricatorRepository $repository, PhabricatorUser $viewer, diff --git a/src/applications/files/uploadsource/PhabricatorFileUploadSource.php b/src/applications/files/uploadsource/PhabricatorFileUploadSource.php index 086fd3345e..1d38402066 100644 --- a/src/applications/files/uploadsource/PhabricatorFileUploadSource.php +++ b/src/applications/files/uploadsource/PhabricatorFileUploadSource.php @@ -72,7 +72,9 @@ abstract class PhabricatorFileUploadSource $data->rewind(); $this->didRewind = true; } else { - $data->next(); + if ($data->valid()) { + $data->next(); + } } if (!$data->valid()) { diff --git a/src/applications/files/uploadsource/PhabricatorIteratorFileUploadSource.php b/src/applications/files/uploadsource/PhabricatorIteratorFileUploadSource.php new file mode 100644 index 0000000000..b8d18936e6 --- /dev/null +++ b/src/applications/files/uploadsource/PhabricatorIteratorFileUploadSource.php @@ -0,0 +1,25 @@ +iterator = $iterator; + return $this; + } + + public function getIterator() { + return $this->iterator; + } + + protected function newDataIterator() { + return $this->getIterator(); + } + + protected function getDataLength() { + return null; + } + +} -- 2.51.2