From 4eb84149c2f192e21f42ddf082dd52894e9b4c0d Mon Sep 17 00:00:00 2001 From: vrana Date: Tue, 12 Feb 2013 18:46:01 -0800 Subject: [PATCH] Convert everything to safe HTML Summary: Sgrepped for `"=~/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( '
'. "%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 .= '
'; + $details[] = hsprintf(''); } $table = new AphrontTableView($rows); @@ -90,11 +90,13 @@ final class DarkConsoleErrorLogPlugin extends DarkConsolePlugin { $table->setHeaders(array('Error')); $table->setNoDataString('No errors.'); - return '
'. - '
'.$table->render().'
'. - '
'.
-      $details.'
'. - '
'; + return hsprintf( + '
'. + '
%s
'. + '
%s
'. + '
', + $table->render(), + phutil_implode_html('', $details)); } } diff --git a/src/aphront/console/plugin/DarkConsoleEventPlugin.php b/src/aphront/console/plugin/DarkConsoleEventPlugin.php index 3c5bad75c2..3b9236b4a2 100644 --- a/src/aphront/console/plugin/DarkConsoleEventPlugin.php +++ b/src/aphront/console/plugin/DarkConsoleEventPlugin.php @@ -42,10 +42,10 @@ final class DarkConsoleEventPlugin extends DarkConsolePlugin { $out = array(); - $out[] = + $out[] = hsprintf( '
'. '

Registered Event Listeners

'. - '
'; + ''); $rows = array(); foreach ($data['listeners'] as $listener) { @@ -66,10 +66,10 @@ final class DarkConsoleEventPlugin extends DarkConsolePlugin { $out[] = $table->render(); - $out[] = + $out[] = hsprintf( '
'. '

Event Log

'. - '
'; + ''); $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); } } diff --git a/src/aphront/console/plugin/DarkConsoleRequestPlugin.php b/src/aphront/console/plugin/DarkConsoleRequestPlugin.php index 0fe56d9aba..0d04d58dc0 100644 --- a/src/aphront/console/plugin/DarkConsoleRequestPlugin.php +++ b/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); } } diff --git a/src/aphront/console/plugin/DarkConsoleServicesPlugin.php b/src/aphront/console/plugin/DarkConsoleServicesPlugin.php index 05bc861f0d..a43a83c785 100644 --- a/src/aphront/console/plugin/DarkConsoleServicesPlugin.php +++ b/src/aphront/console/plugin/DarkConsoleServicesPlugin.php @@ -149,20 +149,21 @@ final class DarkConsoleServicesPlugin extends DarkConsolePlugin { $log = $data['log']; $results = array(); - $results[] = + $results[] = hsprintf( '
'. - phutil_tag( - 'a', - array( - 'href' => $data['analyzeURI'], - 'class' => $data['didAnalyze'] - ? 'disabled button' - : 'green button', - ), - 'Analyze Query Plans'). + '%s'. '

Calls to External Services

'. '
'. - '
'; + '', + phutil_tag( + 'a', + array( + 'href' => $data['analyzeURI'], + 'class' => $data['didAnalyze'] + ? 'disabled button' + : 'green button', + ), + 'Analyze Query Plans')); $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); } } diff --git a/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php b/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php index 4574056deb..d4d69f1ffe 100644 --- a/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php +++ b/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php @@ -51,48 +51,52 @@ final class DarkConsoleXHProfPlugin extends DarkConsolePlugin { 'class' => 'bright-link', ), 'Installation Guide'); - return + return hsprintf( '
'. '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.'. + '', + $install_guide); } $result = array(); - $header = + $header = hsprintf( '
'. - phutil_tag( - 'a', - array( - 'href' => $profile_uri, - 'class' => $run - ? 'disabled button' - : 'green button', - ), - 'Profile Page'). + '%s'. '

XHProf Profiler

'. - '
'; + '', + phutil_tag( + 'a', + array( + 'href' => $profile_uri, + 'class' => $run + ? 'disabled button' + : 'green button', + ), + 'Profile Page')); $result[] = $header; if ($run) { - $result[] = - 'Profile Permalink'. - ''; + '', + $run, + $run); } else { - $result[] = + $result[] = hsprintf( '
'. 'Profiling was not enabled for this page. Use the button above '. 'to enable it.'. - '
'; + ''); } - return implode("\n", $result); + return phutil_implode_html("\n", $result); } diff --git a/src/aphront/response/AphrontWebpageResponse.php b/src/aphront/response/AphrontWebpageResponse.php index 4083b1be88..9bc2a54e83 100644 --- a/src/aphront/response/AphrontWebpageResponse.php +++ b/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); } } diff --git a/src/applications/auth/controller/PhabricatorEmailLoginController.php b/src/applications/auth/controller/PhabricatorEmailLoginController.php index 52f2827b91..3875de3c62 100644 --- a/src/applications/auth/controller/PhabricatorEmailLoginController.php +++ b/src/applications/auth/controller/PhabricatorEmailLoginController.php @@ -138,8 +138,8 @@ EOBODY; $panel = new AphrontPanelView(); $panel->setWidth(AphrontPanelView::WIDTH_FORM); - $panel->appendChild(' -

'.pht('Forgot Password / Email Login').'

'); + $panel->appendChild(phutil_tag('h1', array(), pht( + 'Forgot Password / Email Login'))); $panel->appendChild($email_auth); $panel->setNoBackground(); diff --git a/src/applications/auth/controller/PhabricatorLDAPLoginController.php b/src/applications/auth/controller/PhabricatorLDAPLoginController.php index d3cdd28c5a..bbe64ccbd6 100644 --- a/src/applications/auth/controller/PhabricatorLDAPLoginController.php +++ b/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('

'.pht('LDAP login').'

'); + $panel->appendChild(phutil_tag('h1', array(), pht('LDAP login'))); $panel->appendChild($ldap_form); $error_view = null; diff --git a/src/applications/auth/controller/PhabricatorLoginValidateController.php b/src/applications/auth/controller/PhabricatorLoginValidateController.php index 6ebef14e60..04b08b43f4 100644 --- a/src/applications/auth/controller/PhabricatorLoginValidateController.php +++ b/src/applications/auth/controller/PhabricatorLoginValidateController.php @@ -53,7 +53,9 @@ final class PhabricatorLoginValidateController '

%s

%s

%s

', pht('Login failed:'), $list, - pht('Clear your cookies and try again.'))); + pht( + 'Clear your cookies and try again.', + hsprintf('')))); $view->appendChild(hsprintf( '
'. '%s'. diff --git a/src/applications/auth/controller/PhabricatorOAuthDiagnosticsController.php b/src/applications/auth/controller/PhabricatorOAuthDiagnosticsController.php index 63959ee3e0..03330b8138 100644 --- a/src/applications/auth/controller/PhabricatorOAuthDiagnosticsController.php +++ b/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( '

These tests may be able to '. - 'help diagnose the root cause of problems you experience with '. - $provider->getProviderName() . - ' Authentication. Reload the page to run the tests again.

'); + 'help diagnose the root cause of problems you experience with %s '. + 'Authentication. Reload the page to run the tests again.

', + $provider->getProviderName())); $panel_view->appendChild($table_view); return $this->buildStandardPageResponse( diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index 41c371a2e5..9dbb9cb765 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/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( - '
'. - $response->buildResponseString(). - '
'); + $view->appendChild(hsprintf( + '
%s
', + $response->buildResponseString())); $response = new AphrontWebpageResponse(); $response->setContent($view->render()); return $response; diff --git a/src/applications/conduit/controller/PhabricatorConduitListController.php b/src/applications/conduit/controller/PhabricatorConduitListController.php index 617ca48288..e80a6fcba6 100644 --- a/src/applications/conduit/controller/PhabricatorConduitListController.php +++ b/src/applications/conduit/controller/PhabricatorConduitListController.php @@ -59,11 +59,11 @@ final class PhabricatorConduitListController $utils = new AphrontPanelView(); $utils->setHeader('Utilities'); - $utils->appendChild( + $utils->appendChild(hsprintf( ''); + '')); $utils->setWidth(AphrontPanelView::WIDTH_FULL); $this->setShowSideNav(false); diff --git a/src/applications/config/response/PhabricatorConfigResponse.php b/src/applications/config/response/PhabricatorConfigResponse.php index 479df39cd1..923313bb2a 100644 --- a/src/applications/config/response/PhabricatorConfigResponse.php +++ b/src/applications/config/response/PhabricatorConfigResponse.php @@ -23,20 +23,18 @@ final class PhabricatorConfigResponse extends AphrontHTMLResponse { $view = $this->view->render(); - $template = << - - - Phabricator Setup - {$resources} - - - {$view} - - -EOTEMPLATE; - - return $template; + return hsprintf( + ''. + ''. + ''. + ''. + 'Phabricator Setup'. + '%s'. + ''. + '%s'. + '', + $resources, + $view); } private function buildResources() { @@ -49,11 +47,12 @@ EOTEMPLATE; $resources = array(); foreach ($css as $path) { - $resources[] = ''; + $resources[] = phutil_tag( + 'style', + array('type' => 'text/css'), + Filesystem::readFile($webroot.'/rsrc/css/'.$path)); } - return implode("\n", $resources); + return phutil_implode_html("\n", $resources); } diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php index c6dee0bda6..65eee271f9 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/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() { diff --git a/src/applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php b/src/applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php index e9f9f8ee7b..811ef354be 100644 --- a/src/applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php +++ b/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( - '

Only archived, unsuccessful tasks can be retried.

'); + $dialog->appendChild(phutil_tag('p', array(), pht( + 'Only archived, unsuccessful tasks can be retried.'))); } break; case 'cancel': if ($can_cancel) { $dialog->setTitle('Really cancel task?'); - $dialog->appendChild( - '

The work this task represents will never be performed if you '. - 'cancel it. Are you sure you want to cancel it?

'); + $dialog->appendChild(phutil_tag('p', array(), pht( + 'The work this task represents will never be performed if you '. + 'cancel it. Are you sure you want to cancel it?'))); $dialog->addSubmitButton('Cancel Task'); } else { $dialog->setTitle('Can Not Cancel'); - $dialog->appendChild( - '

Only active tasks can be cancelled.

'); + $dialog->appendChild(phutil_tag('p', array(), pht( + 'Only active tasks can be cancelled.'))); } break; case 'release': if ($can_release) { $dialog->setTitle('Really free task lease?'); - $dialog->appendChild( - '

If the process which owns the task lease is still doing work '. + $dialog->appendChild(phutil_tag('p', array(), pht( + '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?

'); + 'want to free the lease?'))); $dialog->addSubmitButton('Free Lease'); } else { $dialog->setTitle('Can Not Free Lease'); - $dialog->appendChild( - '

Only active, leased tasks may have their leases freed.

'); + $dialog->appendChild(phutil_tag('p', array(), pht( + 'Only active, leased tasks may have their leases freed.'))); } break; default: diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php index afe4847c8c..6503ec2087 100644 --- a/src/applications/differential/controller/DifferentialDiffViewController.php +++ b/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( - "

".pht('This diff belongs to revision %s', $link)."

"); + $top_panel->appendChild(phutil_tag( + '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( - '

'.pht('Review the diff for '. - 'correctness. When you are satisfied, either create a new '. - 'revision or update an existing revision.')); + $action_panel->appendChild(hsprintf( + '

%s

', + pht( + 'Review the diff for correctness. When you are satisfied, either '. + 'create a new revision or update '. + 'an existing revision.', + hsprintf('')))); // TODO: implmenent optgroup support in AphrontFormSelectControl? $select = array(); diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index cedec46990..346b00727d 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/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 = '-'; + $na = phutil_tag('em', array(), '-'); $coverage = $this->getCoverage(); if (!$coverage) { diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php index 3032e15cc1..3c8d90dcfd 100644 --- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php +++ b/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 added.'); + $message = pht('This file was added.', $none); break; case DifferentialChangeType::FILE_IMAGE: - $message = pht('This image was added.'); + $message = pht('This image was added.', $none); break; case DifferentialChangeType::FILE_DIRECTORY: - $message = pht('This directory was added.'); + $message = pht( + 'This directory was added.', + $none); break; case DifferentialChangeType::FILE_BINARY: - $message = pht('This binary file was added.'); + $message = pht( + 'This binary file was added.', + $none); break; case DifferentialChangeType::FILE_SYMLINK: - $message = pht('This symlink was added.'); + $message = pht('This symlink was added.', $none); break; case DifferentialChangeType::FILE_SUBMODULE: - $message = pht('This submodule was added.'); + $message = pht( + 'This submodule was added.', + $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 deleted.'); + $message = pht('This file was deleted.', $none); break; case DifferentialChangeType::FILE_IMAGE: - $message = pht('This image was deleted.'); + $message = pht('This image was deleted.', $none); break; case DifferentialChangeType::FILE_DIRECTORY: - $message = pht('This directory was deleted.'); + $message = pht( + 'This directory was deleted.', + $none); break; case DifferentialChangeType::FILE_BINARY: - $message = pht('This binary file was deleted.'); + $message = pht( + 'This binary file was deleted.', + $none); break; case DifferentialChangeType::FILE_SYMLINK: - $message = pht('This symlink was deleted.'); + $message = pht( + 'This symlink was deleted.', + $none); break; case DifferentialChangeType::FILE_SUBMODULE: - $message = pht('This submodule was deleted.'); + $message = pht( + 'This submodule was deleted.', + $none); break; } break; @@ -235,10 +250,9 @@ abstract class DifferentialChangesetHTMLRenderer } } - return - '
'. - $message. - '
'; + return hsprintf( + '
%s
', + $message); } protected function renderPropertyChangeHeader() { @@ -279,15 +293,20 @@ abstract class DifferentialChangesetHTMLRenderer } } - return - ''. - ''. - ''. - ''. - ''. - ''. - implode('', $rows). - '
'.pht('Property Changes').''.pht('Old Value').''.pht('New Value').'
'; + array_unshift($rows, hsprintf( + ''. + '%s'. + '%s'. + '%s'. + '', + pht('Property Changes'), + 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( diff --git a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php index 18f49f6eaa..567116db05 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php @@ -20,32 +20,32 @@ final class DifferentialChangesetOneUpRenderer switch ($type) { case 'old': case 'new': - $out[] = ''; + $out[] = hsprintf(''); if ($type == 'old') { if ($p['htype']) { $class = 'left old'; } else { $class = 'left'; } - $out[] = ''.$p['line'].''; - $out[] = ''; - $out[] = ''.$p['render'].''; + $out[] = hsprintf('%s', $p['line']); + $out[] = hsprintf(''); + $out[] = hsprintf('%s', $class, $p['render']); } else if ($type == 'new') { if ($p['htype']) { $class = 'right new'; - $out[] = ''; + $out[] = hsprintf(''); } else { $class = 'right'; - $out[] = ''.$p['oline'].''; + $out[] = hsprintf('%s', $p['oline']); } - $out[] = ''.$p['line'].''; - $out[] = ''.$p['render'].''; + $out[] = hsprintf('%s', $p['line']); + $out[] = hsprintf('%s', $class, $p['render']); } - $out[] = ''; + $out[] = hsprintf(''); break; case 'inline': - $out[] = ''; - $out[] = ''; + $out[] = hsprintf(''); + $out[] = hsprintf(''); $inline = $this->buildInlineComment( $p['comment'], @@ -53,16 +53,16 @@ final class DifferentialChangesetOneUpRenderer $inline->setBuildScaffolding(false); $out[] = $inline->render(); - $out[] = ''; + $out[] = hsprintf(''); break; default: - $out[] = ''.$type.''; + $out[] = hsprintf('%s', $type); break; } } if ($out) { - return $this->wrapChangeInTable(implode('', $out)); + return $this->wrapChangeInTable(phutil_implode_html('', $out)); } return null; } diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php index 1fe2a13e32..698ef4113f 100644 --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -205,7 +205,7 @@ final class DifferentialChangesetTwoUpRenderer } } - $n_copy = ''; + $n_copy = hsprintf(''); $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 = ''; + $n_cov = hsprintf('', $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 = ''; + $n_copy = hsprintf('', $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( ''. - ''.$o_num.''. - ''.$o_text.''. - ''.$n_num.''. - $n_copy. + '%s'. + '%s'. + '%s'. + '%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. - ''. - "\xE2\x80\x8B".$n_text. + ''. + "\xE2\x80\x8B%s". ''. - $n_cov. - ''; + '%s'. + '', + 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( ''. ''. - ''.$comment_html.''. + '%s'. ''. ''. - ''; + '', + $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( ''. ''. ''. ''. - ''.$comment_html.''. - ''; + '%s'. + '', + $comment_html); } } if (!$old) { - $th_old = ''; + $th_old = hsprintf(''); } else { - $th_old = '1'; + $th_old = hsprintf('1', $vs); } if (!$new) { - $th_new = ''; + $th_new = hsprintf(''); } else { - $th_new = '1'; + $th_new = hsprintf('1', $id); } - $output = + $output = hsprintf( ''. - $th_old. - ''.$old.''. - $th_new. - ''. - $new. - ''. + '%s'. + '%s'. + '%s'. + '%s'. ''. - implode('', $html_old). - implode('', $html_new); + '%s'. + '%s', + $th_old, + $old, + $th_new, + $new, + phutil_implode_html('', $html_old), + phutil_implode_html('', $html_new)); $output = $this->wrapChangeInTable($output); diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index 5a3f531a1e..422e8fd9ab 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -221,15 +221,20 @@ final class DifferentialChangesetListView extends AphrontView { ), array('Changes discarded. ', $link)); - $template = - ''. - ''. - ''. - '
%s%s
'; - return array( - 'l' => sprintf($template, $div, ''), - 'r' => sprintf($template, '', $div), + 'l' => hsprintf( + ''. + ''. + ''. + '
%s
', + $div), + + 'r' => hsprintf( + ''. + ''. + ''. + '
%s
', + $div), ); } diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index c94392b302..f739781194 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/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) { diff --git a/src/applications/feed/story/PhabricatorFeedStoryAudit.php b/src/applications/feed/story/PhabricatorFeedStoryAudit.php index 9fe789b046..fd3d55e463 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryAudit.php +++ b/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( - $this->linkTo($author_phid). - " {$verb} commit ". - $this->linkTo($commit_phid). - "."); + $view->setTitle(hsprintf( + '%s %s commit %s.', + $this->linkTo($author_phid), + $verb, + $this->linkTo($commit_phid))); $view->setEpoch($this->getEpoch()); diff --git a/src/applications/feed/story/PhabricatorFeedStoryDifferential.php b/src/applications/feed/story/PhabricatorFeedStoryDifferential.php index 6382c39f8b..34ca11edd8 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryDifferential.php +++ b/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; } diff --git a/src/applications/feed/story/PhabricatorFeedStoryManiphest.php b/src/applications/feed/story/PhabricatorFeedStoryManiphest.php index c6c0348d6d..9f4fc2c23c 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryManiphest.php +++ b/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; } diff --git a/src/applications/feed/story/PhabricatorFeedStoryPhriction.php b/src/applications/feed/story/PhabricatorFeedStoryPhriction.php index 9e22f7d5b2..4791214b75 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryPhriction.php +++ b/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( - $this->linkTo($author_phid). - " {$verb} the document ". - $this->linkTo($document_phid).'.'); + $view->setTitle(hsprintf( + '%s %s the document %s.', + $this->linkTo($author_phid), + $verb, + $this->linkTo($document_phid))); $view->setEpoch($data->getEpoch()); $action = $data->getValue('action'); diff --git a/src/applications/feed/story/PhabricatorFeedStoryProject.php b/src/applications/feed/story/PhabricatorFeedStoryProject.php index c517a98496..7b4733dfbc 100644 --- a/src/applications/feed/story/PhabricatorFeedStoryProject.php +++ b/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 '. - $this->linkTo($proj_phid). - ' from '. - $this->renderString($old). - ' to '. - $this->renderString($new). - '.'; + $action = hsprintf( + 'renamed project %s from %s to %s.', + $this->linkTo($proj_phid), + $this->renderString($old), + $this->renderString($new)); } else { - $action = 'created project '. - $this->linkTo($proj_phid). - ' (as '. - $this->renderString($new). - ').'; + $action = hsprintf( + 'created project %s (as %s).', + $this->linkTo($proj_phid), + $this->renderString($new)); } break; case PhabricatorProjectTransactionType::TYPE_STATUS: - $action = 'changed project '. - $this->linkTo($proj_phid). - ' status from '. - $this->renderString( - PhabricatorProjectStatus::getNameForStatus($old)). - ' to '. - $this->renderString( - PhabricatorProjectStatus::getNameForStatus($new)). - '.'; + $action = hsprintf( + 'changed project %s status from %s to %s.', + $this->linkTo($proj_phid), + $this->renderString(PhabricatorProjectStatus::getNameForStatus($old)), + $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 '. - $this->linkTo($proj_phid).': '. - $this->renderHandleList($add).'.'; + $action = hsprintf( + 'added members to project %s: %s.', + $this->linkTo($proj_phid), + $this->renderHandleList($add)); } else if (empty($add)) { - $action = 'removed members from project '. - $this->linkTo($proj_phid).': '. - $this->renderHandleList($rem).'.'; + $action = hsprintf( + 'removed members from project %s: %s.', + $this->linkTo($proj_phid), + $this->renderHandleList($rem)); } else { - $action = 'changed members of project '. - $this->linkTo($proj_phid).', added: '. - $this->renderHandleList($add).'; removed: '. - $this->renderHandleList($rem).'.'; + $action = hsprintf( + 'changed members of project %s, added: %s; removed: %s.', + $this->linkTo($proj_phid), + $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; diff --git a/src/applications/feed/view/PhabricatorFeedStoryView.php b/src/applications/feed/view/PhabricatorFeedStoryView.php index 0ddca8df06..fb2d8df8f1 100644 --- a/src/applications/feed/view/PhabricatorFeedStoryView.php +++ b/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); diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php index 1d66a67f52..870fab3e68 100644 --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -437,10 +437,10 @@ final class HeraldTranscriptController extends HeraldController { $panel = new AphrontPanelView(); $panel->setHeader('Rule Details'); - $panel->appendChild( - '
    '. - implode("\n", $rule_markup). - '
'); + $panel->appendChild(phutil_tag( + 'ul', + array('class' => 'herald-explain-list'), + $rule_markup)); return $panel; } diff --git a/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php b/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php index 67abdf407d..15058a7302 100644 --- a/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldDefaultSpecification.php +++ b/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; } diff --git a/src/applications/maniphest/controller/ManiphestSavedQueryListController.php b/src/applications/maniphest/controller/ManiphestSavedQueryListController.php index b7dd5709ff..2f80e6028d 100644 --- a/src/applications/maniphest/controller/ManiphestSavedQueryListController.php +++ b/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', diff --git a/src/applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php b/src/applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php index 64fa264dc8..ee185362c5 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php @@ -18,10 +18,9 @@ final class ManiphestTaskDescriptionPreviewController ManiphestTask::MARKUP_FIELD_DESCRIPTION, $request->getUser()); - $content = - '
'. - $output. - '
'; + $content = hsprintf( + '
%s
', + $output); return id(new AphrontAjaxResponse()) ->setContent($content); diff --git a/src/applications/maniphest/view/ManiphestTransactionDetailView.php b/src/applications/maniphest/view/ManiphestTransactionDetailView.php index dddaf21fa5..62294c91ae 100644 --- a/src/applications/maniphest/view/ManiphestTransactionDetailView.php +++ b/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) { diff --git a/src/applications/notification/controller/PhabricatorNotificationPanelController.php b/src/applications/notification/controller/PhabricatorNotificationPanelController.php index b25365c607..19a811fe4e 100644 --- a/src/applications/notification/controller/PhabricatorNotificationPanelController.php +++ b/src/applications/notification/controller/PhabricatorNotificationPanelController.php @@ -20,25 +20,23 @@ final class PhabricatorNotificationPanelController $notifications_view = $builder->buildView(); $content = $notifications_view->render(); } else { - $content = - '
'. - 'You have no notifications.'. - '
'; + $content = hsprintf( + '
%s
', + pht('You have no notifications.')); } - $content = - '
'. - pht('Notifications'). - '
'. - $content. - '
'. - phutil_tag( - 'a', - array( - 'href' => '/notification/', - ), - 'View All Notifications'). - '
'; + $content = hsprintf( + '
%s
'. + '%s'. + '
%s
', + pht('Notifications'), + $content, + phutil_tag( + 'a', + array( + 'href' => '/notification/', + ), + 'View All Notifications')); $unread_count = id(new PhabricatorFeedStoryNotification()) ->countUnread($user); diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php index 3bba99426a..c255897caf 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php @@ -142,7 +142,7 @@ final class PhabricatorPeopleProfileController $nav->appendChild($header); - $content = '
'.$content.'
'; + $content = hsprintf('
%s
', $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( '

Activity Feed

-
- '.$view->render().' -
-
'; +
%s
+
', + $view->render()); } } diff --git a/src/applications/phame/controller/post/PhamePostPreviewController.php b/src/applications/phame/controller/post/PhamePostPreviewController.php index 38b007178d..06fe3de4da 100644 --- a/src/applications/phame/controller/post/PhamePostPreviewController.php +++ b/src/applications/phame/controller/post/PhamePostPreviewController.php @@ -23,7 +23,7 @@ extends PhameController { PhamePost::MARKUP_FIELD_BODY, $user); - $content = '
'.$content.'
'; + $content = hsprintf('
%s
', $content); return id(new AphrontAjaxResponse())->setContent($content); } diff --git a/src/applications/phame/skins/PhameBasicBlogSkin.php b/src/applications/phame/skins/PhameBasicBlogSkin.php index 569bb6b3b3..2c6f91e97b 100644 --- a/src/applications/phame/skins/PhameBasicBlogSkin.php +++ b/src/applications/phame/skins/PhameBasicBlogSkin.php @@ -123,7 +123,7 @@ abstract class PhameBasicBlogSkin extends PhameBlogSkin { } protected function render404Page() { - return '

404 Not Found

'; + return hsprintf('

404 Not Found

'); } final public function getResourceURI($resource) { diff --git a/src/applications/phame/skins/PhameBasicTemplateBlogSkin.php b/src/applications/phame/skins/PhameBasicTemplateBlogSkin.php index 39dc7df989..7ccd2cffe9 100644 --- a/src/applications/phame/skins/PhameBasicTemplateBlogSkin.php +++ b/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; } diff --git a/src/applications/pholio/controller/PholioMockViewController.php b/src/applications/pholio/controller/PholioMockViewController.php index 5e0f72c016..361b7257fb 100644 --- a/src/applications/pholio/controller/PholioMockViewController.php +++ b/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')); } diff --git a/src/applications/phpast/controller/PhabricatorXHPASTViewFramesetController.php b/src/applications/phpast/controller/PhabricatorXHPASTViewFramesetController.php index c21f9f2b0b..bec14d9a21 100644 --- a/src/applications/phpast/controller/PhabricatorXHPASTViewFramesetController.php +++ b/src/applications/phpast/controller/PhabricatorXHPASTViewFramesetController.php @@ -14,12 +14,15 @@ final class PhabricatorXHPASTViewFramesetController $response = new AphrontWebpageResponse(); $response->setFrameable(true); - $response->setContent( - ''. - ''. - ''. - ''. - ''); + $response->setContent(hsprintf( + ''. + ''. + ''. + ''. + '', + $id, + $id, + $id)); return $response; } diff --git a/src/applications/ponder/view/PonderAnswerListView.php b/src/applications/ponder/view/PonderAnswerListView.php index fa42d222a9..87c4674ae9 100644 --- a/src/applications/ponder/view/PonderAnswerListView.php +++ b/src/applications/ponder/view/PonderAnswerListView.php @@ -70,7 +70,8 @@ final class PonderAnswerListView extends AphrontView { $panel->appendChild($view); $panel->appendChild($commentview); - $panel->appendChild('
'); + $panel->appendChild( + hsprintf('
')); } diff --git a/src/applications/project/controller/PhabricatorProjectMembersEditController.php b/src/applications/project/controller/PhabricatorProjectMembersEditController.php index 1e84d2823e..c788b9c466 100644 --- a/src/applications/project/controller/PhabricatorProjectMembersEditController.php +++ b/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('
'); + $panel->appendChild(phutil_tag('br')); $panel->appendChild($faux_form); $nav = $this->buildLocalNavigation($project); diff --git a/src/applications/project/controller/PhabricatorProjectProfileController.php b/src/applications/project/controller/PhabricatorProjectProfileController.php index 60e9639e9a..cc49ab3c9c 100644 --- a/src/applications/project/controller/PhabricatorProjectProfileController.php +++ b/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 = '
'.$content.'
'; + $content = hsprintf('
%s
', $content); $header->appendChild($content); return $this->buildStandardPageResponse( @@ -178,22 +178,22 @@ final class PhabricatorProjectProfileController $affiliated = array(); foreach ($handles as $phids => $handle) { - $affiliated[] = '
  • '.$handle->renderLink().'
    • '; + $affiliated[] = phutil_tag('li', array(), $handle->renderLink()); } if ($affiliated) { - $affiliated = ''; + $affiliated = phutil_tag('ul', array(), $affiliated); } else { - $affiliated = '

    No one is affiliated with this project.

    '; + $affiliated = hsprintf('

    %s

    ', pht( + 'No one is affiliated with this project.')); } - return + return hsprintf( '
    '. '

    People

    '. - '
    '. - $affiliated. - '
    '. - '
    '; + '
    %s
    '. + '', + $affiliated); } private function renderFeedPage( @@ -220,13 +220,12 @@ final class PhabricatorProjectProfileController $builder->setUser($this->getRequest()->getUser()); $view = $builder->buildView(); - return + return hsprintf( '
    '. '

    Activity Feed

    '. - '
    '. - $view->render(). - '
    '. - '
    '; + '
    %s
    '. + '', + $view->render()); } @@ -257,9 +256,9 @@ final class PhabricatorProjectProfileController } if (empty($tasks)) { - $task_views = 'No open tasks.'; + $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( '
    -

    '. - "Open Tasks ({$open})". - '

    '. +

    Open Tasks (%s)

    '. '
    '. - $task_views. - '
    '. - $more_link. - '
    '. + '%s'. + '
    %s
    '. '
    -
    '; + ', + $open, + $task_views, + $more_link); return $content; } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelLDAP.php b/src/applications/settings/panel/PhabricatorSettingsPanelLDAP.php index 25df530fdb..4adb085f63 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanelLDAP.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelLDAP.php @@ -75,7 +75,7 @@ final class PhabricatorSettingsPanelLDAP foreach ($forms as $name => $form) { if ($name) { - $panel->appendChild('

    '.$name.'


    '); + $panel->appendChild(hsprintf('

    %s


    ', $name)); } $panel->appendChild($form); } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelOAuth.php b/src/applications/settings/panel/PhabricatorSettingsPanelOAuth.php index b95640ba48..a1df581abb 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanelOAuth.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelOAuth.php @@ -215,7 +215,7 @@ final class PhabricatorSettingsPanelOAuth foreach ($forms as $name => $form) { if ($name) { - $panel->appendChild('

    '.$name.'


    '); + $panel->appendChild(hsprintf('

    %s


    ', $name)); } $panel->appendChild($form); } diff --git a/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php b/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php index f2fe76132d..5c39fda9d1 100644 --- a/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php +++ b/src/applications/slowvote/controller/PhabricatorSlowvotePollController.php @@ -181,7 +181,7 @@ final class PhabricatorSlowvotePollController $panel->setWidth(AphrontPanelView::WIDTH_WIDE); $panel->appendChild($form); - $panel->appendChild('

    '); + $panel->appendChild(hsprintf('

    ')); $panel->appendChild($result_markup); return $this->buildStandardPageResponse( diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php index 07bcbafd47..96c106e60e 100644 --- a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php +++ b/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( - ''.pht('This comment has been deleted.').''); + $event->appendChild(phutil_tag('em', array(), pht( + 'This comment has been deleted.'))); } $events[] = $event; diff --git a/src/applications/uiexample/examples/PhabricatorUIPagerExample.php b/src/applications/uiexample/examples/PhabricatorUIPagerExample.php index f50a335547..1d34e84792 100644 --- a/src/applications/uiexample/examples/PhabricatorUIPagerExample.php +++ b/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( '

    '. 'Use AphrontPagerView to render a pager element.'. - '

    '); + '

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

    '. 'You can show more or fewer pages of surrounding context.'. - '

    '); + '

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

    '. 'When it is prohibitively expensive or complex to attain a complete '. 'count of the items, you can select one extra item and set '. 'hasMorePages(true) if it exists, creating an inexact pager.'. - '

    '); + '

    ')); $inexact_pager = new AphrontPagerView(); $inexact_pager->setPageSize($page_size); diff --git a/src/docs/developer/rendering_html.diviner b/src/docs/developer/rendering_html.diviner index 54b410a3af..5cd2c661d5 100644 --- a/src/docs/developer/rendering_html.diviner +++ b/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 `
    `. +while converting newlines to `
    `. 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', $evil, $evil2, $evil); + + // We have a lint rule disallowing this. + hsprintf($evil); + = Deprecated Functions = The functions @{function@libphutil:phutil_render_tag} and diff --git a/src/infrastructure/celerity/CelerityStaticResourceResponse.php b/src/infrastructure/celerity/CelerityStaticResourceResponse.php index 6891fa4fff..5cd0ff8a34 100644 --- a/src/infrastructure/celerity/CelerityStaticResourceResponse.php +++ b/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 ''; + return hsprintf( + '', + phutil_safe_html($data)); } else { return ''; } diff --git a/src/infrastructure/diff/PhabricatorInlineCommentPreviewController.php b/src/infrastructure/diff/PhabricatorInlineCommentPreviewController.php index cd9a65be96..77e6e67e60 100644 --- a/src/infrastructure/diff/PhabricatorInlineCommentPreviewController.php +++ b/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); diff --git a/src/infrastructure/diff/view/PhabricatorInlineSummaryView.php b/src/infrastructure/diff/view/PhabricatorInlineSummaryView.php index b7c9f61bde..f8686ebba0 100644 --- a/src/infrastructure/diff/view/PhabricatorInlineSummaryView.php +++ b/src/infrastructure/diff/view/PhabricatorInlineSummaryView.php @@ -79,19 +79,26 @@ final class PhabricatorInlineSummaryView extends AphrontView { $where = idx($item, 'where'); - $colspan = ($has_where ? '' : ' colspan="2"'); - $rows[] = + $colspan = ($has_where ? null : 2); + $rows[] = hsprintf( ''. - ''.$lines.''. - ($has_where - ? hsprintf('%s', $where) - : null). - ''. - '
    '. - $item['content']. - '
    '. - ''. - ''; + '%s'. + '%s'. + '%s'. + '', + $lines, + ($has_where + ? hsprintf('%s', $where) + : null), + phutil_tag( + 'td', + array( + 'class' => 'inline-summary-content', + 'colspan' => $colspan, + ), + hsprintf( + '
    %s
    ', + $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)); } } diff --git a/src/view/layout/AphrontPanelView.php b/src/view/layout/AphrontPanelView.php index 596af7c7c8..0ae35ef0df 100644 --- a/src/view/layout/AphrontPanelView.php +++ b/src/view/layout/AphrontPanelView.php @@ -89,8 +89,7 @@ final class AphrontPanelView extends AphrontView { $header, $caption); - // TODO: [HTML] Make HTML safe. - $table = phutil_safe_html(implode('', $this->renderChildren())); + $table = phutil_implode_html('', $this->renderChildren()); require_celerity_resource('aphront-panel-view-css'); diff --git a/src/view/layout/AphrontSideNavFilterView.php b/src/view/layout/AphrontSideNavFilterView.php index d76e5d450a..e4b2b7c9b9 100644 --- a/src/view/layout/AphrontSideNavFilterView.php +++ b/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()), )) )); } diff --git a/src/view/layout/PhabricatorProfileHeaderView.php b/src/view/layout/PhabricatorProfileHeaderView.php index ad4fc0397d..c99b802907 100644 --- a/src/view/layout/PhabricatorProfileHeaderView.php +++ b/src/view/layout/PhabricatorProfileHeaderView.php @@ -65,11 +65,12 @@ final class PhabricatorProfileHeaderView extends AphrontView { %s - ', + + %s', $this->profileName, self::renderSingleView($this->profileActions), $image, - $description). - $this->renderChildren(); + $description, + phutil_implode_html('', $this->renderChildren())); } } diff --git a/src/view/layout/PhabricatorSourceCodeView.php b/src/view/layout/PhabricatorSourceCodeView.php index 166c951b36..f957c6ef88 100644 --- a/src/view/layout/PhabricatorSourceCodeView.php +++ b/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( ''. - ''. - $content_number. - ''. - ''. - $content_line. - ''. - ''; + '%s'. + '%s'. + '', + $content_number, + $content_line); 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))); } } diff --git a/src/view/page/AphrontPageView.php b/src/view/page/AphrontPageView.php index 1db19ea260..8f3704dca2 100644 --- a/src/view/page/AphrontPageView.php +++ b/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 = << - - - - {$title} - {$head} - - {$body} - - -EOHTML; + $response = hsprintf( + ''. + ''. + ''. + ''. + '%s'. + '%s'. + ''. + '%s'. + '', + $title, + $head, + $body); $response = $this->willSendResponse($response); - // TODO: [HTML] Make HTML safe. - return phutil_safe_html($response); + return $response; } diff --git a/src/view/page/PhabricatorBarePageView.php b/src/view/page/PhabricatorBarePageView.php index 2ae201970f..d92836cabb 100644 --- a/src/view/page/PhabricatorBarePageView.php +++ b/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%s', $viewport_tag, - - '', - - $response->renderResourcesOfType('css'), - ); - - return implode("\n", $head); + $framebust, + (PhabricatorEnv::getEnvConfig('phabricator.developer-mode') ? '1' : '0'), + $response->renderResourcesOfType('css')); } protected function getBody() { diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index d692411ca2..ce9cfe42aa 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -205,15 +205,11 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { $response = CelerityAPI::getStaticResourceResponse(); - $head = array( + return hsprintf( + '%s%s', parent::getHead(), - '', - $response->renderSingleResource('javelin-magical-init'), - ); - - return implode("\n", $head); + phutil_safe_html($monospaced), + $response->renderSingleResource('javelin-magical-init')); } 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( + 'str_replace', + hsprintf(''), $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. - $setup_warning. - $header_chrome. - '
    '. - ($console ? '' : null). - parent::getBody(). - '
    '. - '
    '); + hsprintf( + '%s%s%s'. + '
    '. + '%s%s
    '. + '
    ', + $developer_warning, + $setup_warning, + $header_chrome, + ($console ? hsprintf('') : 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() {