Convert AphrontErrorView to safe HTML

Summary: Done by searching for `AphrontErrorView` and then `appendChild()`.

Test Plan:
Looked at Commit Detail.
Looked at Revision Detail.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

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

Author: vrana

src/aphront/configuration/AphrontDefaultApplicationConfiguration.php

@@ -211,7 +211,7 @@ public function handleException(Exception $ex) {
     if ($ex instanceof AphrontUsageException) {
       $error = new AphrontErrorView();
       $error->setTitle(phutil_escape_html($ex->getTitle()));
-      $error->appendChild(phutil_escape_html($ex->getMessage()));
+      $error->appendChild($ex->getMessage());
 
       $view = new PhabricatorStandardPageView();
       $view->setRequest($this->getRequest());

src/aphront/response/AphrontRedirectResponse.php

@@ -49,10 +49,11 @@ public function buildResponseString() {
         ),
         'Continue to: '.$this->getURI());
 
-      $error->appendChild(
+      $error->appendChild(hsprintf(
         '<p>You were stopped here because <tt>debug.stop-on-redirect</tt> '.
         'is set in your configuration.</p>'.
-        '<p>'.$link.'</p>');
+        '<p>%s</p>',
+        $link));
 
       $view->appendChild($error);
 

src/applications/auth/controller/PhabricatorMustVerifyEmailController.php

@@ -31,9 +31,12 @@ public function processRequest() {
       $sent = new AphrontErrorView();
       $sent->setSeverity(AphrontErrorView::SEVERITY_NOTICE);
       $sent->setTitle(pht('Email Sent'));
-      $sent->appendChild('<p>'.
-        pht('Another verification email was sent to <strong>%s</strong>.',
-        phutil_escape_html($email_address)).'</p>');
+      $sent->appendChild(phutil_tag(
+        'p',
+        array(),
+        pht(
+          'Another verification email was sent to %s.',
+          phutil_tag('strong', array(), $email_address))));
     }
 
     $error_view = new AphrontRequestFailureView();

src/applications/conduit/controller/PhabricatorConduitConsoleController.php

@@ -35,20 +35,16 @@ public function processRequest() {
         case ConduitAPIMethod::METHOD_STATUS_DEPRECATED:
           $status_view->setTitle('Deprecated Method');
           $status_view->appendChild(
-            phutil_escape_html(
-              nonempty(
-                $reason,
-                "This method is deprecated.")));
+            nonempty($reason, "This method is deprecated."));
           break;
         case ConduitAPIMethod::METHOD_STATUS_UNSTABLE:
           $status_view->setSeverity(AphrontErrorView::SEVERITY_WARNING);
           $status_view->setTitle('Unstable Method');
           $status_view->appendChild(
-            phutil_escape_html(
-              nonempty(
-                $reason,
-                "This method is new and unstable. Its interface is subject ".
-                "to change.")));
+            nonempty(
+              $reason,
+              "This method is new and unstable. Its interface is subject ".
+              "to change."));
           break;
       }
     }

src/applications/config/controller/PhabricatorConfigEditController.php

