Page MenuHomePhabricator
Files F13996866

D8655.id20524.diff
No OneTemporary
Actions

D8655.id20524.diff
View Options

diff --git a/src/applications/files/config/PhabricatorFilesConfigOptions.php b/src/applications/files/config/PhabricatorFilesConfigOptions.php
--- a/src/applications/files/config/PhabricatorFilesConfigOptions.php
+++ b/src/applications/files/config/PhabricatorFilesConfigOptions.php
@@ -137,6 +137,13 @@
"Set this to a valid Amazon S3 bucket to store files there. You ".
"must also configure S3 access keys in the 'Amazon Web Services' ".
"group.")),
+ $this->newOption('storage.s3.public-uri', 'string', null)
+ ->setSummary(pht('Amazon S3 / CloudFront public URI.'))
+ ->setDescription(
+ pht(
+ "Set this to the URI prefix for serving files directly up on ".
+ "Amazon S3 or CloudFront. This is used by enabling 'Static ".
+ "Web Hosting' on the Amazon S3 bucket.")),
$this->newOption(
'storage.engine-selector',
'class',
diff --git a/src/applications/files/controller/PhabricatorFileDataController.php b/src/applications/files/controller/PhabricatorFileDataController.php
--- a/src/applications/files/controller/PhabricatorFileDataController.php
+++ b/src/applications/files/controller/PhabricatorFileDataController.php
@@ -41,6 +41,12 @@
return new Aphront403Response();
}
+ // Redirect to the public URI if it's present.
+ $public_uri = $file->getDirectPublicURI();
+ if ($public_uri !== null) {
+ return id(new AphrontRedirectResponse())->setURI($public_uri);
+ }
+
$data = $file->loadFileData();
$response = new AphrontFileResponse();
$response->setContent($data);
diff --git a/src/applications/files/engine/PhabricatorFileStorageEngine.php b/src/applications/files/engine/PhabricatorFileStorageEngine.php
--- a/src/applications/files/engine/PhabricatorFileStorageEngine.php
+++ b/src/applications/files/engine/PhabricatorFileStorageEngine.php
@@ -82,6 +82,23 @@
/**
+ * Retrieve an absolute URI to which a download should be redirected. This
+ * can be used to redirect downloads through a CDN such as Amazon CloudFront
+ * or directly to Amazon S3. This can be much more efficient that
+ * Phabricator downloading the file from Amazon and then sending it to the
+ * user.
+ *
+ * Return null to indicate that no redirection should occur.
+ *
+ * @param string The handle returned from @{method:writeFile} when the
+ * file was written.
+ * @return string An absolute URI to which a download should be redirected.
+ * @task file
+ */
+ abstract public function retrieveFileURI($handle);
+
+
+ /**
* Delete the data for a file previously written by @{method:writeFile}.
*
* @param string The handle returned from @{method:writeFile} when the
diff --git a/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php b/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php
--- a/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php
+++ b/src/applications/files/engine/PhabricatorLocalDiskFileStorageEngine.php
@@ -68,6 +68,15 @@
/**
+ * Return null because there are no direct download URI.
+ * @task impl
+ */
+ public function retrieveFileURI($handle) {
+ return null;
+ }
+
+
+ /**
* Deletes the file from local disk, if it exists.
* @task impl
*/
diff --git a/src/applications/files/engine/PhabricatorMySQLFileStorageEngine.php b/src/applications/files/engine/PhabricatorMySQLFileStorageEngine.php
--- a/src/applications/files/engine/PhabricatorMySQLFileStorageEngine.php
+++ b/src/applications/files/engine/PhabricatorMySQLFileStorageEngine.php
@@ -52,6 +52,15 @@
/**
+ * Return null because there are no direct download URI.
+ * @task impl
+ */
+ public function retrieveFileURI($handle) {
+ return null;
+ }
+
+
+ /**
* Delete a blob from MySQL.
* @task impl
*/
diff --git a/src/applications/files/engine/PhabricatorS3FileStorageEngine.php b/src/applications/files/engine/PhabricatorS3FileStorageEngine.php
--- a/src/applications/files/engine/PhabricatorS3FileStorageEngine.php
+++ b/src/applications/files/engine/PhabricatorS3FileStorageEngine.php
@@ -39,6 +39,16 @@
);
$name = 'phabricator/'.implode('/', $parts);
+ // Append the name parameter if provided, since S3 doesn't provide
+ // redirection rules that we can use to remove any trailing characters,
+ // nor does CloudFront. A user accessing files still needs to know
+ // the seed, so this doesn't allow users to access files based purely
+ // on the name.
+ $filename = idx($params, 'name', null);
+ if ($filename !== null) {
+ $name = $name.'/'.$filename;
+ }
+
AphrontWriteGuard::willWrite();
$s3->putObject(
$data,
@@ -69,6 +79,19 @@
/**
+ * Return absolute public URI for redirected download.
+ * @task impl
+ */
+ public function retrieveFileURI($handle) {
+ $base_uri = $this->getPublicURI();
+ if ($base_uri === null) {
+ return null;
+ }
+ return rtrim($base_uri, '/').'/'.$handle;
+ }
+
+
+ /**
* Delete a blob from Amazon S3.
*/
public function deleteFile($handle) {
@@ -97,6 +120,19 @@
}
/**
+ * Retrieve the public web frontend of S3.
+ *
+ * @task internal
+ */
+ private function getPublicURI() {
+ $bucket = PhabricatorEnv::getEnvConfig('storage.s3.public-uri');
+ if (!$bucket) {
+ return null;
+ }
+ return $bucket;
+ }
+
+ /**
* Create a new S3 API object.
*
* @task internal
diff --git a/src/applications/files/engine/PhabricatorTestStorageEngine.php b/src/applications/files/engine/PhabricatorTestStorageEngine.php
--- a/src/applications/files/engine/PhabricatorTestStorageEngine.php
+++ b/src/applications/files/engine/PhabricatorTestStorageEngine.php
@@ -28,6 +28,10 @@
throw new Exception("No such file with handle '{$handle}'!");
}
+ public function retrieveFileURI($handle) {
+ return null;
+ }
+
public function deleteFile($handle) {
AphrontWriteGuard::willWrite();
unset(self::$storage[$handle]);
diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php
--- a/src/applications/files/storage/PhabricatorFile.php
+++ b/src/applications/files/storage/PhabricatorFile.php
@@ -496,12 +496,21 @@
"You must save a file before you can generate a view URI.");
}
+ // We could potentially do the getDirectPublicURI check here, but
+ // then we'd be unable to do the proper checks that the data
+ // controller does.
+
$name = phutil_escape_uri($this->getName());
$path = '/file/data/'.$this->getSecretKey().'/'.$this->getPHID().'/'.$name;
return PhabricatorEnv::getCDNURI($path);
}
+ public function getDirectPublicURI() {
+ $engine = $this->instantiateStorageEngine();
+ return $engine->retrieveFileURI($this->getStorageHandle());
+ }
+
public function getInfoURI() {
return '/file/info/'.$this->getPHID().'/';
}

File Metadata

Mime Type
text/plain
Expires
Fri, Oct 25, 12:33 AM (2 w, 10 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6747522
Default Alt Text
D8655.id20524.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility