Page MenuHomePhabricator
Files F13991322

D8655.id20534.diff
No OneTemporary
Actions

D8655.id20534.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,34 @@
"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.use-public-uri', 'enum', 'off')
+ ->setSummary(pht('Configure usage of Amazon S3 public URI.'))
+ ->setDescription(
+ pht(
+ "Change this to enable redirecting users directly to S3 for files ".
+ "stored on S3. This allows you to offload requests directly to ".
+ "Amazon and reduce load on the Phabricator server, at the cost ".
+ "of exposing all files stored in S3 as publically accessible ".
+ "resources.\n\n".
+ " * **Off**: The default behaviour, all files are routed through ".
+ "Phabricator.\n".
+ " * **Redirect**: Phabricator will redirect to S3 when files are ".
+ "requested. Users will still hit Phabricator for the initial ".
+ "request so that Phabricator can perform 'does this file exist' ".
+ "checks, but the download will occur from Amazon.\n".
+ " * **Direct**: Phabricator will expose download URLs as direct ".
+ "links to Amazon. Users will never hit Phabricator to download ".
+ "files; all file downloads will go through Amazon.\n\n".
+ "NOTE: This currently only affects files which reside on the ".
+ "Amazon S3 storage engine.\n\n".
+ "NOTE: Enabling this option will cause HTTP clients ".
+ "to redirect from Phabricator when downloading files. You should ".
+ "**not enable** this functionality unless you are comfortable ".
+ "with being unable to return files to a non-public policy."))
+ ->setEnumOptions(array(
+ 'off' => pht('Off'),
+ 'redirect' => pht('Redirect'),
+ 'direct' => pht('Direct'))),
$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,14 @@
return new Aphront403Response();
}
+ if (PhabricatorEnv::getEnvConfig("storage.use-public-uri") !== "off") {
+ // 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 = 'https://'.$this->getBucketName().'.s3.amazonaws.com/';
+ if ($base_uri === null) {
+ return null;
+ }
+ return $base_uri.$handle;
+ }
+
+
+ /**
* Delete a blob from Amazon S3.
*/
public function deleteFile($handle) {
@@ -96,6 +119,7 @@
return $bucket;
}
+
/**
* Create a new S3 API object.
*
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,27 @@
"You must save a file before you can generate a view URI.");
}
+ if (PhabricatorEnv::getEnvConfig("storage.use-public-uri") === "direct") {
+ // Redirect to the public URI if it's present. This means Conduit
+ // methods such as phragment.getstate will return URLs directly
+ // pointing to Amazon for downloading files.
+ $public_uri = $this->getDirectPublicURI();
+ if ($public_uri !== null) {
+ return $public_uri;
+ }
+ }
+
$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
Wed, Oct 23, 10:07 AM (2 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6743986
Default Alt Text
D8655.id20534.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility