Convert everything to safe HTML

Summary: Sgrepped for `"=~/</"` and manually changed every HTML.

Test Plan: This doesn't work yet but it is hopefully one of the last diffs before Phabricator will be undoubtedly HTML safe.

Reviewers: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

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

src/aphront/AphrontRequest.php

@@ -9,7 +9,7 @@
 final class AphrontRequest {
 
   // NOTE: These magic request-type parameters are automatically included in
-  // certain requests (e.g., by phabricator_render_form(), JX.Request,
+  // certain requests (e.g., by phabricator_form(), JX.Request,
   // JX.Workflow, and ConduitClient) and help us figure out what sort of
   // response the client expects.
 

src/aphront/console/plugin/DarkConsoleErrorLogPlugin.php

@@ -36,7 +36,7 @@ final class DarkConsoleErrorLogPlugin extends DarkConsolePlugin {
     $data = $this->getData();
 
     $rows = array();
-    $details = '';
+    $details = array();
 
     foreach ($data as $index => $row) {
       $file = $row['file'];
@@ -50,7 +50,7 @@ final class DarkConsoleErrorLogPlugin extends DarkConsolePlugin {
         $row['str'].' at ['.basename($file).':'.$line.']');
       $rows[] = array($tag);
 
-      $details .= hsprintf(
+      $details[] = hsprintf(
         '<div class="dark-console-panel-error-details" id="row-details-%s">'.
         "%s\nStack trace:\n",
         $index,
@@ -73,16 +73,16 @@ final class DarkConsoleErrorLogPlugin extends DarkConsolePlugin {
           }
         }
 
-        $details .= phutil_tag(
+        $details[] = phutil_tag(
           'a',
           array(
             'href' => $href,
           ),
           $line);
-        $details .= "\n";
+        $details[] = "\n";
       }
 
-      $details .= '</div>';
+      $details[] = hsprintf('</div>');
     }
 
     $table = new AphrontTableView($rows);
@@ -90,11 +90,13 @@ final class DarkConsoleErrorLogPlugin extends DarkConsolePlugin {
     $table->setHeaders(array('Error'));
     $table->setNoDataString('No errors.');
 
-    return '<div>'.
+    return hsprintf(
-      '<div>'.$table->render().'</div>'.
+      '<div>'.
-      '<pre class="PhabricatorMonospaced">'.
+        '<div>%s</div>'.
-      $details.'</pre>'.
+        '<pre class="PhabricatorMonospaced">%s</pre>'.
-      '</div>';
+      '</div>',
+      $table->render(),
+      phutil_implode_html('', $details));
   }
 }
 

src/aphront/console/plugin/DarkConsoleEventPlugin.php

@@ -42,10 +42,10 @@ final class DarkConsoleEventPlugin extends DarkConsolePlugin {
 
     $out = array();
 
-    $out[] =
+    $out[] = hsprintf(
       '<div class="dark-console-panel-header">'.
         '<h1>Registered Event Listeners</h1>'.
-      '</div>';
+      '</div>');
 
     $rows = array();
     foreach ($data['listeners'] as $listener) {
@@ -66,10 +66,10 @@ final class DarkConsoleEventPlugin extends DarkConsolePlugin {
 
     $out[] = $table->render();
 
-    $out[] =
+    $out[] = hsprintf(
       '<div class="dark-console-panel-header">'.
         '<h1>Event Log</h1>'.
-      '</div>';
+      '</div>');
 
     $rows = array();
     foreach ($data['events'] as $event) {
@@ -93,6 +93,6 @@ final class DarkConsoleEventPlugin extends DarkConsolePlugin {
     $out[] = $table->render();
 
 
-    return implode("\n", $out);
+    return phutil_implode_html("\n", $out);
   }
 }

src/aphront/console/plugin/DarkConsoleRequestPlugin.php

@@ -62,6 +62,6 @@ final class DarkConsoleRequestPlugin extends DarkConsolePlugin {
       $out[] = $table->render();
     }
 
-    return implode("\n", $out);
+    return phutil_implode_html("\n", $out);
   }
 }

src/aphront/console/plugin/DarkConsoleServicesPlugin.php

@@ -149,8 +149,12 @@ final class DarkConsoleServicesPlugin extends DarkConsolePlugin {
     $log = $data['log'];
     $results = array();
 
-    $results[] =
+    $results[] = hsprintf(
       '<div class="dark-console-panel-header">'.
+        '%s'.
+        '<h1>Calls to External Services</h1>'.
+        '<div style="clear: both;"></div>'.
+      '</div>',
       phutil_tag(
         'a',
         array(
@@ -159,10 +163,7 @@ final class DarkConsoleServicesPlugin extends DarkConsolePlugin {
             ? 'disabled button'
             : 'green button',
         ),
-          'Analyze Query Plans').
+        'Analyze Query Plans'));
-        '<h1>Calls to External Services</h1>'.
-        '<div style="clear: both;"></div>'.
-      '</div>';
 
     $page_total = $data['end'] - $data['start'];
     $totals = array();
@@ -271,7 +272,7 @@ final class DarkConsoleServicesPlugin extends DarkConsolePlugin {
 
     $results[] = $table->render();
 
-    return implode("\n", $results);
+    return phutil_implode_html("\n", $results);
   }
 }
 

src/aphront/console/plugin/DarkConsoleXHProfPlugin.php

@@ -51,18 +51,22 @@ final class DarkConsoleXHProfPlugin extends DarkConsolePlugin {
           'class' => 'bright-link',
         ),
         'Installation Guide');
-      return
+      return hsprintf(
         '<div class="dark-console-no-content">'.
           'The "xhprof" PHP extension is not available. Install xhprof '.
           'to enable the XHProf console plugin. You can find instructions in '.
-          'the '.$install_guide.'.'.
+          'the %s.'.
-        '</div>';
+        '</div>',
+        $install_guide);
     }
 
     $result = array();
 
-    $header =
+    $header = hsprintf(
       '<div class="dark-console-panel-header">'.
+        '%s'.
+        '<h1>XHProf Profiler</h1>'.
+      '</div>',
       phutil_tag(
         'a',
         array(
@@ -71,28 +75,28 @@ final class DarkConsoleXHProfPlugin extends DarkConsolePlugin {
             ? 'disabled button'
             : 'green button',
         ),
-          'Profile Page').
+        'Profile Page'));
-        '<h1>XHProf Profiler</h1>'.
-      '</div>';
     $result[] = $header;
 
     if ($run) {
-      $result[] =
+      $result[] = hsprintf(
-        '<a href="/xhprof/profile/'.$run.'/" '.
+        '<a href="/xhprof/profile/%s/" '.
           'class="bright-link" '.
           'style="float: right; margin: 1em 2em 0 0;'.
             'font-weight: bold;" '.
           'target="_blank">Profile Permalink</a>'.
-        '<iframe src="/xhprof/profile/'.$run.'/?frame=true"></iframe>';
+        '<iframe src="/xhprof/profile/%s/?frame=true"></iframe>',
+        $run,
+        $run);
     } else {
-      $result[] =
+      $result[] = hsprintf(
         '<div class="dark-console-no-content">'.
           'Profiling was not enabled for this page. Use the button above '.
           'to enable it.'.
-        '</div>';
+        '</div>');
     }
 
-    return implode("\n", $result);
+    return phutil_implode_html("\n", $result);
   }
 
 

src/aphront/response/AphrontWebpageResponse.php

@@ -13,7 +13,7 @@ final class AphrontWebpageResponse extends AphrontHTMLResponse {
   }
 
   public function buildResponseString() {
-    return $this->content;
+    return hsprintf('%s', $this->content);
   }
 
 }

src/applications/auth/controller/PhabricatorEmailLoginController.php

@@ -138,8 +138,8 @@ EOBODY;
 
     $panel = new AphrontPanelView();
     $panel->setWidth(AphrontPanelView::WIDTH_FORM);
-    $panel->appendChild('
+    $panel->appendChild(phutil_tag('h1', array(), pht(
-      <h1>'.pht('Forgot Password / Email Login').'</h1>');
+      'Forgot Password / Email Login')));
     $panel->appendChild($email_auth);
     $panel->setNoBackground();
 

src/applications/auth/controller/PhabricatorLDAPLoginController.php

@@ -131,7 +131,7 @@ final class PhabricatorLDAPLoginController extends PhabricatorAuthController {
 
     $panel = new AphrontPanelView();
     $panel->setWidth(AphrontPanelView::WIDTH_FORM);
-    $panel->appendChild('<h1>'.pht('LDAP login').'</h1>');
+    $panel->appendChild(phutil_tag('h1', array(), pht('LDAP login')));
     $panel->appendChild($ldap_form);
 
     $error_view = null;

src/applications/auth/controller/PhabricatorLoginValidateController.php

@@ -53,7 +53,9 @@ final class PhabricatorLoginValidateController
         '<p>%s</p>%s<p>%s</p>',
         pht('Login failed:'),
         $list,
-        pht('<strong>Clear your cookies</strong> and try again.')));
+        pht(
+          '<strong>Clear your cookies</strong> and try again.',
+          hsprintf(''))));
       $view->appendChild(hsprintf(
         '<div class="aphront-failure-continue">'.
           '<a class="button" href="/login/">%s</a>'.

src/applications/auth/controller/PhabricatorOAuthDiagnosticsController.php

@@ -186,11 +186,11 @@ final class PhabricatorOAuthDiagnosticsController
 
     $panel_view = new AphrontPanelView();
     $panel_view->setHeader($title);
-    $panel_view->appendChild(
+    $panel_view->appendChild(hsprintf(
       '<p class="aphront-panel-instructions">These tests may be able to '.
-      'help diagnose the root cause of problems you experience with '.
+      'help diagnose the root cause of problems you experience with %s '.
-      $provider->getProviderName() .
+      'Authentication. Reload the page to run the tests again.</p>',
-      ' Authentication. Reload the page to run the tests again.</p>');
+      $provider->getProviderName()));
     $panel_view->appendChild($table_view);
 
     return $this->buildStandardPageResponse(

src/applications/base/controller/PhabricatorController.php

@@ -203,10 +203,9 @@ abstract class PhabricatorController extends AphrontController {
         $view = new PhabricatorStandardPageView();
         $view->setRequest($request);
         $view->setController($this);
-        $view->appendChild(
+        $view->appendChild(hsprintf(
-          '<div style="padding: 2em 0;">'.
+          '<div style="padding: 2em 0;">%s</div>',
-            $response->buildResponseString().
+          $response->buildResponseString()));
-          '</div>');
         $response = new AphrontWebpageResponse();
         $response->setContent($view->render());
         return $response;

src/applications/conduit/controller/PhabricatorConduitListController.php

@@ -59,11 +59,11 @@ final class PhabricatorConduitListController
 
     $utils = new AphrontPanelView();
     $utils->setHeader('Utilities');
-    $utils->appendChild(
+    $utils->appendChild(hsprintf(
       '<ul>'.
       '<li><a href="/conduit/log/">Log</a> - Conduit Method Calls</li>'.
       '<li><a href="/conduit/token/">Token</a> - Certificate Install</li>'.
-      '</ul>');
+      '</ul>'));
     $utils->setWidth(AphrontPanelView::WIDTH_FULL);
 
     $this->setShowSideNav(false);

src/applications/config/response/PhabricatorConfigResponse.php

@@ -23,20 +23,18 @@ final class PhabricatorConfigResponse extends AphrontHTMLResponse {
 
     $view = $this->view->render();
 
-    $template = <<<EOTEMPLATE
+    return hsprintf(
-<!doctype html>
+      '<!DOCTYPE html>'.
-<html>
+      '<html>'.
-  <head>
+        '<head>'.
-    <title>Phabricator Setup</title>
+          '<meta charset="UTF-8" />'.
-    {$resources}
+          '<title>Phabricator Setup</title>'.
-  </head>
+          '%s'.
-  <body class="setup-fatal">
+        '</head>'.
-    {$view}
+        '<body class="setup-fatal">%s</body>'.
-  </body>
+      '</html>',
-</html>
+      $resources,
-EOTEMPLATE;
+      $view);
-
-    return $template;
   }
 
   private function buildResources() {
@@ -49,11 +47,12 @@ EOTEMPLATE;
 
     $resources = array();
     foreach ($css as $path) {
-      $resources[] = '<style type="text/css">';
+      $resources[] = phutil_tag(
-      $resources[] = Filesystem::readFile($webroot.'/rsrc/css/'.$path);
+        'style',
-      $resources[] = '</style>';
+        array('type' => 'text/css'),
+        Filesystem::readFile($webroot.'/rsrc/css/'.$path));
     }
-    return implode("\n", $resources);
+    return phutil_implode_html("\n", $resources);
   }
 
 

src/applications/conpherence/controller/ConpherenceViewController.php

@@ -149,7 +149,7 @@ final class ConpherenceViewController extends
         ->setMarkupEngine($engine)
         ->render();
     }
-    $transactions = implode(' ', $rendered_transactions);
+    $transactions = phutil_implode_html(' ', $rendered_transactions);
 
     $form =
       id(new AphrontFormView())
@@ -292,7 +292,7 @@ final class ConpherenceViewController extends
         ->setNoDataString(pht('No files attached to conpherence.'))
         ->setHeaders(array('', pht('Name')))
         ->setColumnClasses(array('', 'wide'));
-    return new PhutilSafeHTML($header->render() . $table->render());
+    return hsprintf('%s%s', $header->render(), $table->render());
   }
 
   private function renderTaskWidgetPaneContent() {
@@ -328,7 +328,7 @@ final class ConpherenceViewController extends
         ->setColumnClasses(array('', 'wide'));
       $content[] = $table->render();
     }
-    return new PhutilSafeHTML(implode('', $content));
+    return phutil_implode_html('', $content);
   }
 
   private function renderCalendarWidgetPaneContent() {
@@ -416,7 +416,7 @@ final class ConpherenceViewController extends
       }
     }
 
-    return new PhutilSafeHTML(implode('', $content));
+    return phutil_implode_html('', $content);
   }
 
   private function getCalendarWidgetWeekTimestamps() {

src/applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php

@@ -77,35 +77,35 @@ final class PhabricatorWorkerTaskUpdateController
           $dialog->addSubmitButton('Retry Task');
         } else {
           $dialog->setTitle('Can Not Retry');
-          $dialog->appendChild(
+          $dialog->appendChild(phutil_tag('p', array(), pht(
-            '<p>Only archived, unsuccessful tasks can be retried.</p>');
+            'Only archived, unsuccessful tasks can be retried.')));
         }
         break;
       case 'cancel':
         if ($can_cancel) {
           $dialog->setTitle('Really cancel task?');
-          $dialog->appendChild(
+          $dialog->appendChild(phutil_tag('p', array(), pht(
-            '<p>The work this task represents will never be performed if you '.
+            'The work this task represents will never be performed if you '.
-            'cancel it. Are you sure you want to cancel it?</p>');
+            'cancel it. Are you sure you want to cancel it?')));
           $dialog->addSubmitButton('Cancel Task');
         } else {
           $dialog->setTitle('Can Not Cancel');
-          $dialog->appendChild(
+          $dialog->appendChild(phutil_tag('p', array(), pht(
-            '<p>Only active tasks can be cancelled.</p>');
+            'Only active tasks can be cancelled.')));
         }
         break;
       case 'release':
         if ($can_release) {
           $dialog->setTitle('Really free task lease?');
-          $dialog->appendChild(
+          $dialog->appendChild(phutil_tag('p', array(), pht(
-            '<p>If the process which owns the task lease is still doing work '.
+            'If the process which owns the task lease is still doing work '.
             'on it, the work may be performed twice. Are you sure you '.
-            'want to free the lease?</p>');
+            'want to free the lease?')));
           $dialog->addSubmitButton('Free Lease');
         } else {
           $dialog->setTitle('Can Not Free Lease');
-          $dialog->appendChild(
+          $dialog->appendChild(phutil_tag('p', array(), pht(
-            '<p>Only active, leased tasks may have their leases freed.</p>');
+            'Only active, leased tasks may have their leases freed.')));
         }
         break;
       default:

src/applications/differential/controller/DifferentialDiffViewController.php

@@ -25,16 +25,21 @@ final class DifferentialDiffViewController extends DifferentialController {
           'href' => PhabricatorEnv::getURI('/D'.$diff->getRevisionID()),
         ),
         'D'.$diff->getRevisionID());
-      $top_panel->appendChild(
+      $top_panel->appendChild(phutil_tag(
-        "<h1>".pht('This diff belongs to revision %s', $link)."</h1>");
+        'h1',
+        array(),
+        pht('This diff belongs to revision %s', $link)));
     } else {
       $action_panel = new AphrontPanelView();
       $action_panel->setHeader('Preview Diff');
       $action_panel->setWidth(AphrontPanelView::WIDTH_WIDE);
-      $action_panel->appendChild(
+      $action_panel->appendChild(hsprintf(
-        '<p class="aphront-panel-instructions">'.pht('Review the diff for '.
+        '<p class="aphront-panel-instructions">%s</p>',
-        'correctness. When you are satisfied, either <strong>create a new '.
+        pht(
-        'revision</strong> or <strong>update an existing revision</strong>.'));
+          'Review the diff for correctness. When you are satisfied, either '.
+          '<strong>create a new revision</strong> or <strong>update '.
+          'an existing revision</strong>.',
+          hsprintf(''))));
 
       // TODO: implmenent optgroup support in AphrontFormSelectControl?
       $select = array();

src/applications/differential/parser/DifferentialChangesetParser.php

@@ -1100,7 +1100,7 @@ final class DifferentialChangesetParser {
    * indicator of how well tested a change is.
    */
   public function renderModifiedCoverage() {
-    $na = '<em>-</em>';
+    $na = phutil_tag('em', array(), '-');
 
     $coverage = $this->getCoverage();
     if (!$coverage) {

src/applications/differential/render/DifferentialChangesetHTMLRenderer.php

@@ -21,27 +21,34 @@ abstract class DifferentialChangesetHTMLRenderer
         return null;
       }
     } else {
+      $none = $none;
       switch ($change) {
 
         case DifferentialChangeType::TYPE_ADD:
           switch ($file) {
             case DifferentialChangeType::FILE_TEXT:
-              $message = pht('This file was <strong>added</strong>.');
+              $message = pht('This file was <strong>added</strong>.', $none);
               break;
             case DifferentialChangeType::FILE_IMAGE:
-              $message = pht('This image was <strong>added</strong>.');
+              $message = pht('This image was <strong>added</strong>.', $none);
               break;
             case DifferentialChangeType::FILE_DIRECTORY:
-              $message = pht('This directory was <strong>added</strong>.');
+              $message = pht(
+                'This directory was <strong>added</strong>.',
+                $none);
               break;
             case DifferentialChangeType::FILE_BINARY:
-              $message = pht('This binary file was <strong>added</strong>.');
+              $message = pht(
+                'This binary file was <strong>added</strong>.',
+                $none);
               break;
             case DifferentialChangeType::FILE_SYMLINK:
-              $message = pht('This symlink was <strong>added</strong>.');
+              $message = pht('This symlink was <strong>added</strong>.', $none);
               break;
             case DifferentialChangeType::FILE_SUBMODULE:
-              $message = pht('This submodule was <strong>added</strong>.');
+              $message = pht(
+                'This submodule was <strong>added</strong>.',
+                $none);
               break;
           }
           break;
@@ -49,22 +56,30 @@ abstract class DifferentialChangesetHTMLRenderer
         case DifferentialChangeType::TYPE_DELETE:
           switch ($file) {
             case DifferentialChangeType::FILE_TEXT:
-              $message = pht('This file was <strong>deleted</strong>.');
+              $message = pht('This file was <strong>deleted</strong>.', $none);
               break;
             case DifferentialChangeType::FILE_IMAGE:
-              $message = pht('This image was <strong>deleted</strong>.');
+              $message = pht('This image was <strong>deleted</strong>.', $none);
               break;
             case DifferentialChangeType::FILE_DIRECTORY:
-              $message = pht('This directory was <strong>deleted</strong>.');
+              $message = pht(
+                'This directory was <strong>deleted</strong>.',
+                $none);
               break;
             case DifferentialChangeType::FILE_BINARY:
-              $message = pht('This binary file was <strong>deleted</strong>.');
+              $message = pht(
+                'This binary file was <strong>deleted</strong>.',
+                $none);
               break;
             case DifferentialChangeType::FILE_SYMLINK:
-              $message = pht('This symlink was <strong>deleted</strong>.');
+              $message = pht(
+                'This symlink was <strong>deleted</strong>.',
+                $none);
               break;
             case DifferentialChangeType::FILE_SUBMODULE:
-              $message = pht('This submodule was <strong>deleted</strong>.');
+              $message = pht(
+                'This submodule was <strong>deleted</strong>.',
+                $none);
               break;
           }
           break;
@@ -235,10 +250,9 @@ abstract class DifferentialChangesetHTMLRenderer
       }
     }
 
-    return
+    return hsprintf(
-      '<div class="differential-meta-notice">'.
+      '<div class="differential-meta-notice">%s</div>',
-        $message.
+      $message);
-      '</div>';
   }
 
   protected function renderPropertyChangeHeader() {
@@ -279,15 +293,20 @@ abstract class DifferentialChangesetHTMLRenderer
       }
     }
 
-    return
+    array_unshift($rows, hsprintf(
-      '<table class="differential-property-table">'.
       '<tr class="property-table-header">'.
-          '<th>'.pht('Property Changes').'</th>'.
+        '<th>%s</th>'.
-          '<td class="oval">'.pht('Old Value').'</td>'.
+        '<td class="oval">%s</td>'.
-          '<td class="nval">'.pht('New Value').'</td>'.
+        '<td class="nval">%s</td>'.
-        '</tr>'.
+      '</tr>',
-        implode('', $rows).
+      pht('Property Changes'),
-      '</table>';
+      pht('Old Value'),
+      pht('New Value')));
+
+    return phutil_tag(
+      'table',
+      array('class' => 'differential-property-table'),
+      $rows);
   }
 
   public function renderShield($message, $force = 'default') {
@@ -352,9 +371,6 @@ abstract class DifferentialChangesetHTMLRenderer
       return null;
     }
 
-    // TODO: [HTML] After TwoUpRenderer gets refactored, fix this.
-    $content = phutil_safe_html($content);
-
     return javelin_tag(
       'table',
       array(

src/applications/differential/render/DifferentialChangesetOneUpRenderer.php

@@ -20,32 +20,32 @@ final class DifferentialChangesetOneUpRenderer
       switch ($type) {
         case 'old':
         case 'new':
-          $out[] = '<tr>';
+          $out[] = hsprintf('<tr>');
           if ($type == 'old') {
             if ($p['htype']) {
               $class = 'left old';
             } else {
               $class = 'left';
             }
-            $out[] = '<th>'.$p['line'].'</th>';
+            $out[] = hsprintf('<th>%s</th>', $p['line']);
-            $out[] = '<th></th>';
+            $out[] = hsprintf('<th></th>');
-            $out[] = '<td class="'.$class.'">'.$p['render'].'</td>';
+            $out[] = hsprintf('<td class="%s">%s</td>', $class, $p['render']);
           } else if ($type == 'new') {
             if ($p['htype']) {
               $class = 'right new';
-              $out[] = '<th />';
+              $out[] = hsprintf('<th />');
             } else {
               $class = 'right';
-              $out[] = '<th>'.$p['oline'].'</th>';
+              $out[] = hsprintf('<th>%s</th>', $p['oline']);
             }
-            $out[] = '<th>'.$p['line'].'</th>';
+            $out[] = hsprintf('<th>%s</th>', $p['line']);
-            $out[] = '<td class="'.$class.'">'.$p['render'].'</td>';
+            $out[] = hsprintf('<td class="%s">%s</td>', $class, $p['render']);
           }
-          $out[] = '</tr>';
+          $out[] = hsprintf('</tr>');
           break;
         case 'inline':
-          $out[] = '<tr><th /><th />';
+          $out[] = hsprintf('<tr><th /><th />');
-          $out[] = '<td>';
+          $out[] = hsprintf('<td>');
 
           $inline = $this->buildInlineComment(
             $p['comment'],
@@ -53,16 +53,16 @@ final class DifferentialChangesetOneUpRenderer
           $inline->setBuildScaffolding(false);
           $out[] = $inline->render();
 
-          $out[] = '</td></tr>';
+          $out[] = hsprintf('</td></tr>');
           break;
         default:
-          $out[] = '<tr><th /><th /><td>'.$type.'</td></tr>';
+          $out[] = hsprintf('<tr><th /><th /><td>%s</td></tr>', $type);
           break;
       }
     }
 
     if ($out) {
-      return $this->wrapChangeInTable(implode('', $out));
+      return $this->wrapChangeInTable(phutil_implode_html('', $out));
     }
     return null;
   }

src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php

@@ -205,7 +205,7 @@ final class DifferentialChangesetTwoUpRenderer
         }
       }
 
-      $n_copy = '<td class="copy" />';
+      $n_copy = hsprintf('<td class="copy" />');
       $n_cov = null;
       $n_colspan = 2;
       $n_classes = '';
@@ -224,7 +224,7 @@ final class DifferentialChangesetTwoUpRenderer
             $cov_class = $coverage[$n_num - 1];
           }
           $cov_class = 'cov-'.$cov_class;
-          $n_cov = '<td class="cov '.$cov_class.'"></td>';
+          $n_cov = hsprintf('<td class="cov %s"></td>', $cov_class);
           $n_colspan--;
         }
 
