Convert AphrontFormView to safe HTML

Summary: Searched for `AphrontFormView` and then for `appendChild()`.

Test Plan: /login/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

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

src/applications/auth/controller/PhabricatorLoginController.php

@@ -243,22 +243,21 @@ final class PhabricatorLoginController
       // CSRF for logged-out users is vaugely tricky.
 
       if ($provider->isProviderRegistrationEnabled()) {
-        $title = pht("Login or Register with %s",
-          phutil_escape_html($provider_name));
+        $title = pht("Login or Register with %s", $provider_name);
         $body = pht('Login or register for Phabricator using your %s account.',
-          phutil_escape_html($provider_name));
+          $provider_name);
         $button = pht("Login or Register with %s",
           phutil_escape_html($provider_name));
       } else {
-        $title = pht("Login with %s",
-          phutil_escape_html($provider_name));
-        $body = pht('Login to your existing Phabricator account using your '.
-                '%s account.', phutil_escape_html($provider_name)).
-                '<br /><br />'.
-                '<strong>'.
-                  pht('You can not use %s to register a new account.',
-                  phutil_escape_html($provider_name)).
-                '</strong>';
+        $title = pht("Login with %s", $provider_name);
+        $body = hsprintf(
+          '%s<br /><br /><strong>%s</strong>',
+          pht(
+            'Login to your existing Phabricator account using your %s account.',
+            $provider_name),
+          pht(
+            'You can not use %s to register a new account.',
+            $provider_name));
         $button = pht("Log in with %s", phutil_escape_html($provider_name));
       }
 
@@ -276,8 +275,9 @@ final class PhabricatorLoginController
       $auth_form
         ->setUser($request->getUser())
         ->setMethod('GET')
-        ->appendChild(
-          '<p class="aphront-form-instructions">'.$body.'</p>')
+        ->appendChild(hsprintf(
+          '<p class="aphront-form-instructions">%s</p>',
+          $body))
         ->appendChild(
           id(new AphrontFormSubmitControl())
             ->setValue("{$button} \xC2\xBB"));
@@ -289,9 +289,9 @@ final class PhabricatorLoginController
     $panel->setWidth(AphrontPanelView::WIDTH_FORM);
     $panel->setNoBackground();
     foreach ($forms as $name => $form) {
-      $panel->appendChild('<h1>'.$name.'</h1>');
+      $panel->appendChild(phutil_tag('h1', array(), $name));
       $panel->appendChild($form);
-      $panel->appendChild('<br />');
+      $panel->appendChild(phutil_tag('br'));
     }
 
     $login_message = PhabricatorEnv::getEnvConfig('auth.login-message');

src/applications/conduit/controller/PhabricatorConduitConsoleController.php

@@ -80,10 +80,10 @@ final class PhabricatorConduitConsoleController
         id(new AphrontFormMarkupControl())
           ->setLabel('Errors')
           ->setValue($error_description))
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">Enter parameters using '.
         '<strong>JSON</strong>. For instance, to enter a list, type: '.
-        '<tt>["apple", "banana", "cherry"]</tt>');
+        '<tt>["apple", "banana", "cherry"]</tt>'));
 
     $params = $method_object->defineParamTypes();
     foreach ($params as $param => $desc) {

src/applications/differential/controller/DifferentialDiffCreateController.php

@@ -41,12 +41,15 @@ final class DifferentialDiffCreateController extends DifferentialController {
       ->setAction('/differential/diff/create/')
       ->setEncType('multipart/form-data')
       ->setUser($request->getUser())
-      ->appendChild(
-        '<p class="aphront-form-instructions">'.pht('The best way to create '.
-        'a Differential diff is by using %s, but you '.
-        'can also just paste a diff (e.g., from <tt>svn diff</tt> or '.
-        '<tt>git diff</tt>) into this box or upload it as a file if you '.
-        'really want.', $arcanist_link).'</p>')
+      ->appendChild(hsprintf(
+        '<p class="aphront-form-instructions">%s</p>',
+        pht(
+          'The best way to create a Differential diff is by using %s, but you '.
+            'can also just paste a diff (e.g., from %s or %s) into this box '.
+            'or upload it as a file if you really want.',
+          $arcanist_link,
+          phutil_tag('tt', array(), 'svn diff'),
+          phutil_tag('tt', array(), 'git diff'))))
       ->appendChild(
         id(new AphrontFormTextAreaControl())
           ->setLabel(pht('Raw Diff'))

src/applications/herald/controller/HeraldTestConsoleController.php

@@ -96,11 +96,11 @@ final class HeraldTestConsoleController extends HeraldController {
 
     $form = id(new AphrontFormView())
       ->setUser($user)
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">Enter an object to test rules '.
         'for, like a Diffusion commit (e.g., <tt>rX123</tt>) or a '.
         'Differential revision (e.g., <tt>D123</tt>). You will be shown the '.
-        'results of a dry run on the object.</p>')
+        'results of a dry run on the object.</p>'))
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel(pht('Object Name'))

src/applications/maniphest/controller/ManiphestBatchEditController.php

@@ -96,7 +96,7 @@ final class ManiphestBatchEditController extends ManiphestController {
           'name' => 'actions',
           'id'   => 'batch-form-actions',
         )));
