Convert Remarkup to safe HTML

Test Plan: None.

Reviewers: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4919

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() {

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,

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',

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;

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';
     }

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();
 

src/applications/phriction/storage/PhrictionContent.php

@@ -75,20 +75,19 @@ final class PhrictionContent extends PhrictionDAO
       $engine);
 
     if ($toc) {
-      $toc =
+      $toc = hsprintf(
         '<div class="phabricator-remarkup-toc">'.
-          '<div class="phabricator-remarkup-toc-header">'.
+          '<div class="phabricator-remarkup-toc-header">%s</div>'.
-            pht('Table of Contents').
+          '%s'.
-          '</div>'.
+        '</div>',
-          $toc.
+        pht('Table of Contents'),
-        '</div>';
+        $toc);
     }
 
-    return
+    return hsprintf(
-      '<div class="phabricator-remarkup">'.
+      '<div class="phabricator-remarkup">%s%s</div>',
-        $toc.
+      $toc,
-        $output.
+      $output);
-      '</div>';
   }
 
 

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 = '';
     }

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(
+      $markup = $engine->markupText($comment->getCommentText());
-        $engine->markupText($comment->getCommentText()));
 
       require_celerity_resource('phabricator-remarkup-css');
 

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 (

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',
+          javelin_tag('span', array('sigil' => $prefix.'days'), ''), 'd',
-            array('sigil' => 'phabricator-timer-days'), '').'d',
+          javelin_tag('span', array('sigil' => $prefix.'hours'), ''), 'h',
-          javelin_tag('span',
+          javelin_tag('span', array('sigil' => $prefix.'minutes'), ''), 'm',
-            array('sigil' => 'phabricator-timer-hours'), '').'h',
+          javelin_tag('span', array('sigil' => $prefix.'seconds'), ''), 's',
-          javelin_tag('span',
-            array('sigil' => 'phabricator-timer-minutes'), '').'m',
-          javelin_tag('span',
-            array('sigil' => 'phabricator-timer-seconds'), '').'s',
         ));
       Javelin::initBehavior('countdown-timer', array(
         'timestamp' => $time,

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);

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)) {
+    $name = (string)$matches[1];
-      $phid = $this->images[$matches[1]];
+
+    if (array_key_exists($name, $this->images)) {
+      $phid = $this->images[$name];
 
       $file = id(new PhabricatorFile())->loadOneWhere('phid = %s', $phid);
       $style = null;

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);

src/infrastructure/markup/rule/PhabricatorRemarkupRuleMention.php

@@ -21,13 +21,13 @@ final class PhabricatorRemarkupRuleMention
   const REGEX = '/(?<!\w)@([a-zA-Z0-9._-]*[a-zA-Z0-9_-])/';
 
   public function apply($text) {
-    return preg_replace_callback(
+    return $this->replaceHTML(
       self::REGEX,
       array($this, 'markupMention'),
       $text);
   }
 
-  private function markupMention($matches) {
+  protected function markupMention($matches) {
     $engine = $this->getEngine();
     $token = $engine->storeText('');
 

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]);

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);

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',

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;
-      $youtube_src = 'https://www.youtube.com/embed/'.$v;
+    $iframe = hsprintf(
-      $iframe =
+      '<div class="embedded-youtube-video">%s</div>',
-        '<div class="embedded-youtube-video">'.
+      phutil_tag(
-          phutil_tag(
+        'iframe',
-            'iframe',
+        array(
-            array(
+          'width'       => '650',
-              'width'       => '650',
+          'height'      => '400',
-              'height'      => '400',
+          'style'       => 'margin: 1em auto; border: 0px;',
-              'style'       => 'margin: 1em auto; border: 0px;',
+          'src'         => $youtube_src,
-              'src'         => $youtube_src,
+          'frameborder' => 0,
-              'frameborder' => 0,
+        ),
-            ),
+        ''));
-            '').
+    return $this->getEngine()->storeText($iframe);
-        '</div>';
-      return $this->getEngine()->storeText($iframe);
-    } else {
-      return $this->uri;
-    }
   }
 
 }