@@ -242,7 +242,7 @@ final class DifferentialChangesetTwoUpRenderer
           $n_classes = $n_class;
 
           if ($new_lines[$ii]['type'] == '\\' || !isset($copy_lines[$n_num])) {
-            $n_copy = '<td class="copy '.$n_class.'"></td>';
+            $n_copy = hsprintf('<td class="copy %s"></td>', $n_class);
           } else {
             list($orig_file, $orig_line, $orig_type) = $copy_lines[$n_num];
             $title = ($orig_type == '-' ? 'Moved' : 'Copied').' from ';
@@ -274,13 +274,13 @@ final class DifferentialChangesetTwoUpRenderer
       }
 
       if ($o_num && $left_id) {
-        $o_id = ' id="C'.$left_id.$left_char.'L'.$o_num.'"';
+        $o_id = 'C'.$left_id.$left_char.'L'.$o_num;
       } else {
         $o_id = null;
       }
 
       if ($n_num && $right_id) {
-        $n_id = ' id="C'.$right_id.$right_char.'L'.$n_num.'"';
+        $n_id = 'C'.$right_id.$right_char.'L'.$n_num;
       } else {
         $n_id = null;
       }
@@ -288,20 +288,26 @@ final class DifferentialChangesetTwoUpRenderer
       // NOTE: The Javascript is sensitive to whitespace changes in this
       // block!
 
-      $html[] =
+      $html[] = hsprintf(
         '<tr>'.
-          '<th'.$o_id.'>'.$o_num.'</th>'.
+          '%s'.
-          '<td class="'.$o_classes.'">'.$o_text.'</td>'.
+          '<td class="%s">%s</td>'.
-          '<th'.$n_id.'>'.$n_num.'</th>'.
+          '%s'.
-          $n_copy.
+          '%s'.
           // NOTE: This is a unicode zero-width space, which we use as a hint
           // when intercepting 'copy' events to make sure sensible text ends
           // up on the clipboard. See the 'phabricator-oncopy' behavior.
-          '<td class="'.$n_classes.'" colspan="'.$n_colspan.'">'.
+          '<td class="%s" colspan="%s">'.
-            "\xE2\x80\x8B".$n_text.
+            "\xE2\x80\x8B%s".
           '</td>'.
-          $n_cov.
+          '%s'.
-        '</tr>';
+        '</tr>',
+        phutil_tag('th', array('id' => $o_id), $o_num),
+        $o_classes, $o_text,
+        phutil_tag('th', array('id' => $n_id), $n_num),
+        $n_copy,
+        $n_classes, $n_colspan, $n_text,
+        $n_cov);
 
       if ($context_not_available && ($ii == $rows - 1)) {
         $html[] = $context_not_available;
@@ -351,7 +357,7 @@ final class DifferentialChangesetTwoUpRenderer
       }
     }
 
-    return $this->wrapChangeInTable(implode('', $html));
+    return $this->wrapChangeInTable(phutil_implode_html('', $html));
   }
 
   public function renderFileChange($old_file = null,
@@ -395,51 +401,57 @@ final class DifferentialChangesetTwoUpRenderer
     foreach ($this->getOldComments() as $on_line => $comment_group) {
       foreach ($comment_group as $comment) {
         $comment_html = $this->renderInlineComment($comment, $on_right = false);
-        $html_old[] =
+        $html_old[] = hsprintf(
           '<tr class="inline">'.
           '<th />'.
-          '<td class="left">'.$comment_html.'</td>'.
+          '<td class="left">%s</td>'.
           '<th />'.
           '<td class="right3" colspan="3" />'.
-          '</tr>';
+          '</tr>',
+          $comment_html);
       }
     }
     foreach ($this->getNewComments() as $lin_line => $comment_group) {
       foreach ($comment_group as $comment) {
         $comment_html = $this->renderInlineComment($comment, $on_right = true);
-        $html_new[] =
+        $html_new[] = hsprintf(
           '<tr class="inline">'.
           '<th />'.
           '<td class="left" />'.
           '<th />'.
-          '<td class="right3" colspan="3">'.$comment_html.'</td>'.
+          '<td class="right3" colspan="3">%s</td>'.
-          '</tr>';
+          '</tr>',
+          $comment_html);
       }
     }
 
     if (!$old) {
-      $th_old = '<th></th>';
+      $th_old = hsprintf('<th></th>');
     } else {
-      $th_old = '<th id="C'.$vs.'OL1">1</th>';
+      $th_old = hsprintf('<th id="C%sOL1">1</th>', $vs);
     }
 
     if (!$new) {
-      $th_new = '<th></th>';
+      $th_new = hsprintf('<th></th>');
     } else {
-      $th_new = '<th id="C'.$id.'NL1">1</th>';
+      $th_new = hsprintf('<th id="C%sNL1">1</th>', $id);
     }
 
-    $output =
+    $output = hsprintf(
       '<tr class="differential-image-diff">'.
-      $th_old.
+        '%s'.
-      '<td class="left differential-old-image">'.$old.'</td>'.
+        '<td class="left differential-old-image">%s</td>'.
-      $th_new.
+        '%s'.
-      '<td class="right3 differential-new-image" colspan="3">'.
+        '<td class="right3 differential-new-image" colspan="3">%s</td>'.
-      $new.
-      '</td>'.
       '</tr>'.
-      implode('', $html_old).
+      '%s'.
-      implode('', $html_new);
+      '%s',
+      $th_old,
+      $old,
+      $th_new,
+      $new,
+      phutil_implode_html('', $html_old),
+      phutil_implode_html('', $html_new));
 
     $output = $this->wrapChangeInTable($output);
 

src/applications/differential/view/DifferentialChangesetListView.php

@@ -221,15 +221,20 @@ final class DifferentialChangesetListView extends AphrontView {
       ),
       array('Changes discarded. ', $link));
 
-    $template =
+    return array(
+      'l' => hsprintf(
         '<table><tr>'.
           '<th></th><td>%s</td>'.
-      '<th></th><td colspan="3">%s</td>'.
+          '<th></th><td colspan="3"></td>'.
-      '</tr></table>';
+        '</tr></table>',
+        $div),
 
-    return array(
+      'r' => hsprintf(
-      'l' => sprintf($template, $div, ''),
+        '<table><tr>'.
-      'r' => sprintf($template, '', $div),
+          '<th></th><td></td>'.
+          '<th></th><td colspan="3">%s</td>'.
+        '</tr></table>',
+        $div),
     );
   }
 

src/applications/feed/story/PhabricatorFeedStory.php