-    $form->appendChild('<p>These tasks will be edited:</p>');
+    $form->appendChild(phutil_tag('p', array(), 'These tasks will be edited:'));
     $form->appendChild($list);
     $form->appendChild(
       id(new AphrontFormInsetView())

src/applications/maniphest/view/ManiphestTaskListView.php

@@ -45,7 +45,7 @@ final class ManiphestTaskListView extends ManiphestView {
       $views[] = $view->render();
     }
 
-    return implode("\n", $views);
+    return $this->renderHTMLView($views);
   }
 
 }

src/applications/metamta/controller/PhabricatorMetaMTAReceiveController.php

@@ -49,10 +49,9 @@ final class PhabricatorMetaMTAReceiveController
     $form->setUser($request->getUser());
     $form->setAction($this->getApplicationURI('/receive/'));
     $form
-      ->appendChild(
-        '<p class="aphront-form-instructions">'.
-          pht('This form will simulate sending mail to an object.').
-        '</p>')
+      ->appendChild(hsprintf(
+        '<p class="aphront-form-instructions">%s</p>',
+        pht('This form will simulate sending mail to an object.')))
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel(pht('To'))

src/applications/metamta/controller/PhabricatorMetaMTASendController.php

@@ -55,11 +55,12 @@ final class PhabricatorMetaMTASendController
         'target' => '_blank',
       ),
     pht('Configuring Outbound Email'));
-    $instructions =
-      '<p class="aphront-form-instructions">'.
-      pht('This form will send a normal email using the settings you have '.
-      'configured for Phabricator. For more information, see %s.', $doclink).
-      '</p>';
+    $instructions = hsprintf(
+      '<p class="aphront-form-instructions">%s</p>',
+      pht(
+        'This form will send a normal email using the settings you have '.
+          'configured for Phabricator. For more information, see %s.',
+        $doclink));
 
     $adapter = PhabricatorEnv::getEnvConfig('metamta.mail-adapter');
     $warning = null;

src/applications/people/controller/PhabricatorPeopleEditController.php

@@ -393,9 +393,9 @@ final class PhabricatorPeopleEditController
       ->setAction($request->getRequestURI()->alter('saved', null));
 
     if ($is_self) {
-      $form->appendChild(
+      $form->appendChild(hsprintf(
         '<p class="aphront-form-instructions">NOTE: You can not edit your own '.
-        'role.</p>');
+        'role.</p>'));
     }
 
     $form
@@ -448,10 +448,10 @@ final class PhabricatorPeopleEditController
     $form
       ->setUser($admin)
       ->setAction($request->getRequestURI())
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">You can use this certificate '.
         'to write scripts or bots which interface with Phabricator over '.