@@ -108,7 +108,7 @@ public function processRequest() {
       $error_view = id(new AphrontErrorView())
         ->setTitle(pht('Configuration Hidden'))
         ->setSeverity(AphrontErrorView::SEVERITY_WARNING)
-        ->appendChild('<p>'.phutil_escape_html($msg).'</p>');
+        ->appendChild(phutil_tag('p', array(), $msg));
     } else if ($option->getLocked()) {
       $msg = pht(
         "This configuration is locked and can not be edited from the web ".
@@ -117,7 +117,7 @@ public function processRequest() {
       $error_view = id(new AphrontErrorView())
         ->setTitle(pht('Configuration Locked'))
         ->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
-        ->appendChild('<p>'.phutil_escape_html($msg).'</p>');
+        ->appendChild(phutil_tag('p', array(), $msg));
     }
 
     if ($option->getHidden()) {

src/applications/daemon/controller/PhabricatorWorkerTaskDetailController.php

@@ -23,8 +23,10 @@ public function processRequest() {
 
       $error_view = new AphrontErrorView();
       $error_view->setTitle('No Such Task');
-      $error_view->appendChild(
-        '<p>This task may have recently been garbage collected.</p>');
+      $error_view->appendChild(phutil_tag(
+        'p',
+        array(),
+        'This task may have recently been garbage collected.'));
       $error_view->setSeverity(AphrontErrorView::SEVERITY_NODATA);
 
       $content = $error_view;

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -180,21 +180,20 @@ public function processRequest() {
       $warning = new AphrontErrorView();
       $warning->setTitle('Very Large Diff');
       $warning->setSeverity(AphrontErrorView::SEVERITY_WARNING);
-      $warning->appendChild(
+      $warning->appendChild(hsprintf(
+        '%s <strong>%s</strong>',
         pht(
           'This diff is very large and affects %s files. Load each file '.
             'individually.',
-          new PhutilNumber($count)).
-        " <strong>".
-          phutil_tag(
-            'a',
-            array(
-              'href' => $request_uri
-                ->alter('large', 'true')
-                ->setFragment('toc'),
-            ),
-            pht('Show All Files Inline')).
-        "</strong>");
+          new PhutilNumber($count)),
+        phutil_tag(
+          'a',
+          array(
+            'href' => $request_uri
+              ->alter('large', 'true')
+              ->setFragment('toc'),
+          ),
+          pht('Show All Files Inline'))));
       $warning = $warning->render();
 
       $my_inlines = id(new DifferentialInlineComment())->loadAllWhere(

src/applications/differential/field/specification/DifferentialLintFieldSpecification.php

@@ -245,24 +245,24 @@ public function renderWarningBoxForRevisionAccept() {
 
     if ($status == DifferentialLintStatus::LINT_SKIP) {
       $content =
-        "<p>This diff was created without running lint. Make sure you are ".
-        "OK with that before you accept this diff.</p>";
+        "This diff was created without running lint. Make sure you are ".
+        "OK with that before you accept this diff.";
 
     } else if ($status == DifferentialLintStatus::LINT_POSTPONED) {
       $severity = AphrontErrorView::SEVERITY_WARNING;
       $content =
-        "<p>Postponed linters didn't finish yet. Make sure you are OK with ".
-        "that before you accept this diff.</p>";
+        "Postponed linters didn't finish yet. Make sure you are OK with ".
+        "that before you accept this diff.";
 
     } else {
       $content =
-        "<p>This diff has Lint Problems. Make sure you are OK with them ".
-        "before you accept this diff.</p>";
+        "This diff has Lint Problems. Make sure you are OK with them ".
+        "before you accept this diff.";
     }
 
     return id(new AphrontErrorView())
       ->setSeverity($severity)
-      ->appendChild($content)
+      ->appendChild(phutil_tag('p', array(), $content))
       ->setTitle(idx($titles, $status, 'Warning'));
   }
 

src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php

@@ -200,21 +200,21 @@ public function renderWarningBoxForRevisionAccept() {
         );
       if ($diff->getUnitStatus() == DifferentialUnitStatus::UNIT_POSTPONED) {
         $content =
-          "<p>This diff has postponed unit tests. The results should be ".
+          "This diff has postponed unit tests. The results should be ".
           "coming in soon. You should probably wait for them before accepting ".
-          "this diff.</p>";
+          "this diff.";
       } else if ($diff->getUnitStatus() == DifferentialUnitStatus::UNIT_SKIP) {
         $content =
-          "<p>Unit tests were skipped when this diff was created. Make sure ".
-          "you are OK with that before you accept this diff.</p>";
+          "Unit tests were skipped when this diff was created. Make sure ".
+          "you are OK with that before you accept this diff.";
       } else {
         $content =
-          "<p>This diff has Unit Test Problems. Make sure you are OK with ".
-          "them before you accept this diff.</p>";
+          "This diff has Unit Test Problems. Make sure you are OK with ".
+          "them before you accept this diff.";
       }
       $unit_warning = id(new AphrontErrorView())
         ->setSeverity(AphrontErrorView::SEVERITY_ERROR)
-        ->appendChild($content)
+        ->appendChild(phutil_tag('p', array(), $content))
         ->setTitle(idx($titles, $diff->getUnitStatus(), 'Warning'));
     }
     return $unit_warning;

src/applications/differential/view/DifferentialAddCommentView.php

@@ -46,21 +46,6 @@ public function setCCs(array $names) {
     return $this;
   }
 
-  private function generateWarningView(
-    $status,
-    array $titles,
-    $id,
-    $content) {
-
-    $warning = new AphrontErrorView();
-    $warning->setSeverity(AphrontErrorView::SEVERITY_ERROR);
-    $warning->setID($id);
-    $warning->appendChild($content);
-    $warning->setTitle(idx($titles, $status, 'Warning'));
-
-    return $warning;
-  }
-
   public function render() {
 
     require_celerity_resource('differential-revision-add-comment-css');

src/applications/diffusion/controller/DiffusionBrowseFileController.php

@@ -96,9 +96,8 @@ public function processRequest() {
       $notice->setSeverity(AphrontErrorView::SEVERITY_NOTICE);
       $notice->setTitle('File Renamed');
       $notice->appendChild(
-        "File history passes through a rename from '".
-        phutil_escape_html($drequest->getPath())."' to '".
-        phutil_escape_html($renamed)."'.");
+        "File history passes through a rename from '".$drequest->getPath().
+        "' to '".$renamed."'.");
       $content[] = $notice;
     }
 

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -62,8 +62,8 @@ public function processRequest() {
       $error_panel->appendChild(
         "This Diffusion repository is configured to track only one ".
         "subdirectory of the entire Subversion repository, and this commit ".
-        "didn't affect the tracked subdirectory ('".
-        phutil_escape_html($subpath)."'), so no information is available.");
+        "didn't affect the tracked subdirectory ('".$subpath."'), so no ".
+        "information is available.");
       $content[] = $error_panel;
       $content[] = $top_anchor;
     } else {
@@ -162,8 +162,7 @@ public function processRequest() {
     if ($bad_commit) {
       $error_panel = new AphrontErrorView();
       $error_panel->setTitle('Bad Commit');
-      $error_panel->appendChild(
-        phutil_escape_html($bad_commit['description']));
+      $error_panel->appendChild($bad_commit['description']);
 
       $content[] = $error_panel;
     } else if ($is_foreign) {
@@ -207,8 +206,10 @@ public function processRequest() {
         $warning_view = id(new AphrontErrorView())
           ->setSeverity(AphrontErrorView::SEVERITY_WARNING)
           ->setTitle('Very Large Commit')
-          ->appendChild(
-            "<p>This commit is very large. Load each file individually.</p>");
+          ->appendChild(phutil_tag(
+            'p',
+            array(),
+            "This commit is very large. Load each file individually."));
 
         $change_panel->appendChild($warning_view);
         $change_panel->addButton($show_all_button);

src/applications/diffusion/controller/DiffusionExternalController.php

@@ -60,17 +60,19 @@ public function processRequest() {
     if (empty($commits)) {
       $desc = null;
       if ($uri) {
-        $desc = phutil_escape_html($uri).', at ';
+        $desc = $uri.', at ';
       }
-      $desc .= phutil_escape_html($id);
+      $desc .= $id;
 
       $content = id(new AphrontErrorView())
         ->setTitle('Unknown External')
         ->setSeverity(AphrontErrorView::SEVERITY_WARNING)
-        ->appendChild(
-          "<p>This external ({$desc}) does not appear in any tracked ".
+        ->appendChild(phutil_tag(
+          'p',
+          array(),
+          "This external ({$desc}) does not appear in any tracked ".
           "repository. It may exist in an untracked repository that ".
-          "Diffusion does not know about.</p>");
+          "Diffusion does not know about."));
     } else if (count($commits) == 1) {
       $commit = head($commits);
       $repo = $repositories[$commit->getRepositoryID()];

src/applications/diffusion/view/DiffusionEmptyResultView.php

@@ -43,8 +43,6 @@ public function render() {
         $deleted = $this->browseQuery->getDeletedAtCommit();
         $existed = $this->browseQuery->getExistedAtCommit();
 
-        $deleted = self::linkCommit($drequest->getRepository(), $deleted);
-
         $browse = $this->linkBrowse(
           $drequest->getPath(),
           array(
@@ -54,19 +52,22 @@ public function render() {
           )
         );
 
-        $existed = "r{$callsign}{$existed}";
-
         $title = 'Path Was Deleted';
-        $body = "This path does not exist at {$commit}. It was deleted in ".
-                "{$deleted} and last {$browse} at {$existed}.";
+        $body = hsprintf(
+          "This path does not exist at %s. It was deleted in %s and last %s ".
+            "at %s.",
+          $commit,
+          self::linkCommit($drequest->getRepository(), $deleted),
+          $browse,
+          "r{$callsign}{$existed}");
         $severity = AphrontErrorView::SEVERITY_WARNING;
         break;
       case DiffusionBrowseQuery::REASON_IS_UNTRACKED_PARENT:
         $subdir = $drequest->getRepository()->getDetail('svn-subpath');
         $title = 'Directory Not Tracked';
         $body =
           "This repository is configured to track only one subdirectory ".
-          "of the entire repository ('".phutil_escape_html($subdir)."'), ".
+          "of the entire repository ('{$subdir}'), ".
           "but you aren't looking at something in that subdirectory, so no ".
           "information is available.";
         $severity = AphrontErrorView::SEVERITY_WARNING;
@@ -78,7 +79,7 @@ public function render() {
     $error_view = new AphrontErrorView();
     $error_view->setSeverity($severity);
     $error_view->setTitle($title);
-    $error_view->appendChild('<p>'.$body.'</p>');
+    $error_view->appendChild(phutil_tag('p', array(), $body));
 
     return $error_view->render();
   }

src/applications/fact/controller/PhabricatorFactHomeController.php

@@ -94,9 +94,10 @@ private function buildChartForm() {
       return id(new AphrontErrorView())
         ->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
         ->setTitle(pht('No Chartable Facts'))
-        ->appendChild(
-          '<p>'.pht(
-            'There are no facts that can be plotted yet.').'</p>');
+        ->appendChild(phutil_tag(
+          'p',
+          array(),
+          pht('There are no facts that can be plotted yet.')));
     }
 
     $form = id(new AphrontFormView())

src/applications/herald/controller/HeraldTranscriptController.php

@@ -35,8 +35,10 @@ public function processRequest() {
       $notice = id(new AphrontErrorView())
         ->setSeverity(AphrontErrorView::SEVERITY_NOTICE)
         ->setTitle('Old Transcript')
-        ->appendChild(
-          '<p>Details of this transcript have been garbage collected.</p>');
+        ->appendChild(phutil_tag(
+          'p',
+          array(),
+          'Details of this transcript have been garbage collected.'));
       $nav->appendChild($notice);
     } else {
       $filter = $this->getFilterPHIDs();

src/applications/metamta/controller/PhabricatorMetaMTASendController.php

@@ -67,12 +67,16 @@ public function processRequest() {
       $warning = new AphrontErrorView();
       $warning->setTitle('Email is Disabled');
       $warning->setSeverity(AphrontErrorView::SEVERITY_WARNING);
-      $warning->appendChild(
-        '<p>'.pht('This installation of Phabricator is currently set to use '.
-        '<tt>PhabricatorMailImplementationTestAdapter</tt> to deliver '.
-        'outbound email. This completely disables outbound email! All '.
-        'outbound email will be thrown in a deep, dark hole until you '.
-        'configure a real adapter.').'</p>');
+      $warning->appendChild(phutil_tag(
+        'p',
+        array(),
+        pht(
+          'This installation of Phabricator is currently set to use %s to '.
+            'deliver outbound email. This completely disables outbound email! '.
+            'All outbound email will be thrown in a deep, dark hole until you '.
+            'configure a real adapter.',
+          phutil_tag('tt', array(), 'PhabricatorMailImplementationTestAdapter'))
+        ));
     }
 
     $phdlink_href = PhabricatorEnv::getDoclink(

src/applications/notification/controller/PhabricatorNotificationStatusController.php

@@ -24,13 +24,14 @@ public function processRequest() {
     } catch (Exception $ex) {
       $status = new AphrontErrorView();
       $status->setTitle("Notification Server Issue");
-      $status->appendChild(
+      $status->appendChild(hsprintf(
         'Unable to determine server status. This probably means the server '.
         'is not in great shape. The specific issue encountered was:'.
         '<br />'.
         '<br />'.
-        '<strong>'.phutil_escape_html(get_class($ex)).'</strong> '.
-        nl2br(phutil_escape_html($ex->getMessage())));
+        '<strong>%s</strong> %s',
+        get_class($ex),
+        phutil_escape_html_newlines($ex->getMessage())));
     }
 
     return $this->buildStandardPageResponse(