From 63acd90cefaf6717cc179d820ab34c359f5df09c Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 15 May 2014 19:21:36 -0700 Subject: [PATCH] Allow dashboard panels to detect rendering cycles and arrest stack overflows Summary: Ref T4986. Ref T4983. Panels will soon be able to contain other panels, either via Remarkup (`{W1}`) or maybe through new types of meta-panels. Allow panels to detect that they are being rendered very deeply and/or within themselves. Test Plan: Faked some errors, got failed panel renders. Since panels can't //really// contain other panels yet, this doesn't really have an impact. Reviewers: btrahan Reviewed By: btrahan Subscribers: chad, epriestley Maniphest Tasks: T4983, T4986 Differential Revision: https://secure.phabricator.com/D9140 --- resources/celerity/map.php | 52 ++++++++-------- ...bricatorDashboardPanelRenderController.php | 13 ++++ ...habricatorDashboardPanelViewController.php | 1 + ...abricatorDashboardPanelRenderingEngine.php | 60 +++++++++++++++++-- .../PhabricatorDashboardRenderingEngine.php | 1 + .../behavior-dashboard-async-panel.js | 1 + 6 files changed, 96 insertions(+), 32 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 8fc98d8861..fb8c760a53 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -52,7 +52,7 @@ return array( 'rsrc/css/application/conpherence/widget-pane.css' => 'bf275a6c', 'rsrc/css/application/contentsource/content-source-view.css' => '4b8b05d4', 'rsrc/css/application/countdown/timer.css' => '86b7b0a0', - 'rsrc/css/application/dashboard/dashboard.css' => '5b532b7b', + 'rsrc/css/application/dashboard/dashboard.css' => '2b41640b', 'rsrc/css/application/diff/inline-comment-summary.css' => '8cfd34e8', 'rsrc/css/application/differential/add-comment.css' => 'c478bcaa', 'rsrc/css/application/differential/changeset-view.css' => '1570a1ff', @@ -358,8 +358,8 @@ return array( 'rsrc/js/application/conpherence/behavior-pontificate.js' => '53f6f2dd', 'rsrc/js/application/conpherence/behavior-widget-pane.js' => '40b1ff90', 'rsrc/js/application/countdown/timer.js' => '889c96f3', - 'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => '4398eabb', - 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'aa3f313b', + 'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => 'fd965b41', + 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'fa187a68', 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'f2441746', 'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => '533a187b', 'rsrc/js/application/differential/behavior-comment-jump.js' => '71755c79', @@ -553,8 +553,8 @@ return array( 'javelin-behavior-conpherence-widget-pane' => '40b1ff90', 'javelin-behavior-countdown-timer' => '889c96f3', 'javelin-behavior-dark-console' => 'e9fdb5e5', - 'javelin-behavior-dashboard-async-panel' => '4398eabb', - 'javelin-behavior-dashboard-move-panels' => 'aa3f313b', + 'javelin-behavior-dashboard-async-panel' => 'fd965b41', + 'javelin-behavior-dashboard-move-panels' => 'fa187a68', 'javelin-behavior-device' => '03d6ed07', 'javelin-behavior-differential-add-reviewers-and-ccs' => '533a187b', 'javelin-behavior-differential-comment-jump' => '71755c79', @@ -701,7 +701,7 @@ return array( 'phabricator-core-css' => '40151074', 'phabricator-countdown-css' => '86b7b0a0', 'phabricator-crumbs-view-css' => '6a23399c', - 'phabricator-dashboard-css' => '5b532b7b', + 'phabricator-dashboard-css' => '2b41640b', 'phabricator-drag-and-drop-file-upload' => 'ae6abfba', 'phabricator-draggable-list' => '1681c4d4', 'phabricator-fatal-config-template-css' => '25d446d6', @@ -1133,12 +1133,6 @@ return array( 8 => 'phuix-action-list-view', 9 => 'phuix-action-view', ), - '4398eabb' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-dom', - 2 => 'javelin-workflow', - ), '441f2137' => array( 0 => 'javelin-behavior', @@ -1270,11 +1264,6 @@ return array( 2 => 'javelin-util', 3 => 'phabricator-shaped-request', ), - '7319e029' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-dom', - ), '62e18640' => array( 0 => 'javelin-install', @@ -1329,6 +1318,11 @@ return array( 1 => 'javelin-stratcom', 2 => 'javelin-dom', ), + '7319e029' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-dom', + ), '76f4ebed' => array( 0 => 'javelin-install', @@ -1598,15 +1592,6 @@ return array( 1 => 'javelin-stratcom', 2 => 'javelin-dom', ), - 'aa3f313b' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-dom', - 2 => 'javelin-util', - 3 => 'javelin-stratcom', - 4 => 'javelin-workflow', - 5 => 'phabricator-draggable-list', - ), 'ad7a69ca' => array( 0 => 'javelin-install', @@ -2034,11 +2019,26 @@ return array( 4 => 'javelin-stratcom', 5 => 'phabricator-shaped-request', ), + 'fa187a68' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-dom', + 2 => 'javelin-util', + 3 => 'javelin-stratcom', + 4 => 'javelin-workflow', + 5 => 'phabricator-draggable-list', + ), 'fbbce3bf' => array( 0 => 'phabricator-busy', 1 => 'javelin-behavior', ), + 'fd965b41' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-dom', + 2 => 'javelin-workflow', + ), 'fe2e0ba4' => array( 0 => 'javelin-behavior', diff --git a/src/applications/dashboard/controller/PhabricatorDashboardPanelRenderController.php b/src/applications/dashboard/controller/PhabricatorDashboardPanelRenderController.php index 501ab16fc9..a92ac821db 100644 --- a/src/applications/dashboard/controller/PhabricatorDashboardPanelRenderController.php +++ b/src/applications/dashboard/controller/PhabricatorDashboardPanelRenderController.php @@ -21,9 +21,22 @@ final class PhabricatorDashboardPanelRenderController return new Aphront404Response(); } + if ($request->isAjax()) { + $parent_phids = $request->getStrList('parentPanelPHIDs', null); + if ($parent_phids === null) { + throw new Exception( + pht( + 'Required parameter `parentPanelPHIDs` is not present in '. + 'request.')); + } + } else { + $parent_phids = array(); + } + $rendered_panel = id(new PhabricatorDashboardPanelRenderingEngine()) ->setViewer($viewer) ->setPanel($panel) + ->setParentPanelPHIDs($parent_phids) ->renderPanel(); if ($request->isAjax()) { diff --git a/src/applications/dashboard/controller/PhabricatorDashboardPanelViewController.php b/src/applications/dashboard/controller/PhabricatorDashboardPanelViewController.php index 944ee4e04b..e51ca5b758 100644 --- a/src/applications/dashboard/controller/PhabricatorDashboardPanelViewController.php +++ b/src/applications/dashboard/controller/PhabricatorDashboardPanelViewController.php @@ -41,6 +41,7 @@ final class PhabricatorDashboardPanelViewController $rendered_panel = id(new PhabricatorDashboardPanelRenderingEngine()) ->setViewer($viewer) ->setPanel($panel) + ->setParentPanelPHIDs(array()) ->renderPanel(); return $this->buildApplicationPage( diff --git a/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php b/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php index 253a244b77..dddafea66f 100644 --- a/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php +++ b/src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php @@ -5,6 +5,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { private $panel; private $viewer; private $enableAsyncRendering; + private $parentPanelPHIDs; /** * Allow the engine to render the panel via Ajax. @@ -14,6 +15,11 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { return $this; } + public function setParentPanelPHIDs(array $parents) { + $this->parentPanelPHIDs = $parents; + return $this; + } + public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; return $this; @@ -44,13 +50,15 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { $panel->getPanelType())); } - if ($this->enableAsyncRendering) { - if ($panel_type->shouldRenderAsync()) { - return $this->renderAsyncPanel($panel); - } - } - try { + $this->detectRenderingCycle($panel); + + if ($this->enableAsyncRendering) { + if ($panel_type->shouldRenderAsync()) { + return $this->renderAsyncPanel($panel); + } + } + return $panel_type->renderPanel($viewer, $panel); } catch (Exception $ex) { return $this->renderErrorPanel( @@ -75,6 +83,7 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { 'dashboard-async-panel', array( 'panelID' => $panel_id, + 'parentPanelPHIDs' => $this->parentPanelPHIDs, 'uri' => '/dashboard/panel/render/'.$panel->getID().'/', )); @@ -87,4 +96,43 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject { ->appendChild(pht('Loading...')); } + /** + * Detect graph cycles in panels, and deeply nested panels. + * + * This method throws if the current rendering stack is too deep or contains + * a cycle. This can happen if you embed layout panels inside each other, + * build a big stack of panels, or embed a panel in remarkup inside another + * panel. Generally, all of this stuff is ridiculous and we just want to + * shut it down. + * + * @param PhabricatorDashboardPanel Panel being rendered. + * @return void + */ + private function detectRenderingCycle(PhabricatorDashboardPanel $panel) { + if ($this->parentPanelPHIDs === null) { + throw new Exception( + pht( + 'You must call setParentPanelPHIDs() before rendering panels.')); + } + + $max_depth = 4; + if (count($this->parentPanelPHIDs) >= $max_depth) { + throw new Exception( + pht( + 'To render more than %s levels of panels nested inside other '. + 'panels, purchase a subscription to Phabricator Gold.', + new PhutilNumber($max_depth))); + } + + if (in_array($panel->getPHID(), $this->parentPanelPHIDs)) { + throw new Exception( + pht( + 'You awake in a twisting maze of mirrors, all alike. '. + 'You are likely to be eaten by a graph cycle. '. + 'Should you escape alive, you resolve to be more careful about '. + 'putting dashboard panels inside themselves.')); + } + } + + } diff --git a/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php b/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php index dd138a647d..6f4199ef02 100644 --- a/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php +++ b/src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php @@ -43,6 +43,7 @@ final class PhabricatorDashboardRenderingEngine extends Phobject { ->setViewer($viewer) ->setPanel($panel) ->setEnableAsyncRendering(true) + ->setParentPanelPHIDs(array()) ->renderPanel(); } $column_class = $layout_config->getColumnClass( diff --git a/webroot/rsrc/js/application/dashboard/behavior-dashboard-async-panel.js b/webroot/rsrc/js/application/dashboard/behavior-dashboard-async-panel.js index 03cf91e7bf..3938bfe643 100644 --- a/webroot/rsrc/js/application/dashboard/behavior-dashboard-async-panel.js +++ b/webroot/rsrc/js/application/dashboard/behavior-dashboard-async-panel.js @@ -10,6 +10,7 @@ JX.behavior('dashboard-async-panel', function(config) { panel.style.opacity = '0.5'; new JX.Workflow(config.uri) + .setData({parentPanelPHIDs: config.parentPanelPHIDs.join(',')}) .setHandler(function(r) { JX.DOM.replace(panel, JX.$H(r.panelMarkup)); })