-        'Conduit.</p>');
+        'Conduit.</p>'));
 
     if ($user->getIsSystemAgent()) {
       $form
@@ -530,7 +530,7 @@ final class PhabricatorPeopleEditController
     $form
       ->setUser($admin)
       ->setAction($request->getRequestURI())
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">'.
           '<strong>Be careful when renaming users!</strong> '.
           'The old username will no longer be tied to the user, so anything '.
@@ -549,7 +549,7 @@ final class PhabricatorPeopleEditController
           'after their username is changed (their username is part of the '.
           'salt in the password hash). They will receive an email with '.
           'instructions on how to do this.'.
-        '</p>')
+        '</p>'))
       ->appendChild(
         id(new AphrontFormStaticControl())
           ->setLabel('Old Username')
@@ -622,7 +622,7 @@ final class PhabricatorPeopleEditController
     $form
       ->setUser($admin)
       ->setAction($request->getRequestURI())
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">'.
           '<strong>Be careful when deleting users!</strong> '.
           'If this user interacted with anything, it is generally better '.
@@ -636,7 +636,7 @@ final class PhabricatorPeopleEditController
           'It is generally safe to delete newly created users (and test users '.
           'and so on), but less safe to delete established users. If '.
           'possible, disable them instead.'.
-        '</p>')
+        '</p>'))
       ->appendChild(
         id(new AphrontFormStaticControl())
           ->setLabel('Username')
@@ -670,11 +670,11 @@ final class PhabricatorPeopleEditController
       ),
       'User Guide: Account Roles');
 
-    return
+    return hsprintf(
       '<p class="aphront-form-instructions">'.
-        'For a detailed explanation of account roles, see '.
-        $roles_link.'.'.
-      '</p>';
+        'For a detailed explanation of account roles, see %s.'.
+      '</p>',
+      $roles_link);
   }
 
 }

src/applications/project/controller/PhabricatorProjectProfileEditController.php

@@ -168,10 +168,10 @@ final class PhabricatorProjectProfileEditController
           ->setLabel('Blurb')
           ->setName('blurb')
           ->setValue($profile->getBlurb()))
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">NOTE: Policy settings are not '.
         'yet fully implemented. Some interfaces still ignore these settings, '.
-        'particularly "Visible To".</p>')
+        'particularly "Visible To".</p>'))
       ->appendChild(
         id(new AphrontFormPolicyControl())
           ->setUser($user)

src/applications/repository/controller/PhabricatorRepositoryCreateController.php

@@ -78,11 +78,11 @@ final class PhabricatorRepositoryCreateController
           ->setValue($repository->getName())
           ->setError($e_name)
           ->setCaption('Human-readable repository name.'))
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">Select a "Callsign" &mdash; a '.
         'short, uppercase string to identify revisions in this repository. If '.
         'you choose "EX", revisions in this repository will be identified '.
-        'with the prefix "rEX".</p>')
+        'with the prefix "rEX".</p>'))
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel('Callsign')

src/applications/repository/controller/PhabricatorRepositoryEditController.php

@@ -122,16 +122,15 @@ final class PhabricatorRepositoryEditController
           ->setLabel('Callsign')
           ->setName('callsign')
           ->setValue($repository->getCallsign()))
-      ->appendChild('
+      ->appendChild(hsprintf('
         <p class="aphront-form-instructions">'.
           'If source code in this repository uses a character '.
           'encoding other than UTF-8 (for example, ISO-8859-1), '.
           'specify it here. You can usually leave this field blank. '.
           'See User Guide: '.
-          '<a href="'.$encoding_doc_link.'">'.
-            'UTF-8 and Character Encoding'.
-          '</a> for more information.'.
-        '</p>')
+          '<a href="%s">UTF-8 and Character Encoding</a> for more information.'.
+        '</p>',
+        $encoding_doc_link))
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel('Encoding')
@@ -358,13 +357,14 @@ final class PhabricatorRepositoryEditController
     $form
       ->setUser($user)
       ->setAction('/repository/edit/'.$repository->getID().'/tracking/')
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">Phabricator can track '.
         'repositories, importing commits as they happen and notifying '.
         'Differential, Diffusion, Herald, and other services. To enable '.
         'tracking for a repository, configure it here and then start (or '.
         'restart) the daemons. More information is available in the '.
