From 718d22d607db2171e1518d3265646c25c84b5c26 Mon Sep 17 00:00:00 2001 From: vrana Date: Mon, 11 Feb 2013 23:42:37 -0800 Subject: [PATCH] Convert Remarkup to safe HTML Test Plan: None. Reviewers: epriestley CC: aran, Korvin Maniphest Tasks: T2432 Differential Revision: https://secure.phabricator.com/D4919 --- ...rentialBlameRevisionFieldSpecification.php | 2 +- .../DifferentialUnitFieldSpecification.php | 2 +- .../controller/DiffusionBrowseController.php | 2 +- .../controller/DiffusionCommitController.php | 3 +- .../query/browse/DiffusionBrowseQuery.php | 2 +- .../PhabricatorPeopleProfileController.php | 6 ++-- .../phriction/storage/PhrictionContent.php | 21 ++++++----- .../ConduitAPI_remarkup_process_Method.php | 2 +- .../PhabricatorSlowvotePollController.php | 3 +- .../markup/PhabricatorMarkupEngine.php | 6 ++-- .../rule/PhabricatorRemarkupRuleCountdown.php | 17 ++++----- .../rule/PhabricatorRemarkupRuleEmbedFile.php | 2 +- .../PhabricatorRemarkupRuleImageMacro.php | 8 +++-- .../rule/PhabricatorRemarkupRuleMeme.php | 2 +- .../rule/PhabricatorRemarkupRuleMention.php | 4 +-- .../PhabricatorRemarkupRuleObjectHandle.php | 4 +-- .../PhabricatorRemarkupRuleObjectName.php | 2 +- .../rule/PhabricatorRemarkupRulePhriction.php | 4 +-- .../rule/PhabricatorRemarkupRuleYoutube.php | 36 +++++++++---------- 19 files changed, 58 insertions(+), 70 deletions(-) diff --git a/src/applications/differential/field/specification/DifferentialBlameRevisionFieldSpecification.php b/src/applications/differential/field/specification/DifferentialBlameRevisionFieldSpecification.php index 11f7b5cf19..a246aa1a31 100644 --- a/src/applications/differential/field/specification/DifferentialBlameRevisionFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialBlameRevisionFieldSpecification.php @@ -49,7 +49,7 @@ final class DifferentialBlameRevisionFieldSpecification return null; } $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine(); - return phutil_safe_html($engine->markupText($this->value)); + return $engine->markupText($this->value); } public function shouldAppearOnConduitView() { diff --git a/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php b/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php index ae189573ff..6b91e90f43 100644 --- a/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php +++ b/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php @@ -113,7 +113,7 @@ final class DifferentialUnitFieldSpecification $userdata = idx($test, 'userdata'); if ($userdata) { $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine(); - $userdata = phutil_safe_html($engine->markupText($userdata)); + $userdata = $engine->markupText($userdata); $rows[] = array( 'style' => 'details', 'value' => $userdata, diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index d24b9a209b..fbc56ffea4 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -106,7 +106,7 @@ final class DiffusionBrowseController extends DiffusionController { private function markupText($text) { $engine = PhabricatorMarkupEngine::newDiffusionMarkupEngine(); - $text = phutil_safe_html($engine->markupText($text)); + $text = $engine->markupText($text); $text = phutil_tag( 'div', diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 3f87201454..db7f751137 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -97,8 +97,7 @@ final class DiffusionCommitController extends DiffusionController { array( 'class' => 'diffusion-commit-message phabricator-remarkup', ), - phutil_safe_html( - $engine->markupText($commit_data->getCommitMessage())))); + $engine->markupText($commit_data->getCommitMessage()))); $content[] = $top_anchor; $content[] = $headsup_view; diff --git a/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php b/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php index 859941bd04..6a439ab2c4 100644 --- a/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php +++ b/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php @@ -126,7 +126,7 @@ abstract class DiffusionBrowseQuery { } else { // Markup extensionless files as remarkup so we get links and such. $engine = PhabricatorMarkupEngine::newDiffusionMarkupEngine(); - $readme_content = phutil_safe_html($engine->markupText($readme_content)); + $readme_content = $engine->markupText($readme_content); $class = 'phabricator-remarkup'; } diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php index d16f5d948e..3bba99426a 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php @@ -172,13 +172,11 @@ final class PhabricatorPeopleProfileController $blurb = nonempty( $profile->getBlurb(), - '//'. - pht('Nothing is known about this rare specimen.') - .'//' + '//'.pht('Nothing is known about this rare specimen.').'//' ); $engine = PhabricatorMarkupEngine::newProfileMarkupEngine(); - $blurb = phutil_safe_html($engine->markupText($blurb)); + $blurb = $engine->markupText($blurb); $viewer = $this->getRequest()->getUser(); diff --git a/src/applications/phriction/storage/PhrictionContent.php b/src/applications/phriction/storage/PhrictionContent.php index 6c13c55fb7..ebc252cba8 100644 --- a/src/applications/phriction/storage/PhrictionContent.php +++ b/src/applications/phriction/storage/PhrictionContent.php @@ -75,20 +75,19 @@ final class PhrictionContent extends PhrictionDAO $engine); if ($toc) { - $toc = + $toc = hsprintf( '
'. - '
'. - pht('Table of Contents'). - '
'. - $toc. - '
'; + '
%s
'. + '%s'. + '', + pht('Table of Contents'), + $toc); } - return - '
'. - $toc. - $output. - '
'; + return hsprintf( + '
%s%s
', + $toc, + $output); } diff --git a/src/applications/remarkup/conduit/ConduitAPI_remarkup_process_Method.php b/src/applications/remarkup/conduit/ConduitAPI_remarkup_process_Method.php index 7d51f23d26..fd827987fc 100644 --- a/src/applications/remarkup/conduit/ConduitAPI_remarkup_process_Method.php +++ b/src/applications/remarkup/conduit/ConduitAPI_remarkup_process_Method.php @@ -45,7 +45,7 @@ final class ConduitAPI_remarkup_process_Method extends ConduitAPIMethod { $text = $engine->markupText($content); if ($text) { - $content = phutil_safe_html($text)->getHTMLContent(); + $content = hsprintf('%s', $text)->getHTMLContent(); } else { $content = ''; } diff --git a/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php b/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php index 5285ff6061..f2fe76132d 100644 --- a/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php +++ b/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php @@ -203,8 +203,7 @@ final class PhabricatorSlowvotePollController foreach ($comments as $comment) { $handle = $handles[$comment->getAuthorPHID()]; - $markup = phutil_safe_html( - $engine->markupText($comment->getCommentText())); + $markup = $engine->markupText($comment->getCommentText()); require_celerity_resource('phabricator-remarkup-css'); diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php index 5e9a96d31d..f242519c18 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 = 2; + private $version = 3; /* -( Markup Pipeline )---------------------------------------------------- */ @@ -160,7 +160,7 @@ final class PhabricatorMarkupEngine { "Call process() before getOutput()."); } - return new PhutilSafeHTML($this->objects[$key]['output']); + return $this->objects[$key]['output']; } @@ -424,7 +424,6 @@ final class PhabricatorMarkupEngine { $rules[] = new PhabricatorRemarkupRuleMention(); - $rules[] = new PhutilRemarkupRuleEscapeHTML(); $rules[] = new PhutilRemarkupRuleBold(); $rules[] = new PhutilRemarkupRuleItalic(); $rules[] = new PhutilRemarkupRuleDel(); @@ -450,7 +449,6 @@ final class PhabricatorMarkupEngine { foreach ($blocks as $block) { if ($block instanceof PhutilRemarkupEngineRemarkupLiteralBlockRule) { $literal_rules = array(); - $literal_rules[] = new PhutilRemarkupRuleEscapeHTML(); $literal_rules[] = new PhutilRemarkupRuleLinebreaks(); $block->setMarkupRules($literal_rules); } else if ( diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleCountdown.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleCountdown.php index 548a2b1d64..3356aed8e2 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleCountdown.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleCountdown.php @@ -8,13 +8,13 @@ final class PhabricatorRemarkupRuleCountdown extends PhutilRemarkupRule { const KEY_RULE_COUNTDOWN = 'rule.countdown'; public function apply($text) { - return preg_replace_callback( + return $this->replaceHTML( "@\B{C(\d+)}\B@", array($this, 'markupCountdown'), $text); } - private function markupCountdown($matches) { + protected function markupCountdown($matches) { $countdown = id(new PhabricatorTimer())->load($matches[1]); if (!$countdown) { return $matches[0]; @@ -46,20 +46,17 @@ final class PhabricatorRemarkupRuleCountdown extends PhutilRemarkupRule { foreach ($metadata as $id => $info) { list($time, $token) = $info; + $prefix = 'phabricator-timer-'; $count = phutil_tag( 'span', array( 'id' => $id, ), array( - javelin_tag('span', - array('sigil' => 'phabricator-timer-days'), '').'d', - javelin_tag('span', - array('sigil' => 'phabricator-timer-hours'), '').'h', - javelin_tag('span', - array('sigil' => 'phabricator-timer-minutes'), '').'m', - javelin_tag('span', - array('sigil' => 'phabricator-timer-seconds'), '').'s', + javelin_tag('span', array('sigil' => $prefix.'days'), ''), 'd', + javelin_tag('span', array('sigil' => $prefix.'hours'), ''), 'h', + javelin_tag('span', array('sigil' => $prefix.'minutes'), ''), 'm', + javelin_tag('span', array('sigil' => $prefix.'seconds'), ''), 's', )); Javelin::initBehavior('countdown-timer', array( 'timestamp' => $time, diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleEmbedFile.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleEmbedFile.php index 9ef0ac8ad9..e7c25dd1fe 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleEmbedFile.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleEmbedFile.php @@ -10,7 +10,7 @@ final class PhabricatorRemarkupRuleEmbedFile const KEY_EMBED_FILE_PHIDS = 'phabricator.embedded-file-phids'; public function apply($text) { - return preg_replace_callback( + return $this->replaceHTML( "@{F(\d+)([^}]+?)?}@", array($this, 'markupEmbedFile'), $text); diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleImageMacro.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleImageMacro.php index eb048f8c4c..006bfda0e8 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleImageMacro.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleImageMacro.php @@ -9,7 +9,7 @@ final class PhabricatorRemarkupRuleImageMacro private $images; public function apply($text) { - return preg_replace_callback( + return $this->replaceHTML( '@^([a-zA-Z0-9:_\-]+)$@m', array($this, 'markupImageMacro'), $text); @@ -25,8 +25,10 @@ final class PhabricatorRemarkupRuleImageMacro } } - if (array_key_exists($matches[1], $this->images)) { - $phid = $this->images[$matches[1]]; + $name = (string)$matches[1]; + + if (array_key_exists($name, $this->images)) { + $phid = $this->images[$name]; $file = id(new PhabricatorFile())->loadOneWhere('phid = %s', $phid); $style = null; diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleMeme.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleMeme.php index 1254d044ad..b569eb60ac 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleMeme.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleMeme.php @@ -9,7 +9,7 @@ final class PhabricatorRemarkupRuleMeme private $images; public function apply($text) { - return preg_replace_callback( + return $this->replaceHTML( '@{meme,([^}]+)}$@m', array($this, 'markupMeme'), $text); diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleMention.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleMention.php index f68d05fb64..b2a98fc9af 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleMention.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleMention.php @@ -21,13 +21,13 @@ final class PhabricatorRemarkupRuleMention const REGEX = '/(?replaceHTML( self::REGEX, array($this, 'markupMention'), $text); } - private function markupMention($matches) { + protected function markupMention($matches) { $engine = $this->getEngine(); $token = $engine->storeText(''); diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObjectHandle.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObjectHandle.php index 4925459e4e..8aa082b621 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObjectHandle.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObjectHandle.php @@ -13,13 +13,13 @@ abstract class PhabricatorRemarkupRuleObjectHandle public function apply($text) { $prefix = $this->getObjectNamePrefix(); - return preg_replace_callback( + return $this->replaceHTML( "@\B{{$prefix}(\d+)}\B@", array($this, 'markupObjectHandle'), $text); } - private function markupObjectHandle($matches) { + protected function markupObjectHandle($matches) { // TODO: These are single gets but should be okay for now, they're behind // the cache. $phid = $this->loadObjectPHID($matches[1]); diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObjectName.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObjectName.php index b81c37cc2f..859bebdc74 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObjectName.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleObjectName.php @@ -15,7 +15,7 @@ abstract class PhabricatorRemarkupRuleObjectName public function apply($text) { $prefix = $this->getObjectNamePrefix(); $id = $this->getObjectIDPattern(); - return preg_replace_callback( + return $this->replaceHTML( "@\b({$prefix})({$id})(?:#([-\w\d]+))?\b@", array($this, 'markupObjectNameLink'), $text); diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRulePhriction.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRulePhriction.php index 410d507b47..56a84d0d75 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRulePhriction.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRulePhriction.php @@ -7,7 +7,7 @@ final class PhabricatorRemarkupRulePhriction extends PhutilRemarkupRule { public function apply($text) { - return preg_replace_callback( + return $this->replaceHTML( '@\B\\[\\[([^|\\]]+)(?:\\|([^\\]]+))?\\]\\]\B@U', array($this, 'markupDocumentLink'), $text); @@ -28,7 +28,7 @@ final class PhabricatorRemarkupRulePhriction $href = (string) id(new PhutilURI($slug))->setFragment($fragment); if ($this->getEngine()->getState('toc')) { - $text = phutil_escape_html($name); + $text = $name; } else { $text = phutil_tag( 'a', diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleYoutube.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleYoutube.php index f4e11e4cc6..3b099ef3be 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleYoutube.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleYoutube.php @@ -10,7 +10,8 @@ final class PhabricatorRemarkupRuleYoutube $this->uri = new PhutilURI($text); if ($this->uri->getDomain() && - preg_match('/(^|\.)youtube\.com$/', $this->uri->getDomain())) { + preg_match('/(^|\.)youtube\.com$/', $this->uri->getDomain()) && + idx($this->uri->getQueryParams(), 'v')) { return $this->markupYoutubeLink(); } @@ -19,25 +20,20 @@ final class PhabricatorRemarkupRuleYoutube public function markupYoutubeLink() { $v = idx($this->uri->getQueryParams(), 'v'); - if ($v) { - $youtube_src = 'https://www.youtube.com/embed/'.$v; - $iframe = - '
'. - phutil_tag( - 'iframe', - array( - 'width' => '650', - 'height' => '400', - 'style' => 'margin: 1em auto; border: 0px;', - 'src' => $youtube_src, - 'frameborder' => 0, - ), - ''). - '
'; - return $this->getEngine()->storeText($iframe); - } else { - return $this->uri; - } + $youtube_src = 'https://www.youtube.com/embed/'.$v; + $iframe = hsprintf( + '
%s
', + phutil_tag( + 'iframe', + array( + 'width' => '650', + 'height' => '400', + 'style' => 'margin: 1em auto; border: 0px;', + 'src' => $youtube_src, + 'frameborder' => 0, + ), + '')); + return $this->getEngine()->storeText($iframe); } }