diff --git a/conf/default.conf.php b/conf/default.conf.php index 612a853411..0cb3f9db72 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -856,17 +856,6 @@ return array( 'image/vnd.microsoft.icon' => true, ), - // Phabricator can proxy images from other servers so you can paste the URI - // to a funny picture of a cat into the comment box and have it show up as an - // image. However, this means the webserver Phabricator is running on will - // make HTTP requests to arbitrary URIs. If the server has access to internal - // resources, this could be a security risk. You should only enable it if you - // are installed entirely a VPN and VPN access is required to access - // Phabricator, or if the webserver has no special access to anything. If - // unsure, it is safer to leave this disabled. - 'files.enable-proxy' => false, - - // -- Storage --------------------------------------------------------------- // // Phabricator allows users to upload files, and can keep them in various diff --git a/resources/sql/patches/dropfileproxyimage.sql b/resources/sql/patches/dropfileproxyimage.sql new file mode 100644 index 0000000000..982c0ebf8d --- /dev/null +++ b/resources/sql/patches/dropfileproxyimage.sql @@ -0,0 +1 @@ +DROP TABLE {$NAMESPACE}_file.file_proxyimage; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 62965e0e03..606f232572 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -744,8 +744,6 @@ phutil_register_library_map(array( 'PhabricatorFileLinkListView' => 'view/layout/PhabricatorFileLinkListView.php', 'PhabricatorFileLinkView' => 'view/layout/PhabricatorFileLinkView.php', 'PhabricatorFileListController' => 'applications/files/controller/PhabricatorFileListController.php', - 'PhabricatorFileProxyController' => 'applications/files/controller/PhabricatorFileProxyController.php', - 'PhabricatorFileProxyImage' => 'applications/files/storage/PhabricatorFileProxyImage.php', 'PhabricatorFileQuery' => 'applications/files/query/PhabricatorFileQuery.php', 'PhabricatorFileShortcutController' => 'applications/files/controller/PhabricatorFileShortcutController.php', 'PhabricatorFileSideNavView' => 'applications/files/view/PhabricatorFileSideNavView.php', @@ -987,7 +985,6 @@ phutil_register_library_map(array( 'PhabricatorRemarkupRuleObjectName' => 'infrastructure/markup/rule/PhabricatorRemarkupRuleObjectName.php', 'PhabricatorRemarkupRulePaste' => 'infrastructure/markup/rule/PhabricatorRemarkupRulePaste.php', 'PhabricatorRemarkupRulePhriction' => 'infrastructure/markup/rule/PhabricatorRemarkupRulePhriction.php', - 'PhabricatorRemarkupRuleProxyImage' => 'infrastructure/markup/rule/PhabricatorRemarkupRuleProxyImage.php', 'PhabricatorRemarkupRuleYoutube' => 'infrastructure/markup/rule/PhabricatorRemarkupRuleYoutube.php', 'PhabricatorRepository' => 'applications/repository/storage/PhabricatorRepository.php', 'PhabricatorRepositoryArcanistProject' => 'applications/repository/storage/PhabricatorRepositoryArcanistProject.php', @@ -1955,8 +1952,6 @@ phutil_register_library_map(array( 'PhabricatorFileLinkListView' => 'AphrontView', 'PhabricatorFileLinkView' => 'AphrontView', 'PhabricatorFileListController' => 'PhabricatorFileController', - 'PhabricatorFileProxyController' => 'PhabricatorFileController', - 'PhabricatorFileProxyImage' => 'PhabricatorFileDAO', 'PhabricatorFileQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorFileShortcutController' => 'PhabricatorFileController', 'PhabricatorFileSideNavView' => 'AphrontView', @@ -2172,7 +2167,6 @@ phutil_register_library_map(array( 'PhabricatorRemarkupRuleObjectName' => 'PhutilRemarkupRule', 'PhabricatorRemarkupRulePaste' => 'PhabricatorRemarkupRuleObjectName', 'PhabricatorRemarkupRulePhriction' => 'PhutilRemarkupRule', - 'PhabricatorRemarkupRuleProxyImage' => 'PhutilRemarkupRule', 'PhabricatorRemarkupRuleYoutube' => 'PhutilRemarkupRule', 'PhabricatorRepository' => 'PhabricatorRepositoryDAO', 'PhabricatorRepositoryArcanistProject' => 'PhabricatorRepositoryDAO', diff --git a/src/applications/files/controller/PhabricatorFileProxyController.php b/src/applications/files/controller/PhabricatorFileProxyController.php deleted file mode 100644 index 51b1e98654..0000000000 --- a/src/applications/files/controller/PhabricatorFileProxyController.php +++ /dev/null @@ -1,54 +0,0 @@ -getRequest(); - $uri = $request->getStr('uri'); - - $proxy = id(new PhabricatorFileProxyImage())->loadOneWhere( - 'uri = %s', - $uri); - - if (!$proxy) { - // This write is fine to skip CSRF checks for, we're just building a - // cache of some remote image. - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - - $file = PhabricatorFile::newFromFileDownload( - $uri, - nonempty(basename($uri), 'proxied-file')); - if ($file) { - $proxy = new PhabricatorFileProxyImage(); - $proxy->setURI($uri); - $proxy->setFilePHID($file->getPHID()); - $proxy->save(); - } - - unset($unguarded); - } - - if ($proxy) { - $file = id(new PhabricatorFile())->loadOneWhere('phid = %s', - $proxy->getFilePHID()); - if ($file) { - $view_uri = $file->getBestURI(); - } else { - $bad_phid = $proxy->getFilePHID(); - throw new Exception( - "Unable to load file with phid {$bad_phid}." - ); - } - return id(new AphrontRedirectResponse())->setURI($view_uri); - } - - return new Aphront400Response(); - } -} diff --git a/src/applications/files/storage/PhabricatorFileProxyImage.php b/src/applications/files/storage/PhabricatorFileProxyImage.php deleted file mode 100644 index 180bde5fd1..0000000000 --- a/src/applications/files/storage/PhabricatorFileProxyImage.php +++ /dev/null @@ -1,18 +0,0 @@ - false, - ) + parent::getConfiguration(); - } - - static public function getProxyImageURI($uri) { - return '/file/proxy/?uri='.phutil_escape_uri($uri); - } -} - diff --git a/src/docs/userguide/remarkup.diviner b/src/docs/userguide/remarkup.diviner index 7840a67814..1b680e46af 100644 --- a/src/docs/userguide/remarkup.diviner +++ b/src/docs/userguide/remarkup.diviner @@ -307,16 +307,14 @@ Valid options are: = Embedding Media = -If you set configuration flags, you can embed media directly in text: +If you set a configuration flag, you can embed media directly in text: - - **files.enable-proxy**: allows you to paste in image URLs and have them - render inline. - **remarkup.enable-embedded-youtube**: allows you to paste in YouTube videos and have them render inline. -These options are disabled by default because they have security and/or -silliness implications, read their descriptions in ##default.conf.php## before -enabling them. +This option is disabled by default because it has security and/or +silliness implications. Read the description in ##default.conf.php## before +enabling it. = Image Macros = diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php index f8bf913409..3e0273e1aa 100644 --- a/src/infrastructure/markup/PhabricatorMarkupEngine.php +++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php @@ -41,7 +41,7 @@ final class PhabricatorMarkupEngine { private $objects = array(); private $viewer; - private $version = 0; + private $version = 1; /* -( Markup Pipeline )---------------------------------------------------- */ @@ -286,7 +286,6 @@ final class PhabricatorMarkupEngine { return self::newMarkupEngine( array( 'macros' => false, - 'fileproxy' => false, 'youtube' => false, )); @@ -345,7 +344,6 @@ final class PhabricatorMarkupEngine { private static function getMarkupEngineDefaultConfiguration() { return array( 'pygments' => PhabricatorEnv::getEnvConfig('pygments.enabled'), - 'fileproxy' => PhabricatorEnv::getEnvConfig('files.enable-proxy'), 'youtube' => PhabricatorEnv::getEnvConfig( 'remarkup.enable-embedded-youtube'), 'custom-inline' => array(), @@ -394,10 +392,6 @@ final class PhabricatorMarkupEngine { $rules[] = new PhutilRemarkupRuleDocumentLink(); - if ($options['fileproxy']) { - $rules[] = new PhabricatorRemarkupRuleProxyImage(); - } - if ($options['youtube']) { $rules[] = new PhabricatorRemarkupRuleYoutube(); } diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleProxyImage.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleProxyImage.php deleted file mode 100644 index 0083a3bae7..0000000000 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleProxyImage.php +++ /dev/null @@ -1,45 +0,0 @@ -]@', - array($this, 'markupProxyImage'), - $text); - - $text = preg_replace_callback( - '@(?<=^|\s)(\w{3,}://\S+'.$filetypes.')(?=\s|$)@', - array($this, 'markupProxyImage'), - $text); - - return $text; - } - - public function markupProxyImage($matches) { - - $uri = PhabricatorFileProxyImage::getProxyImageURI($matches[1]); - - return $this->getEngine()->storeText( - phutil_render_tag( - 'a', - array( - 'href' => $uri, - 'target' => '_blank', - ), - phutil_render_tag( - 'img', - array( - 'src' => $uri, - 'class' => 'remarkup-proxy-image', - )))); - } - -} diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 9a048650c1..90308fee61 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1020,6 +1020,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'php', 'name' => $this->getPatchPath('liskcounters.php'), ), + 'dropfileproxyimage.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('dropfileproxyimage.sql'), + ), ); }