-        '<strong>'.$user_guide_link.'</strong>.</p>');
+        '<strong>%s</strong>.</p>',
+        $user_guide_link));
 
     $form
       ->appendChild(

src/applications/settings/panel/PhabricatorSettingsPanelConduit.php

@@ -67,10 +67,10 @@ final class PhabricatorSettingsPanelConduit
     $cert_form = new AphrontFormView();
     $cert_form
       ->setUser($user)
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">This certificate allows you to '.
         'authenticate over Conduit, the Phabricator API. Normally, you just '.
-        'run <tt>arc install-certificate</tt> to install it.')
+        'run <tt>arc install-certificate</tt> to install it.'))
       ->appendChild(
         id(new AphrontFormTextAreaControl())
           ->setLabel('Certificate')
@@ -87,10 +87,10 @@ final class PhabricatorSettingsPanelConduit
       ->setUser($user)
       ->setAction($this->getPanelURI())
       ->setWorkflow(true)
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">You can regenerate this '.
         'certificate, which will invalidate the old certificate and create '.
-        'a new one.</p>')
+        'a new one.</p>'))
       ->appendChild(
         id(new AphrontFormSubmitControl())
           ->setValue('Regenerate Certificate'));

src/applications/settings/panel/PhabricatorSettingsPanelEmailPreferences.php

@@ -165,7 +165,7 @@ final class PhabricatorSettingsPanelEmailPreferences
             'break threading in some clients.'));
 
     $form
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<br />'.
         '<p class="aphront-form-instructions">'.
           'You can customize what mail you receive from Phabricator here.'.
@@ -176,7 +176,7 @@ final class PhabricatorSettingsPanelEmailPreferences
           'still receive an email as long as at least one of the changes '.
           'is set to notify you.'.
         '</p>'
-        );
+        ));
 
     $mailtags = $preferences->getPreference('mailtags', array());
 

src/applications/settings/panel/PhabricatorSettingsPanelLDAP.php

@@ -35,10 +35,11 @@ final class PhabricatorSettingsPanelLDAP
       $unlink_form
         ->setUser($user)
         ->setAction('/ldap/login/')