@@ -224,7 +224,7 @@ abstract class PhabricatorFeedStory implements PhabricatorPolicyInterface {
     foreach ($phids as $phid) {
       $list[] = $this->linkTo($phid);
     }
-    return implode(', ', $list);
+    return phutil_implode_html(', ', $list);
   }
 
   final protected function linkTo($phid) {

src/applications/feed/story/PhabricatorFeedStoryAudit.php

@@ -15,11 +15,11 @@ final class PhabricatorFeedStoryAudit extends PhabricatorFeedStory {
     $action = $this->getValue('action');
     $verb = PhabricatorAuditActionConstants::getActionPastTenseVerb($action);
 
-    $view->setTitle(
+    $view->setTitle(hsprintf(
-      $this->linkTo($author_phid).
+      '%s %s commit %s.',
-      " {$verb} commit ".
+      $this->linkTo($author_phid),
-      $this->linkTo($commit_phid).
+      $verb,
-      ".");
+      $this->linkTo($commit_phid)));
 
     $view->setEpoch($this->getEpoch());
 

src/applications/feed/story/PhabricatorFeedStoryDifferential.php

@@ -51,7 +51,11 @@ final class PhabricatorFeedStoryDifferential extends PhabricatorFeedStory {
 
     $verb = DifferentialAction::getActionPastTenseVerb($action);
 
-    $one_line = "{$actor_link} {$verb} revision {$revision_link}";
+    $one_line = hsprintf(
+      '%s %s revision %s',
+      $actor_link,
+      $verb,
+      $revision_link);
 
     return $one_line;
   }

src/applications/feed/story/PhabricatorFeedStoryManiphest.php

@@ -66,16 +66,23 @@ final class PhabricatorFeedStoryManiphest
       case ManiphestAction::ACTION_REASSIGN:
         if ($owner_phid) {
           if ($owner_phid == $actor_phid) {
-            $one_line = "{$actor_link} claimed {$task_link}";
+            $one_line = hsprintf('%s claimed %s', $actor_link, $task_link);
           } else {
-            $one_line = "{$actor_link} {$verb} {$task_link} to {$owner_link}";
+            $one_line = hsprintf('%s %s %s to %s',
+              $actor_link,
+              $verb,
+              $owner_link,
+              $task_link);
           }
         } else {
-          $one_line = "{$actor_link} placed {$task_link} up for grabs";
+          $one_line = hsprintf(
+            '%s placed %s up for grabs',
+            $actor_link,
+            $task_link);
         }
         break;
       default:
-        $one_line = "{$actor_link} {$verb} {$task_link}";
+        $one_line = hsprintf('%s %s %s', $actor_link, $verb, $task_link);
         break;
     }
 

src/applications/feed/story/PhabricatorFeedStoryPhriction.php

@@ -17,10 +17,11 @@ final class PhabricatorFeedStoryPhriction extends PhabricatorFeedStory {
     $action = $data->getValue('action');
     $verb = PhrictionActionConstants::getActionPastTenseVerb($action);
 
-    $view->setTitle(
+    $view->setTitle(hsprintf(
-      $this->linkTo($author_phid).
+      '%s %s the document %s.',
-      " {$verb} the document ".
+      $this->linkTo($author_phid),
-      $this->linkTo($document_phid).'.');
+      $verb,
+      $this->linkTo($document_phid)));
     $view->setEpoch($data->getEpoch());
 
     $action = $data->getValue('action');

src/applications/feed/story/PhabricatorFeedStoryProject.php

@@ -21,31 +21,25 @@ final class PhabricatorFeedStoryProject extends PhabricatorFeedStory {
     switch ($type) {
       case PhabricatorProjectTransactionType::TYPE_NAME:
         if (strlen($old)) {
-          $action = 'renamed project '.
+          $action = hsprintf(
-            $this->linkTo($proj_phid).
+            'renamed project %s from %s to %s.',
-            ' from '.
+            $this->linkTo($proj_phid),
-            $this->renderString($old).
+            $this->renderString($old),
-            ' to '.
+            $this->renderString($new));
-            $this->renderString($new).
-            '.';
         } else {
-          $action = 'created project '.
+          $action = hsprintf(
-            $this->linkTo($proj_phid).
+            'created project %s (as %s).',
-            ' (as '.
+            $this->linkTo($proj_phid),
-            $this->renderString($new).
+            $this->renderString($new));
-            ').';
         }
         break;
       case PhabricatorProjectTransactionType::TYPE_STATUS:
-        $action = 'changed project '.
+        $action = hsprintf(
-                  $this->linkTo($proj_phid).
+          'changed project %s status from %s to %s.',
-                  ' status from '.
+          $this->linkTo($proj_phid),
-                  $this->renderString(
+          $this->renderString(PhabricatorProjectStatus::getNameForStatus($old)),
-                    PhabricatorProjectStatus::getNameForStatus($old)).
+          $this->renderString(PhabricatorProjectStatus::getNameForStatus($new))
-                  ' to '.
+        );
-                  $this->renderString(
-                    PhabricatorProjectStatus::getNameForStatus($new)).
-                  '.';
         break;
       case PhabricatorProjectTransactionType::TYPE_MEMBERS:
         $add = array_diff($new, $old);
@@ -53,30 +47,33 @@ final class PhabricatorFeedStoryProject extends PhabricatorFeedStory {
 
         if ((count($add) == 1) && (count($rem) == 0) &&
             (head($add) == $author_phid)) {
-          $action = 'joined project '.$this->linkTo($proj_phid).'.';
+          $action = hsprintf('joined project %s.', $this->linkTo($proj_phid));
         } else if ((count($add) == 0) && (count($rem) == 1) &&
                    (head($rem) == $author_phid)) {
-          $action = 'left project '.$this->linkTo($proj_phid).'.';
+          $action = hsprintf('left project %s.', $this->linkTo($proj_phid));
         } else if (empty($rem)) {
-          $action = 'added members to project '.
+          $action = hsprintf(
-            $this->linkTo($proj_phid).': '.
+            'added members to project %s: %s.',
-            $this->renderHandleList($add).'.';
+            $this->linkTo($proj_phid),
+            $this->renderHandleList($add));
         } else if (empty($add)) {
-          $action = 'removed members from project '.
+          $action = hsprintf(
-            $this->linkTo($proj_phid).': '.
+            'removed members from project %s: %s.',
-            $this->renderHandleList($rem).'.';
+            $this->linkTo($proj_phid),
+            $this->renderHandleList($rem));
         } else {
-          $action = 'changed members of project '.
+          $action = hsprintf(
-            $this->linkTo($proj_phid).', added: '.
+            'changed members of project %s, added: %s; removed: %s.',
-            $this->renderHandleList($add).'; removed: '.
+            $this->linkTo($proj_phid),
-            $this->renderHandleList($rem).'.';
+            $this->renderHandleList($add),
+            $this->renderHandleList($rem));
         }
         break;
       default:
-        $action = 'updated project '.$this->linkTo($proj_phid).'.';
+        $action = hsprintf('updated project %s.', $this->linkTo($proj_phid));
         break;
     }
-    $view->setTitle($this->linkTo($author_phid).' '.$action);
+    $view->setTitle(hsprintf('%s %s', $this->linkTo($author_phid), $action));
     $view->setOneLineStory(true);
 
     return $view;

src/applications/feed/view/PhabricatorFeedStoryView.php

@@ -67,7 +67,7 @@ final class PhabricatorFeedStoryView extends PhabricatorFeedView {
           'href' => $this->getHref(),
         ),
       ),
-      phutil_safe_html($this->title));
+      $this->title);
   }
 
   public function render() {
@@ -77,7 +77,7 @@ final class PhabricatorFeedStoryView extends PhabricatorFeedView {
       array(
         'class' => 'phabricator-feed-story-head',
       ),
-      nonempty(phutil_safe_html($this->title), 'Untitled Story'));
+      nonempty($this->title, 'Untitled Story'));
 
     $body = null;
     $foot = null;
@@ -89,7 +89,7 @@ final class PhabricatorFeedStoryView extends PhabricatorFeedView {
         array(
           'class' => 'phabricator-feed-story-body',
         ),
-        phutil_safe_html(implode('', $this->renderChildren())));
+        $this->renderChildren());
 
       if ($this->epoch) {
         $foot = phabricator_datetime($this->epoch, $this->user);

src/applications/herald/controller/HeraldTranscriptController.php

@@ -437,10 +437,10 @@ final class HeraldTranscriptController extends HeraldController {
 
     $panel = new AphrontPanelView();
     $panel->setHeader('Rule Details');
-    $panel->appendChild(
+    $panel->appendChild(phutil_tag(
-      '<ul class="herald-explain-list">'.
+      'ul',
-        implode("\n", $rule_markup).
+      array('class' => 'herald-explain-list'),
-      '</ul>');
+      $rule_markup));
 
     return $panel;
   }

src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php

@@ -203,10 +203,6 @@ class ManiphestAuxiliaryFieldDefaultSpecification
         break;
     }
 
-    if ($target == self::RENDER_TARGET_HTML) {
-      $desc = phutil_escape_html($desc);
-    }
-
     return $desc;
   }
 

src/applications/maniphest/controller/ManiphestSavedQueryListController.php

@@ -111,7 +111,7 @@ final class ManiphestSavedQueryListController extends ManiphestController {
         'Save Default Query'));
     $panel->appendChild($table);
 
-    $form = phabricator_render_form(
+    $form = phabricator_form(
       $user,
       array(
         'method' => 'POST',

src/applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php

@@ -18,10 +18,9 @@ final class ManiphestTaskDescriptionPreviewController
       ManiphestTask::MARKUP_FIELD_DESCRIPTION,
       $request->getUser());
 
-    $content =
+    $content = hsprintf(
-      '<div class="phabricator-remarkup">'.
+      '<div class="phabricator-remarkup">%s</div>',
-        $output.
+      $output);
-      '</div>';
 
     return id(new AphrontAjaxResponse())
       ->setContent($content);

src/applications/maniphest/view/ManiphestTransactionDetailView.php

@@ -590,9 +590,6 @@ final class ManiphestTransactionDetailView extends ManiphestView {
           DifferentialChangesetParser::parseRangeSpecification($spec);
         $output = $parser->render($range_s, $range_e, $mask);
 
-        // TODO: [HTML] DifferentialChangesetParser needs cleanup.
-        $output = phutil_safe_html($output);
-
         return $output;
     }
 
@@ -627,7 +624,7 @@ final class ManiphestTransactionDetailView extends ManiphestView {
         $links[] = $this->handles[$phid]->renderLink();
       }
     }
-    return implode(', ', $links);
+    return phutil_implode_html(', ', $links);
   }
 
   private function renderString($string) {

src/applications/notification/controller/PhabricatorNotificationPanelController.php

@@ -20,25 +20,23 @@ final class PhabricatorNotificationPanelController
       $notifications_view = $builder->buildView();
       $content = $notifications_view->render();
     } else {
-      $content =
+      $content = hsprintf(
-        '<div class="phabricator-notification no-notifications">'.
+        '<div class="phabricator-notification no-notifications">%s</div>',
-          'You have no notifications.'.
+        pht('You have no notifications.'));
-        '</div>';
     }
 
