Page MenuHomePhabricator
Files F15516198

D12151.diff
No OneTemporary
Actions

D12151.diff
View Options

diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php
--- a/src/applications/auth/provider/PhabricatorAuthProvider.php
+++ b/src/applications/auth/provider/PhabricatorAuthProvider.php
@@ -247,13 +247,19 @@
$image_uri,
array(
'name' => $name,
- 'canCDN' => true,
+ 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
));
+ if ($image_file->isViewableImage()) {
+ $image_file
+ ->setViewPolicy(PhabricatorPolicies::getMostOpenPolicy())
+ ->setCanCDN(true)
+ ->save();
+ $account->setProfileImagePHID($image_file->getPHID());
+ } else {
+ $image_file->delete();
+ }
unset($unguarded);
- if ($image_file) {
- $account->setProfileImagePHID($image_file->getPHID());
- }
} catch (Exception $ex) {
// Log this but proceed, it's not especially important that we
// be able to pull profile images.
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
@@ -451,24 +451,93 @@
}
+ /**
+ * Download a remote resource over HTTP and save the response body as a file.
+ *
+ * This method respects `security.outbound-blacklist`, and protects against
+ * HTTP redirection (by manually following "Location" headers and verifying
+ * each destination). It does not protect against DNS rebinding. See
+ * discussion in T6755.
+ */
public static function newFromFileDownload($uri, array $params = array()) {
- PhabricatorEnv::requireValidRemoteURIForFetch(
- $uri,
- array(
- 'http',
- 'https',
- ));
-
$timeout = 5;
- list($file_data) = id(new HTTPSFuture($uri))
- ->setTimeout($timeout)
- ->resolvex();
- $params = $params + array(
- 'name' => basename($uri),
- );
+ $redirects = array();
+ $current = $uri;
+ while (true) {
+ try {
+ if (count($redirects) > 10) {
+ throw new Exception(
+ pht('Too many redirects trying to fetch remote URI.'));
+ }
- return self::newFromFileData($file_data, $params);
+ PhabricatorEnv::requireValidRemoteURIForFetch(
+ $current,
+ array(
+ 'http',
+ 'https',
+ ));
+
+ list($status, $body, $headers) = id(new HTTPSFuture($current))
+ ->setFollowLocation(false)
+ ->setTimeout($timeout)
+ ->resolve();
+
+ if ($status->isRedirect()) {
+ // This is an HTTP 3XX status, so look for a "Location" header.
+ $location = null;
+ foreach ($headers as $header) {
+ list($name, $value) = $header;
+ if (phutil_utf8_strtolower($name) == 'location') {
+ $location = $value;
+ break;
+ }
+ }
+
+ // HTTP 3XX status with no "Location" header, just treat this like
+ // a normal HTTP error.
+ if ($location === null) {
+ throw $status;
+ }
+
+ if (isset($redirects[$location])) {
+ throw new Exception(
+ pht(
+ 'Encountered loop while following redirects.'));
+ }
+
+ $redirects[$location] = $location;
+ $current = $location;
+ // We'll fall off the bottom and go try this URI now.
+ } else if ($status->isError()) {
+ // This is something other than an HTTP 2XX or HTTP 3XX status, so
+ // just bail out.
+ throw $status;
+ } else {
+ // This is HTTP 2XX, so use the the response body to save the
+ // file data.
+ $params = $params + array(
+ 'name' => basename($uri),
+ );
+
+ return self::newFromFileData($body, $params);
+ }
+ } catch (Exception $ex) {
+ if ($redirects) {
+ throw new PhutilProxyException(
+ pht(
+ 'Failed to fetch remote URI "%s" after following %s redirect(s) '.
+ '(%s): %s',
+ $uri,
+ new PhutilNumber(count($redirects)),
+ implode(' > ', array_keys($redirects)),
+ $ex->getMessage()),
+ $ex);
+ } else {
+ throw $ex;
+ }
+ }
+ }
}
public static function normalizeFileName($file_name) {

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 19, 5:10 PM (2 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7225886
Default Alt Text
D12151.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility