diff --git a/conf/default.conf.php b/conf/default.conf.php index 426d00f774..6b856ea69f 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -1199,4 +1199,21 @@ return array( 'style.monospace' => '10px "Menlo", "Consolas", "Monaco", monospace', +// -- Debugging ------------------------------------------------------------- // + + // Enable this to change HTTP redirects into normal pages with a link to the + // redirection target. For example, after you submit a form you'll get a page + // saying "normally, you'd be redirected...". This is useful to examine + // service or profiler information on write pathways, or debug redirects. It + // also makes the UX horrible for normal use, so you should enable it only + // when debugging. + // + // NOTE: This does not currently work for forms with Javascript "workflow", + // since the redirect happens in Javascript. + 'debug.stop-on-redirect' => false, + + // Enable this to always profile every page. This is very slow! You should + // only enable it when debugging. + 'debug.profile-every-request' => false, + ); diff --git a/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php b/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php index e2a03d7804..0c38b60f99 100644 --- a/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php +++ b/src/aphront/console/plugin/DarkConsoleXHProfPlugin.php @@ -103,8 +103,8 @@ final class DarkConsoleXHProfPlugin extends DarkConsolePlugin { public function willShutdown() { - if (isset($_REQUEST['__profile__']) && - $_REQUEST['__profile__'] != 'all') { + if (DarkConsoleXHProfPluginAPI::isProfilerRequested() && + (DarkConsoleXHProfPluginAPI::isProfilerRequested() !== 'all')) { $this->xhprofID = DarkConsoleXHProfPluginAPI::stopProfiler(); } } diff --git a/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php b/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php index 38bc063aed..8ab312c4df 100644 --- a/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php +++ b/src/aphront/console/plugin/xhprof/DarkConsoleXHProfPluginAPI.php @@ -29,6 +29,18 @@ final class DarkConsoleXHProfPluginAPI { return extension_loaded('xhprof'); } + public static function isProfilerRequested() { + if (!empty($_REQUEST['__profile__'])) { + return $_REQUEST['__profile__']; + } + + if (PhabricatorEnv::getEnvConfig('debug.profile-every-request')) { + return PhabricatorEnv::getEnvConfig('debug.profile-every-request'); + } + + return false; + } + public static function includeXHProfLib() { // TODO: this is incredibly stupid, but we may not have Phutil metamodule // stuff loaded yet so we can't just phutil_get_library_root() our way @@ -41,8 +53,9 @@ final class DarkConsoleXHProfPluginAPI { require_once $root.'/externals/xhprof/xhprof_lib.php'; } + public static function hookProfiler() { - if (empty($_REQUEST['__profile__'])) { + if (!self::isProfilerRequested()) { return; } @@ -71,17 +84,41 @@ final class DarkConsoleXHProfPluginAPI { $data = serialize($data); $file_class = 'PhabricatorFile'; - // Since these happen on GET we can't do guarded writes. - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + // Since these happen on GET we can't do guarded writes. These also + // sometimes happen after we've disposed of the write guard; in this + // case we need to disable the whole mechanism. - $file = call_user_func( - array($file_class, 'newFromFileData'), - $data, - array( - 'mime-type' => 'application/xhprof', - 'name' => 'profile.xhprof', - )); - return $file->getPHID(); + $use_scope = AphrontWriteGuard::isGuardActive(); + if ($use_scope) { + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + } else { + AphrontWriteGuard::allowDangerousUnguardedWrites(true); + } + + $caught = null; + try { + $file = call_user_func( + array($file_class, 'newFromFileData'), + $data, + array( + 'mime-type' => 'application/xhprof', + 'name' => 'profile.xhprof', + )); + } catch (Exception $ex) { + $caught = $ex; + } + + if ($use_scope) { + unset($unguarded); + } else { + AphrontWriteGuard::allowDangerousUnguardedWrites(false); + } + + if ($caught) { + throw $caught; + } else { + return $file->getPHID(); + } } else { return null; } diff --git a/src/aphront/response/AphrontRedirectResponse.php b/src/aphront/response/AphrontRedirectResponse.php index f49681436b..7d060bb16b 100644 --- a/src/aphront/response/AphrontRedirectResponse.php +++ b/src/aphront/response/AphrontRedirectResponse.php @@ -34,15 +34,47 @@ class AphrontRedirectResponse extends AphrontResponse { return (string)$this->uri; } + public function shouldStopForDebugging() { + return PhabricatorEnv::getEnvConfig('debug.stop-on-redirect'); + } + public function getHeaders() { - $headers = array( - array('Location', $this->uri), - ); + $headers = array(); + if (!$this->shouldStopForDebugging()) { + $headers[] = array('Location', $this->uri); + } $headers = array_merge(parent::getHeaders(), $headers); return $headers; } public function buildResponseString() { + if ($this->shouldStopForDebugging()) { + $view = new PhabricatorStandardPageView(); + $view->setRequest($this->getRequest()); + $view->setApplicationName('Debug'); + $view->setTitle('Stopped on Redirect'); + + $error = new AphrontErrorView(); + $error->setSeverity(AphrontErrorView::SEVERITY_NOTICE); + $error->setTitle('Stopped on Redirect'); + + $link = phutil_render_tag( + 'a', + array( + 'href' => $this->getURI(), + ), + 'Continue to: '.phutil_escape_html($this->getURI())); + + $error->appendChild( + '

You were stopped here because debug.stop-on-redirect '. + 'is set in your configuration.

'. + '

'.$link.'

'); + + $view->appendChild($error); + + return $view->render(); + } + return ''; } diff --git a/src/aphront/writeguard/AphrontWriteGuard.php b/src/aphront/writeguard/AphrontWriteGuard.php index 38d6ce7ad2..e0a61284cf 100644 --- a/src/aphront/writeguard/AphrontWriteGuard.php +++ b/src/aphront/writeguard/AphrontWriteGuard.php @@ -108,6 +108,17 @@ final class AphrontWriteGuard { } + /** + * Determine if there is an active write guard. + * + * @return bool + * @task manage + */ + public static function isGuardActive() { + return (bool)self::$instance; + } + + /* -( Protecting Writes )-------------------------------------------------- */ diff --git a/webroot/index.php b/webroot/index.php index 1b5076134a..cfca18e57a 100644 --- a/webroot/index.php +++ b/webroot/index.php @@ -220,8 +220,8 @@ $headers = array_merge($headers, $response->getHeaders()); $sink->writeHeaders($headers); // TODO: This shouldn't be possible in a production-configured environment. -if (isset($_REQUEST['__profile__']) && - ($_REQUEST['__profile__'] == 'all')) { +if (DarkConsoleXHProfPluginAPI::isProfilerRequested() && + DarkConsoleXHProfPluginAPI::isProfilerRequested() === 'all') { $profile = DarkConsoleXHProfPluginAPI::stopProfiler(); $profile = '