-    $content =
+    $content = hsprintf(
-      '<div class="phabricator-notification-header">'.
+      '<div class="phabricator-notification-header">%s</div>'.
-        pht('Notifications').
+      '%s'.
-      '</div>'.
+      '<div class="phabricator-notification-view-all">%s</div>',
-      $content.
+      pht('Notifications'),
-      '<div class="phabricator-notification-view-all">'.
+      $content,
       phutil_tag(
         'a',
         array(
           'href' => '/notification/',
         ),
-          'View All Notifications').
+        'View All Notifications'));
-      '</div>';
 
     $unread_count = id(new PhabricatorFeedStoryNotification())
       ->countUnread($user);

src/applications/people/controller/PhabricatorPeopleProfileController.php

@@ -142,7 +142,7 @@ final class PhabricatorPeopleProfileController
 
     $nav->appendChild($header);
 
-    $content = '<div style="padding: 1em;">'.$content.'</div>';
+    $content = hsprintf('<div style="padding: 1em;">%s</div>', $content);
     $header->appendChild($content);
 
     if ($user->getPHID() == $viewer->getPHID()) {
@@ -230,12 +230,11 @@ final class PhabricatorPeopleProfileController
     $builder->setUser($viewer);
     $view = $builder->buildView();
 
-    return
+    return hsprintf(
       '<div class="phabricator-profile-info-group">
         <h1 class="phabricator-profile-info-header">Activity Feed</h1>
-        <div class="phabricator-profile-info-pane">
+        <div class="phabricator-profile-info-pane">%s</div>
-          '.$view->render().'
+      </div>',
-        </div>
+      $view->render());
-      </div>';
   }
 }

src/applications/phame/controller/post/PhamePostPreviewController.php

@@ -23,7 +23,7 @@ extends PhameController {
       PhamePost::MARKUP_FIELD_BODY,
       $user);
 
-    $content = '<div class="phabricator-remarkup">'.$content.'</div>';
+    $content = hsprintf('<div class="phabricator-remarkup">%s</div>', $content);
 
     return id(new AphrontAjaxResponse())->setContent($content);
   }

src/applications/phame/skins/PhameBasicBlogSkin.php

@@ -123,7 +123,7 @@ abstract class PhameBasicBlogSkin extends PhameBlogSkin {
   }
 
   protected function render404Page() {
-    return '<h2>404 Not Found</h2>';
+    return hsprintf('<h2>404 Not Found</h2>');
   }
 
   final public function getResourceURI($resource) {

src/applications/phame/skins/PhameBasicTemplateBlogSkin.php

@@ -26,7 +26,7 @@ final class PhameBasicTemplateBlogSkin extends PhameBasicBlogSkin {
             'href'  => $this->getResourceURI('css/'.$path),
           ));
       }
-      $this->cssResources = implode("\n", $this->cssResources);
+      $this->cssResources = phutil_implode_html("\n", $this->cssResources);
     }
 
     $request = $this->getRequest();
@@ -43,7 +43,7 @@ final class PhameBasicTemplateBlogSkin extends PhameBasicBlogSkin {
     );
 
     $response = new AphrontWebpageResponse();
-    $response->setContent(implode("\n", $content));
+    $response->setContent(phutil_implode_html("\n", $content));
 
     return $response;
   }

src/applications/pholio/controller/PholioMockViewController.php

@@ -145,7 +145,7 @@ final class PholioMockViewController extends PholioController {
       foreach ($subscribers as $subscriber) {
         $sub_view[] = $this->getHandle($subscriber)->renderLink();
       }
-      $sub_view = implode(', ', $sub_view);
+      $sub_view = phutil_implode_html(', ', $sub_view);
     } else {
       $sub_view = phutil_tag('em', array(), pht('None'));
     }

src/applications/phpast/controller/PhabricatorXHPASTViewFramesetController.php

@@ -14,12 +14,15 @@ final class PhabricatorXHPASTViewFramesetController
 
     $response = new AphrontWebpageResponse();
     $response->setFrameable(true);
-    $response->setContent(
+    $response->setContent(hsprintf(
-      '<frameset cols="33%, 34%, 33%">'.
+      '<frameset cols="33%%, 34%%, 33%%">'.
-        '<frame src="/xhpast/input/'.$id.'/" />'.
+        '<frame src="/xhpast/input/%s/" />'.
-        '<frame src="/xhpast/tree/'.$id.'/" />'.
+        '<frame src="/xhpast/tree/%s/" />'.
-        '<frame src="/xhpast/stream/'.$id.'/" />'.
+        '<frame src="/xhpast/stream/%s/" />'.
-      '</frameset>');
+      '</frameset>',
+      $id,
+      $id,
+      $id));
 
     return $response;
   }

src/applications/ponder/view/PonderAnswerListView.php

@@ -70,7 +70,8 @@ final class PonderAnswerListView extends AphrontView {
 
       $panel->appendChild($view);
       $panel->appendChild($commentview);
-      $panel->appendChild('<div style="height: 40px; clear : both"></div>');
+      $panel->appendChild(
+        hsprintf('<div style="height: 40px; clear : both"></div>'));
 
     }
 

src/applications/project/controller/PhabricatorProjectMembersEditController.php

@@ -112,7 +112,7 @@ final class PhabricatorProjectMembersEditController
     $panel->setHeader($header_name);
     $panel->setWidth(AphrontPanelView::WIDTH_FORM);
     $panel->appendChild($form);
-    $panel->appendChild('<br />');
+    $panel->appendChild(phutil_tag('br'));
     $panel->appendChild($faux_form);
 
     $nav = $this->buildLocalNavigation($project);

src/applications/project/controller/PhabricatorProjectProfileController.php

@@ -53,7 +53,7 @@ final class PhabricatorProjectProfileController
         $query->setViewer($this->getRequest()->getUser());
         $stories = $query->execute();
 
-        $content .= $this->renderStories($stories);
+        $content = hsprintf('%s%s', $content, $this->renderStories($stories));
         break;
       case 'about':
         $content = $this->renderAboutPage($project, $profile);
@@ -112,7 +112,7 @@ final class PhabricatorProjectProfileController
 
     $nav_view->appendChild($header);
 
-    $content = '<div style="padding: 1em;">'.$content.'</div>';
+    $content = hsprintf('<div style="padding: 1em;">%s</div>', $content);
     $header->appendChild($content);
 
     return $this->buildStandardPageResponse(
@@ -178,22 +178,22 @@ final class PhabricatorProjectProfileController
 
     $affiliated = array();
     foreach ($handles as $phids => $handle) {
-      $affiliated[] = '<li>'.$handle->renderLink().'</li>';
+      $affiliated[] = phutil_tag('li', array(), $handle->renderLink());
     }
 
     if ($affiliated) {
-      $affiliated = '<ul>'.implode("\n", $affiliated).'</ul>';
+      $affiliated = phutil_tag('ul', array(), $affiliated);
     } else {
-      $affiliated = '<p><em>No one is affiliated with this project.</em></p>';
+      $affiliated = hsprintf('<p><em>%s</em></p>', pht(
+        'No one is affiliated with this project.'));
     }
 
-    return
+    return hsprintf(
       '<div class="phabricator-profile-info-group">'.
         '<h1 class="phabricator-profile-info-header">People</h1>'.
-        '<div class="phabricator-profile-info-pane">'.
+        '<div class="phabricator-profile-info-pane">%s</div>'.
-         $affiliated.
+      '</div>',
-        '</div>'.
+      $affiliated);
-      '</div>';
   }
 
   private function renderFeedPage(
@@ -220,13 +220,12 @@ final class PhabricatorProjectProfileController
     $builder->setUser($this->getRequest()->getUser());
     $view = $builder->buildView();
 
-    return
+    return hsprintf(
       '<div class="phabricator-profile-info-group">'.
         '<h1 class="phabricator-profile-info-header">Activity Feed</h1>'.
-        '<div class="phabricator-profile-info-pane">'.
+        '<div class="phabricator-profile-info-pane">%s</div>'.
-         $view->render().
+      '</div>',
-        '</div>'.
+      $view->render());
-      '</div>';
   }
 
 
@@ -257,9 +256,9 @@ final class PhabricatorProjectProfileController
     }
 
     if (empty($tasks)) {
-      $task_views = '<em>No open tasks.</em>';
+      $task_views = phutil_tag('em', array(), pht('No open tasks.'));
     } else {
-      $task_views = implode('', $task_views);
+      $task_views = phutil_implode_html('', $task_views);
     }
 
     $open = number_format($count);
@@ -271,18 +270,17 @@ final class PhabricatorProjectProfileController
       ),
       "View All Open Tasks \xC2\xBB");
 
-    $content =
+    $content = hsprintf(
       '<div class="phabricator-profile-info-group">
-        <h1 class="phabricator-profile-info-header">'.
+        <h1 class="phabricator-profile-info-header">Open Tasks (%s)</h1>'.
-          "Open Tasks ({$open})".
-        '</h1>'.
         '<div class="phabricator-profile-info-pane">'.
-          $task_views.
+          '%s'.
-          '<div class="phabricator-profile-info-pane-more-link">'.
+          '<div class="phabricator-profile-info-pane-more-link">%s</div>'.
-            $more_link.
-          '</div>'.
         '</div>
-      </div>';
+      </div>',
+      $open,
+      $task_views,
+      $more_link);
 
     return $content;
   }

src/applications/settings/panel/PhabricatorSettingsPanelLDAP.php

@@ -75,7 +75,7 @@ final class PhabricatorSettingsPanelLDAP
 
     foreach ($forms as $name => $form) {
       if ($name) {
-        $panel->appendChild('<br /><h1>'.$name.'</h1><br />');
+        $panel->appendChild(hsprintf('<br /><h1>%s</h1><br />', $name));
       }
       $panel->appendChild($form);
     }

src/applications/settings/panel/PhabricatorSettingsPanelOAuth.php

@@ -215,7 +215,7 @@ final class PhabricatorSettingsPanelOAuth
 
     foreach ($forms as $name => $form) {
       if ($name) {
-        $panel->appendChild('<br /><h1>'.$name.'</h1><br />');
+        $panel->appendChild(hsprintf('<br /><h1>%s</h1><br />', $name));
       }
       $panel->appendChild($form);
     }

src/applications/slowvote/controller/PhabricatorSlowvotePollController.php

@@ -181,7 +181,7 @@ final class PhabricatorSlowvotePollController
     $panel->setWidth(AphrontPanelView::WIDTH_WIDE);
 
     $panel->appendChild($form);
-    $panel->appendChild('<br /><br />');
+    $panel->appendChild(hsprintf('<br /><br />'));
     $panel->appendChild($result_markup);
 
     return $this->buildStandardPageResponse(

src/applications/transactions/view/PhabricatorApplicationTransactionView.php

@@ -99,8 +99,8 @@ class PhabricatorApplicationTransactionView extends AphrontView {
         $event->appendChild(
           $engine->getOutput($xaction->getComment(), $field));
       } else if ($has_deleted_comment) {
-        $event->appendChild(
+        $event->appendChild(phutil_tag('em', array(), pht(
-          '<em>'.pht('This comment has been deleted.').'</em>');
+          'This comment has been deleted.')));
       }
 
       $events[] = $event;

src/applications/uiexample/examples/PhabricatorUIPagerExample.php

@@ -35,10 +35,10 @@ final class PhabricatorUIPagerExample extends PhabricatorUIExample {
     $panel = new AphrontPanelView();
     $panel->appendChild($table);
 
-    $panel->appendChild(
+    $panel->appendChild(hsprintf(
       '<p class="phabricator-ui-example-note">'.
         'Use <tt>AphrontPagerView</tt> to render a pager element.'.
-      '</p>');
+      '</p>'));
 
     $pager = new AphrontPagerView();
     $pager->setPageSize($page_size);
@@ -47,10 +47,10 @@ final class PhabricatorUIPagerExample extends PhabricatorUIExample {
     $pager->setURI($request->getRequestURI(), 'offset');
     $panel->appendChild($pager);
 
-    $panel->appendChild(
+    $panel->appendChild(hsprintf(
       '<p class="phabricator-ui-example-note">'.
         'You can show more or fewer pages of surrounding context.'.
-      '</p>');
+      '</p>'));
 
     $many_pages_pager = new AphrontPagerView();
     $many_pages_pager->setPageSize($page_size);
@@ -60,12 +60,12 @@ final class PhabricatorUIPagerExample extends PhabricatorUIExample {
     $many_pages_pager->setSurroundingPages(7);
     $panel->appendChild($many_pages_pager);
 
-    $panel->appendChild(
+    $panel->appendChild(hsprintf(
       '<p class="phabricator-ui-example-note">'.
         'When it is prohibitively expensive or complex to attain a complete '.
         'count of the items, you can select one extra item and set '.
         '<tt>hasMorePages(true)</tt> if it exists, creating an inexact pager.'.
-      '</p>');
+      '</p>'));
 
     $inexact_pager = new AphrontPagerView();
     $inexact_pager->setPageSize($page_size);

src/docs/developer/rendering_html.diviner

@@ -148,16 +148,36 @@ calling @{function:phutil_safe_html} on it. This is **dangerous**, because if
 you are wrong and the string is not actually safe, you have introduced an XSS
 vulnerability. Consequently, you should avoid calling this if possible.
 
-You can use @{function@libphutil:phutil_escape_html} to explicitly escape an
-HTML string. You should not normally need to use it.
-
 You can use @{function@libphutil:phutil_escape_html_newlines} to escape HTML
-while converting newlines to `<br />`.
+while converting newlines to `<br />`. You should not need to explicitly use
+@{function@libphutil:phutil_escape_html} anywhere.
+
+If you need to apply a string function (such as `trim()`) to safe HTML, use
+@{method@libphutil:PhutilSafeHTML::applyFunction}.
 
 If you need to extract the content of a @{class@libphutil:PhutilSafeHTML}
 object, you should call `getHTMLContent()`, not cast it to a string. Eventually,
 we would like to remove the string cast entirely.
 
+Functions @{function@libphutil:phutil_tag} and @{function@libphutil:hsprintf}
+are not safe if you pass the user input for the tag or attribute name. All the
+following examples are dangerous:
+
+  counterexample
+  phutil_tag($evil);
+
+  phutil_tag('span', array($evil => $evil2));
+
+  // Use PhutilURI to check if $evil is valid HTTP link.
+  phutil_tag('a', array('href' => $evil));
+
+  phutil_tag('span', array('onmouseover' => $evil));
+
+  hsprintf('<%s>%s</%s>', $evil, $evil2, $evil);
+
+  // We have a lint rule disallowing this.
+  hsprintf($evil);
+
 = Deprecated Functions =
 
 The functions @{function@libphutil:phutil_render_tag} and

src/infrastructure/celerity/CelerityStaticResourceResponse.php

@@ -98,8 +98,9 @@ final class CelerityStaticResourceResponse {
       $this->hasRendered[$resource['uri']] = true;
 
       $output[] = $this->renderResource($resource);
+      $output[] = "\n";
     }
-    return implode("\n", $output)."\n";
+    return phutil_implode_html('', $output);
   }
 
   private function renderResource(array $resource) {
@@ -179,8 +180,9 @@ final class CelerityStaticResourceResponse {
 
     if ($data) {
       $data = implode("\n", $data);
-      return '<script type="text/javascript">//<![CDATA['."\n".
+      return hsprintf(
-             $data.'//]]></script>';
+        '<script type="text/javascript">//<![CDATA['."\n".'%s//]]></script>',
+        phutil_safe_html($data));
     } else {
       return '';
     }

src/infrastructure/diff/PhabricatorInlineCommentPreviewController.php

@@ -34,7 +34,7 @@ abstract class PhabricatorInlineCommentPreviewController
       $view->setPreview(true);
       $views[] = $view->render();
     }
-    $views = implode("\n", $views);
+    $views = phutil_implode_html("\n", $views);
 
     return id(new AphrontAjaxResponse())
       ->setContent($views);

src/infrastructure/diff/view/PhabricatorInlineSummaryView.php

@@ -79,19 +79,26 @@ final class PhabricatorInlineSummaryView extends AphrontView {
 
         $where = idx($item, 'where');
 
-        $colspan = ($has_where ? '' : ' colspan="2"');
+        $colspan = ($has_where ? null : 2);
-        $rows[] =
+        $rows[] = hsprintf(
           '<tr>'.
-            '<td class="inline-line-number">'.$lines.'</td>'.
+            '<td class="inline-line-number">%s</td>'.
+            '%s'.
+            '%s'.
+          '</tr>',
+          $lines,
           ($has_where
             ? hsprintf('<td class="inline-which-diff">%s</td>', $where)
-              : null).
+            : null),
-            '<td class="inline-summary-content"'.$colspan.'>'.
+          phutil_tag(
-              '<div class="phabricator-remarkup">'.
+            'td',
-                $item['content'].
+            array(
-              '</div>'.
+              'class' => 'inline-summary-content',
-            '</td>'.
+              'colspan' => $colspan,
-          '</tr>';
+            ),
+            hsprintf(
+              '<div class="phabricator-remarkup">%s</div>',
+              $item['content'])));
       }
     }
 
@@ -100,7 +107,7 @@ final class PhabricatorInlineSummaryView extends AphrontView {
       array(
         'class' => 'phabricator-inline-summary-table',
       ),
-      new PhutilSafeHTML(implode("\n", $rows)));
+      phutil_implode_html("\n", $rows));
   }
 
 }