-        ->appendChild(
+        ->appendChild(hsprintf(
           '<p class="aphront-form-instructions">There is currently no '.
           'LDAP account linked to your Phabricator account. You can link an '.
-          'account, which will allow you to use it to log into Phabricator</p>')
+          'account, which will allow you to use it to log into Phabricator.'.
+          '</p>'))
         ->appendChild(
           id(new AphrontFormTextControl())
           ->setLabel('LDAP username')
@@ -57,10 +58,10 @@ final class PhabricatorSettingsPanelLDAP
       $unlink_form = new AphrontFormView();
       $unlink_form
         ->setUser($user)
-        ->appendChild(
+        ->appendChild(hsprintf(
           '<p class="aphront-form-instructions">You may unlink this account '.
           'from your LDAP account. This will prevent you from logging in with '.
-          'your LDAP credentials.</p>')
+          'your LDAP credentials.</p>'))
         ->appendChild(
           id(new AphrontFormSubmitControl())
           ->addCancelButton('/ldap/unlink/', $unlink));

src/applications/settings/panel/PhabricatorSettingsPanelOAuth.php

@@ -84,11 +84,12 @@ final class PhabricatorSettingsPanelOAuth
     $forms[] = $form;
     if (!$oauth_info) {
       $form
-        ->appendChild(
-          '<p class="aphront-form-instructions">There is currently no '.
-          phutil_escape_html($provider_name).' account linked to your '.
-          'Phabricator account. You can link an account, which will allow you '.
-          'to use it to log into Phabricator.</p>');
+        ->appendChild(hsprintf(
+          '<p class="aphront-form-instructions">There is currently no %s '.
+            'account linked to your Phabricator account. You can link an '.
+            'account, which will allow you to use it to log into Phabricator.'.
+            '</p>',
+          $provider_name));
 
       $this->prepareAuthForm($form);
 
@@ -100,11 +101,12 @@ final class PhabricatorSettingsPanelOAuth
       $expires = $oauth_info->getTokenExpires();
 
       $form
-        ->appendChild(
+        ->appendChild(hsprintf(
           '<p class="aphront-form-instructions">Your account is linked with '.
-          'a '.phutil_escape_html($provider_name).' account. You may use your '.
-          phutil_escape_html($provider_name).' credentials to log into '.
-          'Phabricator.</p>')
+            'a %s account. You may use your %s credentials to log into '.
+            'Phabricator.</p>',
+          $provider_name,
+          $provider_name))
         ->appendChild(
           id(new AphrontFormStaticControl())
             ->setLabel($provider_name.' ID')
@@ -133,11 +135,12 @@ final class PhabricatorSettingsPanelOAuth
         $unlink_form = new AphrontFormView();
         $unlink_form
           ->setUser($user)
-          ->appendChild(
+          ->appendChild(hsprintf(
             '<p class="aphront-form-instructions">You may unlink this account '.
-            'from your '.phutil_escape_html($provider_name).' account. This '.
-            'will prevent you from logging in with your '.
-            phutil_escape_html($provider_name).' credentials.</p>')
+              'from your %s account. This will prevent you from logging in '.
+              'with your %s credentials.</p>',
+            $provider_name,
+            $provider_name))
           ->appendChild(
             id(new AphrontFormSubmitControl())
               ->addCancelButton('/oauth/'.$provider_key.'/unlink/', $unlink));
@@ -165,18 +168,22 @@ final class PhabricatorSettingsPanelOAuth
       $rappable_status = PhabricatorUserOAuthInfo::getRappableTokenStatus(
         $status);
       $beat = self::getBeat();
-      $rap = $beat . "Yo yo yo<br />".
+      $rap = hsprintf(
+        "%s Yo yo yo<br />".
         'My name\'s DJ Token and I\'m here to say<br />'.
         // pronounce as "dollar rappable status" for meter to work
-        "$rappable_status, hey hey hey hey<br />".
+        "%s, hey hey hey hey<br />".
         'I rap \'bout tokens, that might be why<br />'.
-        'I\'m such a cool and popular guy';
+        'I\'m such a cool and popular guy',
+        $beat,
+        $rappable_status);
 
       $token_form = new AphrontFormView();
       $token_form
         ->setUser($user)
-        ->appendChild(
-          '<p class="aphront-from-instructions">'.$rap.'</p>')
+        ->appendChild(hsprintf(
+          '<p class="aphront-form-instructions">%s</p>',
+          $rap))
         ->appendChild(
           id(new AphrontFormStaticControl())
             ->setLabel('Token Status')
@@ -282,12 +289,12 @@ final class PhabricatorSettingsPanelOAuth
   private static function getBeat() {
     // Gangsta's Paradise (karaoke version).
     // Chosen because it's the only thing I listen to.
-    $song_id = "Gangsta\\'s Paradise";
+    $song_id = "Gangsta's Paradise";
 
     // Make a musical note which you can click for the beat.
-    $beat = '<a href="javascript:void(0);" onclick="javascript:alert('.
-      "'Think about $song_id.'".
-      '); return 0;">&#9835; </a>';
+    $beat = hsprintf(
+      '<a href="javascript:void(0);" onclick="%s">&#9835;</a>',
+      jsprintf('alert(%s); return 0;', "Think about {$song_id}."));
     return $beat;
   }
 }

src/applications/settings/panel/PhabricatorSettingsPanelProfile.php

@@ -182,10 +182,10 @@ final class PhabricatorSettingsPanelProfile
                 'href' => $profile_uri,
               ),
               $profile_uri)))
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">Write something about yourself! '.
         'Make sure to include <strong>important information</strong> like '.
-        'your favorite Pokemon and which Starcraft race you play.</p>')
+        'your favorite Pokemon and which Starcraft race you play.</p>'))
       ->appendChild(
         id(new AphrontFormTextAreaControl())
           ->setLabel('Blurb')

src/applications/slowvote/controller/PhabricatorSlowvoteCreateController.php

@@ -65,9 +65,9 @@ final class PhabricatorSlowvoteCreateController
 
     $form = id(new AphrontFormView())
       ->setUser($user)
-      ->appendChild(
+      ->appendChild(hsprintf(
         '<p class="aphront-form-instructions">Resolve issues and build '.
-        'consensus through protracted deliberation.</p>')
+        'consensus through protracted deliberation.</p>'))
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setLabel('Question')

src/applications/slowvote/controller/PhabricatorSlowvotePollController.php

@@ -158,8 +158,9 @@ final class PhabricatorSlowvotePollController
 
     $form = id(new AphrontFormView())
       ->setUser($user)
-      ->appendChild(
-        '<p class="aphront-form-instructions">'.$instructions.'</p>')
+      ->appendChild(hsprintf(
+        '<p class="aphront-form-instructions">%s</p>',
+        $instructions))
       ->appendChild(
         id(new AphrontFormMarkupControl())
           ->setLabel('Vote')