From 80fb84bd94cc1d9378464e01b7dc22216ab01df8 Mon Sep 17 00:00:00 2001 From: vrana Date: Mon, 11 Feb 2013 18:59:30 -0800 Subject: [PATCH] Convert PhabricatorTransactionView to safe HTML Test Plan: Looked at revision detail with comments. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D4915 --- .../controller/ConpherenceController.php | 3 +- .../view/DifferentialRevisionCommentView.php | 16 +++--- .../query/browse/DiffusionBrowseQuery.php | 3 +- .../diffusion/view/DiffusionCommentView.php | 13 +++-- .../view/ManiphestTransactionDetailView.php | 8 +-- .../ponder/view/PonderPostBodyView.php | 2 +- .../form/control/AphrontFormCropControl.php | 2 +- .../layout/PhabricatorTransactionView.php | 52 +++++++++---------- 8 files changed, 47 insertions(+), 52 deletions(-) diff --git a/src/applications/conpherence/controller/ConpherenceController.php b/src/applications/conpherence/controller/ConpherenceController.php index 006049b9c7..67fecfe264 100644 --- a/src/applications/conpherence/controller/ConpherenceController.php +++ b/src/applications/conpherence/controller/ConpherenceController.php @@ -159,8 +159,7 @@ abstract class ConpherenceController extends PhabricatorController { $item->addClass('hide-unread-count'); } - // TODO: [HTML] Clean this up when we clean up HTML stuff in Conpherence. - $nav->addCustomBlock(phutil_safe_html($item->render())); + $nav->addCustomBlock($item->render()); } if (empty($conpherences) || $read) { $nav->addCustomBlock($this->getNoConpherencesBlock()); diff --git a/src/applications/differential/view/DifferentialRevisionCommentView.php b/src/applications/differential/view/DifferentialRevisionCommentView.php index 3f6a46bba0..226e9a9e82 100644 --- a/src/applications/differential/view/DifferentialRevisionCommentView.php +++ b/src/applications/differential/view/DifferentialRevisionCommentView.php @@ -87,10 +87,9 @@ final class DifferentialRevisionCommentView extends AphrontView { $comment, PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); - $content = - '
'. - $content. - '
'; + $content = hsprintf( + '
%s
', + $content); } $inline_render = $this->renderInlineComments(); @@ -208,11 +207,10 @@ final class DifferentialRevisionCommentView extends AphrontView { } if (!$hide_comments) { - $xaction_view->appendChild( - '
'. - $content. - '
'. - $this->renderSingleView($inline_render)); + $xaction_view->appendChild(hsprintf( + '
%s%s
', + $content, + $this->renderSingleView($inline_render))); } return $xaction_view->render(); diff --git a/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php b/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php index a4d2d44ceb..859941bd04 100644 --- a/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php +++ b/src/applications/diffusion/query/browse/DiffusionBrowseQuery.php @@ -119,8 +119,7 @@ abstract class DiffusionBrowseQuery { $readme_content = $highlighter ->getHighlightFuture($readme_content) ->resolve(); - $readme_content = nl2br($readme_content); - $readme_content = phutil_safe_html($readme_content); + $readme_content = phutil_escape_html_newlines($readme_content); require_celerity_resource('syntax-highlighting-css'); $class = 'remarkup-code'; diff --git a/src/applications/diffusion/view/DiffusionCommentView.php b/src/applications/diffusion/view/DiffusionCommentView.php index bdd2257a73..890052e667 100644 --- a/src/applications/diffusion/view/DiffusionCommentView.php +++ b/src/applications/diffusion/view/DiffusionCommentView.php @@ -139,13 +139,12 @@ final class DiffusionCommentView extends AphrontView { if (!strlen($comment->getContent()) && empty($this->inlineComments)) { return null; } else { - return - '
'. - $engine->getOutput( - $comment, - PhabricatorAuditComment::MARKUP_FIELD_BODY). - $this->renderSingleView($this->renderInlines()). - '
'; + return hsprintf( + '
%s%s
', + $engine->getOutput( + $comment, + PhabricatorAuditComment::MARKUP_FIELD_BODY), + $this->renderSingleView($this->renderInlines())); } } diff --git a/src/applications/maniphest/view/ManiphestTransactionDetailView.php b/src/applications/maniphest/view/ManiphestTransactionDetailView.php index a4b1222a99..dddaf21fa5 100644 --- a/src/applications/maniphest/view/ManiphestTransactionDetailView.php +++ b/src/applications/maniphest/view/ManiphestTransactionDetailView.php @@ -186,10 +186,10 @@ final class ManiphestTransactionDetailView extends ManiphestView { $comment_block = $this->markupEngine->getOutput( $comment_transaction, ManiphestTransaction::MARKUP_FIELD_BODY); - $comment_block = - '
'. - $comment_block. - '
'; + $comment_block = phutil_tag( + 'div', + array('class' => 'maniphest-transaction-comments phabricator-remarkup'), + $comment_block); } else { $comment_block = null; } diff --git a/src/applications/ponder/view/PonderPostBodyView.php b/src/applications/ponder/view/PonderPostBodyView.php index 4bfb4362ea..1f02c5e8ff 100644 --- a/src/applications/ponder/view/PonderPostBodyView.php +++ b/src/applications/ponder/view/PonderPostBodyView.php @@ -64,7 +64,7 @@ final class PonderPostBodyView extends AphrontView { $content); $author = $this->handles[$target->getAuthorPHID()]; - $actions = array($author->renderLink().' '.$this->action); + $actions = array(hsprintf('%s %s', $author->renderLink(), $this->action)); $author_link = $author->renderLink(); $xaction_view = id(new PhabricatorTransactionView()) ->setUser($user) diff --git a/src/view/form/control/AphrontFormCropControl.php b/src/view/form/control/AphrontFormCropControl.php index 26cb70c0e4..94b65cd7d4 100644 --- a/src/view/form/control/AphrontFormCropControl.php +++ b/src/view/form/control/AphrontFormCropControl.php @@ -29,7 +29,7 @@ final class AphrontFormCropControl extends AphrontFormControl { $file = $this->getValue(); if ($file === null) { - return phutil_render_tag( + return phutil_tag( 'img', array( 'src' => PhabricatorUser::getDefaultProfileImageURI() diff --git a/src/view/layout/PhabricatorTransactionView.php b/src/view/layout/PhabricatorTransactionView.php index b1c27ea7b8..89f0fb85ae 100644 --- a/src/view/layout/PhabricatorTransactionView.php +++ b/src/view/layout/PhabricatorTransactionView.php @@ -58,7 +58,7 @@ final class PhabricatorTransactionView extends AphrontView { $actions = $this->renderTransactionActions(); $style = $this->renderTransactionStyle(); $content = $this->renderTransactionContent(); - $classes = phutil_escape_html(implode(' ', $this->classes)); + $classes = implode(' ', $this->classes); $transaction_id = $this->anchorName ? 'anchor-'.$this->anchorName : null; @@ -69,15 +69,15 @@ final class PhabricatorTransactionView extends AphrontView { 'id' => $transaction_id, 'style' => $style, ), - // TODO: [HTML] Make HTML safe. - phutil_safe_html( - '
'. - '
'. - $info. - $actions. - '
'. - $content. - '
')); + hsprintf( + '
'. + '
%s%s
'. + '%s'. + '
', + $classes, + $info, + $actions, + $content)); } @@ -107,24 +107,24 @@ final class PhabricatorTransactionView extends AphrontView { ->setAnchorName($this->anchorName) ->render(); - $info[] = $anchor.phutil_tag( - 'a', - array( - 'href' => '#'.$this->anchorName, - ), - $this->anchorText); + $info[] = hsprintf( + '%s%s', + $anchor, + phutil_tag( + 'a', + array('href' => '#'.$this->anchorName), + $this->anchorText)); } - $info = implode(' · ', $info); + $info = phutil_implode_html(" \xC2\xB7 ", $info); - return - ''. - $info. - ''; + return hsprintf( + '%s', + $info); } private function renderTransactionActions() { - return implode('', $this->actions); + return phutil_implode_html('', $this->actions); } private function renderTransactionStyle() { @@ -140,10 +140,10 @@ final class PhabricatorTransactionView extends AphrontView { if (!$content) { return null; } - return - '
'. - implode('', $content). - '
'; + return phutil_tag( + 'div', + array('class' => 'phabricator-transaction-content'), + $this->renderSingleView($content)); } }