src/view/layout/AphrontPanelView.php

@@ -89,8 +89,7 @@ final class AphrontPanelView extends AphrontView {
       $header,
       $caption);
 
-    // TODO: [HTML] Make HTML safe.
+    $table = phutil_implode_html('', $this->renderChildren());
-    $table = phutil_safe_html(implode('', $this->renderChildren()));
 
     require_celerity_resource('aphront-panel-view-css');
 

src/view/layout/AphrontSideNavFilterView.php

@@ -294,7 +294,7 @@ final class AphrontSideNavFilterView extends AphrontView {
           ),
           array(
             $crumbs,
-            phutil_safe_html(implode('', $this->renderChildren())),
+            phutil_implode_html('', $this->renderChildren()),
           ))
       ));
   }

src/view/layout/PhabricatorProfileHeaderView.php

@@ -65,11 +65,12 @@ final class PhabricatorProfileHeaderView extends AphrontView {
         <tr>
           <td class="profile-header-description">%s</td>
         </tr>
-      </table>',
+      </table>
+      %s',
       $this->profileName,
       self::renderSingleView($this->profileActions),
       $image,
-      $description).
+      $description,
-      $this->renderChildren();
+      phutil_implode_html('', $this->renderChildren()));
   }
 }

src/view/layout/PhabricatorSourceCodeView.php

@@ -39,20 +39,18 @@ final class PhabricatorSourceCodeView extends AphrontView {
           pht('...'));
       } else {
         $content_number = $line_number;
-        $content_line = "\xE2\x80\x8B".$line;
+        $content_line = hsprintf("\xE2\x80\x8B%s", $line);
       }
 
       // TODO: Provide nice links.
 
-      $rows[] =
+      $rows[] = hsprintf(
         '<tr>'.
-          '<th class="phabricator-source-line">'.
+          '<th class="phabricator-source-line">%s</th>'.
-            $content_number.
+          '<td class="phabricator-source-code">%s</td>'.
-          '</th>'.
+        '</tr>',
-          '<td class="phabricator-source-code">'.
+        $content_number,
-            $content_line.
+        $content_line);
-          '</td>'.
-        '</tr>';
 
       if ($hit_limit) {
         break;
@@ -76,7 +74,7 @@ final class PhabricatorSourceCodeView extends AphrontView {
         array(
           'class' => implode(' ', $classes),
         ),
-        new PhutilSafeHTML(implode('', $rows))));
+        phutil_implode_html('', $rows)));
   }
 
 }

src/view/page/AphrontPageView.php

@@ -22,7 +22,7 @@ abstract class AphrontPageView extends AphrontView {
   }
 
   protected function getBody() {
-    return implode('', $this->renderChildren());
+    return phutil_implode_html('', $this->renderChildren());
   }
 
   protected function getTail() {
@@ -45,37 +45,37 @@ abstract class AphrontPageView extends AphrontView {
 
     $this->willRenderPage();
 
-    $title = phutil_escape_html($this->getTitle());
+    $title = $this->getTitle();
     $head  = $this->getHead();
     $body  = $this->getBody();
     $tail  = $this->getTail();
 
     $body_classes = $this->getBodyClasses();
 
-    $body = phutil_render_tag(
+    $body = phutil_tag(
       'body',
       array(
         'class' => nonempty($body_classes, null),
       ),
-      $body.$tail);
+      array($body, $tail));
 
-    $response = <<<EOHTML
+    $response = hsprintf(
-<!DOCTYPE html>
+      '<!DOCTYPE html>'.
-<html>
+      '<html>'.
-  <head>
+        '<head>'.
-    <meta charset="UTF-8" />
+          '<meta charset="UTF-8" />'.
-    <title>{$title}</title>
+          '<title>%s</title>'.
-    {$head}
+          '%s'.
-  </head>
+        '</head>'.
-  {$body}
+        '%s'.
-</html>
+      '</html>',
-
+      $title,
-EOHTML;
+      $head,
+      $body);
 
     $response = $this->willSendResponse($response);
 
-    // TODO: [HTML] Make HTML safe.
+    return $response;
-    return phutil_safe_html($response);
 
   }
 

src/view/page/PhabricatorBarePageView.php

@@ -55,13 +55,13 @@ class PhabricatorBarePageView extends AphrontPageView {
   protected function willRenderPage() {
     // We render this now to resolve static resources so they can appear in the
     // document head.
-    $this->bodyContent = implode('', $this->renderChildren());
+    $this->bodyContent = phutil_implode_html('', $this->renderChildren());
   }
 
   protected function getHead() {
     $framebust = null;
     if (!$this->getFrameable()) {
-      $framebust = '(top != self) && top.location.replace(self.location.href);';
+      $framebust = '(top == self) || top.location.replace(self.location.href);';
     }
 
     $viewport_tag = null;
@@ -78,22 +78,12 @@ class PhabricatorBarePageView extends AphrontPageView {
 
     $response = CelerityAPI::getStaticResourceResponse();
 
-    $head = array(
+    return hsprintf(
+      '%s<script type="text/javascript">%s window.__DEV__=%s;</script>%s',
       $viewport_tag,
-
+      $framebust,
-      '<script type="text/javascript">'.
+      (PhabricatorEnv::getEnvConfig('phabricator.developer-mode') ? '1' : '0'),
-        $framebust.
+      $response->renderResourcesOfType('css'));
-        'window.__DEV__='.
-        (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')
-          ? '1'
-          : '0').
-        ';'.
-      '</script>',
-
-      $response->renderResourcesOfType('css'),
-    );
-
-    return implode("\n", $head);
   }
 
   protected function getBody() {

src/view/page/PhabricatorStandardPageView.php

@@ -205,15 +205,11 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
 
     $response = CelerityAPI::getStaticResourceResponse();
 
-    $head = array(
+    return hsprintf(
+      '%s<style type="text/css">.PhabricatorMonospaced { font: %s; }</style>%s',
       parent::getHead(),
-      '<style type="text/css">'.
+      phutil_safe_html($monospaced),
-        '.PhabricatorMonospaced { font: '.$monospaced.'; }'.
+      $response->renderSingleResource('javelin-magical-init'));
-      '</style>',
-      $response->renderSingleResource('javelin-magical-init'),
-    );
-
-    return implode("\n", $head);
   }
 
   public function setGlyph($glyph) {
@@ -232,8 +228,9 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
     $console = $request->getApplicationConfiguration()->getConsole();
 
     if ($console) {
-      $response = str_replace(
+      $response = PhutilSafeHTML::applyFunction(
-        '<darkconsole />',
+        'str_replace',
+        hsprintf('<darkconsole />'),
         $console->render($request),
         $response);
     }
@@ -288,20 +285,22 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
     }
 
     return
-      phutil_render_tag(
+      phutil_tag(
         'div',
         array(
           'id' => 'base-page',
           'class' => 'phabricator-standard-page',
         ),
-        $developer_warning.
+        hsprintf(
-        $setup_warning.
+          '%s%s%s'.
-        $header_chrome.
           '<div class="phabricator-standard-page-body">'.
-          ($console ? '<darkconsole />' : null).
+            '%s%s<div style="clear: both;"></div>'.
-          parent::getBody().
+          '</div>',
-          '<div style="clear: both;"></div>'.
+        $developer_warning,
-        '</div>');
+        $setup_warning,
+        $header_chrome,
+        ($console ? hsprintf('<darkconsole />') : null),
+        parent::getBody()));
   }
 
   protected function getTail() {
@@ -350,7 +349,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
       $response->renderHTMLFooter(),
     );
 
-    return implode("\n", $tail);
+    return phutil_implode_html("\n", $tail);
   }
 
   protected function getBodyClasses() {