Issues in WebStart.php - New Issues - Inspection of "refactor logging retrieval/display code out of Log..." - stwalkerster/waca - Measure and Improve Code Quality continuously with Scrutinizer

Failed Conditions

Branch — newinternal (104de7)

by Simon

created 2016-03-26 04:01 UTC

includes/WebStart.php (10 issues)

Labels

Severity

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php
namespace Waca;

use ErrorException;
use Exception;
use Waca\DataObjects\User;
use Waca\Exceptions\EnvironmentException;
use Waca\Exceptions\ReadableException;
use Waca\Helpers\BlacklistHelper;
use Waca\Helpers\FakeBlacklistHelper;
use Waca\Helpers\TypeAheadHelper;
use Waca\Providers\GlobalStateProvider;
use Waca\Router\IRequestRouter;
use Waca\Security\SecurityManager;
use Waca\Security\TokenManager;
use Waca\Tasks\ITask;
use Waca\Tasks\InternalPageBase;
use Waca\Tasks\PageBase;

/**
 * Internal application entry point.
 *
 * @package Waca
 */
class WebStart extends ApplicationBase
{
	/**
	 * @var IRequestRouter
	 */
	private $requestRouter;
	/** @var bool */
	private $isPublic;

	/**
	 * WebStart constructor.
	 *
	 * @param SiteConfiguration $configuration The site configuration
	 * @param IRequestRouter    $router        The request router to use
	 */
	public function __construct(SiteConfiguration $configuration, IRequestRouter $router)
	{
		parent::__construct($configuration);

		$this->requestRouter = $router;
	}

	/**
	 * @param ITask             $page
	 * @param SiteConfiguration $siteConfiguration
	 * @param PdoDatabase       $database
	 * @param PdoDatabase       $notificationsDatabase
	 *
	 * @return void
	 */
	protected function setupHelpers(
		ITask $page,
		SiteConfiguration $siteConfiguration,
		PdoDatabase $database,
		PdoDatabase $notificationsDatabase
	) {
		parent::setupHelpers($page, $siteConfiguration, $database, $notificationsDatabase);

		if ($page instanceof PageBase) {
			$page->setTokenManager(new TokenManager());

			if ($page instanceof InternalPageBase) {
				$page->setTypeAheadHelper(new TypeAheadHelper());

				$identificationVerifier = new IdentificationVerifier($page->getHttpHelper(), $siteConfiguration,
					$database);
				$page->setIdentificationVerifier($identificationVerifier);

				$page->setSecurityManager(new SecurityManager($identificationVerifier,
					$siteConfiguration->getForceIdentification()));

				if ($siteConfiguration->getTitleBlacklistEnabled()) {
					$page->setBlacklistHelper(new FakeBlacklistHelper());
				}
				else {
					$page->setBlacklistHelper(new BlacklistHelper($page->getHttpHelper(),
						$siteConfiguration->getMediawikiWebServiceEndpoint()));
				}
			}
		}
	}

	/**
	 * Application entry point.
	 *
	 * Sets up the environment and runs the application, performing any global cleanup operations when done.
	 */
	public function run()
	{
		try {
			if ($this->setupEnvironment()) {
				$this->main();
			}
		}
		catch (EnvironmentException $ex) {
			ob_end_clean();
			print Offline::getOfflineMessage(false, $ex->getMessage());
		}
		catch (ReadableException $ex) {
			ob_end_clean();
			print $ex->getReadableError();
		}
		finally {
			$this->cleanupEnvironment();
		}
	}

	/**
	 * Global exception handler
	 *
	 * Smarty would be nice to use, but it COULD BE smarty that throws the errors.
	 * Let's build something ourselves, and hope it works.
	 *
	 * @param $exception
	 *
	 * @category Security-Critical - has the potential to leak data when exception is thrown.
	 */
	public static function exceptionHandler(Exception $exception)
	{
		/** @global $siteConfiguration SiteConfiguration */
		global $siteConfiguration;
function myFunction($a, $b) {
    // Do something
}

		$errorDocument = <<<HTML
<!DOCTYPE html>
<html lang="en"><head>
<meta charset="utf-8">
<title>Oops! Something went wrong!</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link href="{$siteConfiguration->getBaseUrl()}/lib/bootstrap/css/bootstrap.min.css" rel="stylesheet">
<style>
  body {
    padding-top: 60px;
  }
</style>
<link href="{$siteConfiguration->getBaseUrl()}/lib/bootstrap/css/bootstrap-responsive.min.css" rel="stylesheet">
</head><body><div class="container">
<h1>Oops! Something went wrong!</h1>
<p>We'll work on fixing this for you, so why not come back later?</p>
<p class="muted">If our trained monkeys ask, tell them this error ID: <code>$1$</code></p>
$2$
</div></body></html>
HTML;

		$errorData = self::getExceptionData($exception);
		$errorData['server'] = $_SERVER;
		$errorData['get'] = $_GET;
		$errorData['post'] = $_POST;

		$state = serialize($errorData);
		$errorId = sha1($state);

		// Save the error for later analysis
		file_put_contents($siteConfiguration->getErrorLog() . '/' . $errorId . '.log', $state);

		// clear and discard any content that's been saved to the output buffer
		if (ob_get_level() > 0) {
			ob_end_clean();
		}

		// push error ID into the document.
		$message = str_replace('$1$', $errorId, $errorDocument);

		if ($siteConfiguration->getDebuggingTraceEnabled()) {
			ob_start();
			var_dump($errorData);
			$textErrorData = ob_get_contents();
			ob_end_clean();

			$message = str_replace('$2$', $textErrorData, $message);
		}
		else {
			$message = str_replace('$2$', "", $message);
		}

		header('HTTP/1.1 500 Internal Server Error');

		// output the document
		print $message;
	}

	public static function errorHandler($err_severity, $err_msg, $err_file, $err_line)

	{
		// call into the main exception handler above
		throw new ErrorException($err_msg, 0, $err_severity, $err_file, $err_line);
	}

	/**
	 * Environment setup
	 *
	 * This method initialises the tool environment. If the tool cannot be initialised correctly, it will return false
	 * and shut down prematurely.
	 *
	 * @return bool
	 * @throws EnvironmentException
	 */
	protected function setupEnvironment()
	{
		// initialise global exception handler
		set_exception_handler(array(self::class, 'exceptionHandler'));
		set_error_handler(array(self::class, 'errorHandler'), E_RECOVERABLE_ERROR);

		// start output buffering if necessary
		if (ob_get_level() === 0) {
			ob_start();
		}

		// initialise super-global providers
		WebRequest::setGlobalStateProvider(new GlobalStateProvider());

		if (Offline::isOffline()) {
			print Offline::getOfflineMessage($this->isPublic());
			ob_end_flush();

			return false;
		}

		// Call parent setup
		if (!parent::setupEnvironment()) {
			return false;
		}

		// Start up sessions
		Session::start();

		// Check the user is allowed to be logged in still. This must be before we call any user-loading functions and
		// get the current user cached.
		// I'm not sure if this function call being here is particularly a good thing, but it's part of starting up a
		// session I suppose.
		$this->checkForceLogout();

		// environment initialised!
		return true;
	}

	/**
	 * Main application logic
	 */
	protected function main()
	{
		// Get the right route for the request
		$page = $this->requestRouter->route();

		$siteConfiguration = $this->getConfiguration();
		$database = PdoDatabase::getDatabaseConnection('acc');

		if ($siteConfiguration->getIrcNotificationsEnabled()) {
			$notificationsDatabase = PdoDatabase::getDatabaseConnection('notifications');
		}
		else {
			// @todo federated table here?

			$notificationsDatabase = $database;
		}

		$this->setupHelpers($page, $siteConfiguration, $database, $notificationsDatabase);

		/* @todo Remove this global statement! It's here for User.php, which does far more than it should. */

		global $oauthHelper;
function myFunction($a, $b) {
    // Do something
}
		$oauthHelper = $page->getOAuthHelper();

		/* @todo Remove this global statement! It's here for Request.php, which does far more than it should. */

		global $globalXffTrustProvider;
function myFunction($a, $b) {
    // Do something
}
		$globalXffTrustProvider = $page->getXffTrustProvider();

		// run the route code for the request.
		$page->execute();
	}

	/**
	 * Any cleanup tasks should go here
	 *
	 * Note that we need to be very careful here, as exceptions may have been thrown and handled.
	 * This should *only* be for cleaning up, no logic should go here.
	 */
	protected function cleanupEnvironment()
	{
		// Clean up anything we splurged after sending the page.
		if (ob_get_level() > 0) {
			for ($i = ob_get_level(); $i > 0; $i--) {
				ob_end_clean();
			}
		}
	}

	/**
	 * @param Exception $exception
	 *
	 * @return null|array
	 */
	private static function getExceptionData($exception)
	{
		if ($exception == null) {
			return null;
		}

		return array(
			'exception' => get_class($exception),
			'message'   => $exception->getMessage(),
			'stack'     => $exception->getTraceAsString(),
			'previous'  => self::getExceptionData($exception->getPrevious()),
		);
	}

	private function checkForceLogout()
	{
		$database = PdoDatabase::getDatabaseConnection('acc');

		$sessionUserId = WebRequest::getSessionUserId();
		iF ($sessionUserId === null) {
			return;
		}

		// Note, User::getCurrent() caches it's result, which we *really* don't want to trigger.
		$currentUser = User::getById($sessionUserId, $database);

		if ($currentUser === false) {
			// Umm... this user has a session cookie with a userId set, but no user exists...
			Session::restart();
		}

		if ($currentUser->getForceLogout()) {
			Session::restart();

			$currentUser->setForceLogout(false);
			$currentUser->save();
		}
	}

	public function isPublic()
	{
		return $this->isPublic;
	}

	public function setPublic($isPublic)
	{
		$this->isPublic = $isPublic;
	}
}


Branch — newinternal (104de7)

includes/WebStart.php (10 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

1			<?php
2			namespace Waca;
3
4			use ErrorException;
5			use Exception;
6			use Waca\DataObjects\User;
7			use Waca\Exceptions\EnvironmentException;
8			use Waca\Exceptions\ReadableException;
9			use Waca\Helpers\BlacklistHelper;
10			use Waca\Helpers\FakeBlacklistHelper;
11			use Waca\Helpers\TypeAheadHelper;
12			use Waca\Providers\GlobalStateProvider;
13			use Waca\Router\IRequestRouter;
14			use Waca\Security\SecurityManager;
15			use Waca\Security\TokenManager;
16			use Waca\Tasks\ITask;
17			use Waca\Tasks\InternalPageBase;
18			use Waca\Tasks\PageBase;
19
20			/**
21			* Internal application entry point.
22			*
23			* @package Waca
24			*/
25			class WebStart extends ApplicationBase
26			{
27			/**
28			* @var IRequestRouter
29			*/
30			private $requestRouter;
31			/** @var bool */
32			private $isPublic;
33
34			/**
35			* WebStart constructor.
36			*
37			* @param SiteConfiguration $configuration The site configuration
38			* @param IRequestRouter $router The request router to use
39			*/
40			public function __construct(SiteConfiguration $configuration, IRequestRouter $router)
41			{
42			parent::__construct($configuration);
43
44			$this->requestRouter = $router;
45			}
46
47			/**
48			* @param ITask $page
49			* @param SiteConfiguration $siteConfiguration
50			* @param PdoDatabase $database
51			* @param PdoDatabase $notificationsDatabase
52			*
53			* @return void
54			*/
55			protected function setupHelpers(
56			ITask $page,
57			SiteConfiguration $siteConfiguration,
58			PdoDatabase $database,
59			PdoDatabase $notificationsDatabase
60			) {
61			parent::setupHelpers($page, $siteConfiguration, $database, $notificationsDatabase);
62
63			if ($page instanceof PageBase) {
64			$page->setTokenManager(new TokenManager());
65
66			if ($page instanceof InternalPageBase) {
67			$page->setTypeAheadHelper(new TypeAheadHelper());
68
69			$identificationVerifier = new IdentificationVerifier($page->getHttpHelper(), $siteConfiguration,
70			$database);
71			$page->setIdentificationVerifier($identificationVerifier);
72
73			$page->setSecurityManager(new SecurityManager($identificationVerifier,
74			$siteConfiguration->getForceIdentification()));
75
76			if ($siteConfiguration->getTitleBlacklistEnabled()) {
77			$page->setBlacklistHelper(new FakeBlacklistHelper());
78			}
79			else {
80			$page->setBlacklistHelper(new BlacklistHelper($page->getHttpHelper(),
81			$siteConfiguration->getMediawikiWebServiceEndpoint()));
82			}
83			}
84			}
85			}
86
87			/**
88			* Application entry point.
89			*
90			* Sets up the environment and runs the application, performing any global cleanup operations when done.
91			*/
92			public function run()
93			{
94			try {
95			if ($this->setupEnvironment()) {
96			$this->main();
97			}
98			}
99			catch (EnvironmentException $ex) {
100			ob_end_clean();
101			print Offline::getOfflineMessage(false, $ex->getMessage());
102			}
103			catch (ReadableException $ex) {
104			ob_end_clean();
105			print $ex->getReadableError();
106			}
107			finally {
108			$this->cleanupEnvironment();
109			}
110			}
111
112			/**
113			* Global exception handler
114			*
115			* Smarty would be nice to use, but it COULD BE smarty that throws the errors.
116			* Let's build something ourselves, and hope it works.
117			*
118			* @param $exception
119			*
120			* @category Security-Critical - has the potential to leak data when exception is thrown.
121			*/
122			public static function exceptionHandler(Exception $exception)
123			{
124			/** @global $siteConfiguration SiteConfiguration */
125			global $siteConfiguration;
			0 ignored issues – show Compatibility Best Practice introduced 2016-03-26 04:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
126
127			$errorDocument = <<<HTML
128			<!DOCTYPE html>
129			<html lang="en"><head>
130			<meta charset="utf-8">
131			<title>Oops! Something went wrong!</title>
132			<meta name="viewport" content="width=device-width, initial-scale=1.0">
133			<link href="{$siteConfiguration->getBaseUrl()}/lib/bootstrap/css/bootstrap.min.css" rel="stylesheet">
134			<style>
135			body {
136			padding-top: 60px;
137			}
138			</style>
139			<link href="{$siteConfiguration->getBaseUrl()}/lib/bootstrap/css/bootstrap-responsive.min.css" rel="stylesheet">
140			</head><body><div class="container">
141			<h1>Oops! Something went wrong!</h1>
142			<p>We'll work on fixing this for you, so why not come back later?</p>
143			<p class="muted">If our trained monkeys ask, tell them this error ID: <code>$1$</code></p>
144			$2$
145			</div></body></html>
146			HTML;
147
148			$errorData = self::getExceptionData($exception);
149			$errorData['server'] = $_SERVER;
150			$errorData['get'] = $_GET;
151			$errorData['post'] = $_POST;
152
153			$state = serialize($errorData);
154			$errorId = sha1($state);
155
156			// Save the error for later analysis
157			file_put_contents($siteConfiguration->getErrorLog() . '/' . $errorId . '.log', $state);
158
159			// clear and discard any content that's been saved to the output buffer
160			if (ob_get_level() > 0) {
161			ob_end_clean();
162			}
163
164			// push error ID into the document.
165			$message = str_replace('$1$', $errorId, $errorDocument);
166
167			if ($siteConfiguration->getDebuggingTraceEnabled()) {
168			ob_start();
169			var_dump($errorData);
170			$textErrorData = ob_get_contents();
171			ob_end_clean();
172
173			$message = str_replace('$2$', $textErrorData, $message);
174			}
175			else {
176			$message = str_replace('$2$', "", $message);
177			}
178
179			header('HTTP/1.1 500 Internal Server Error');
180
181			// output the document
182			print $message;
183			}
184
185			public static function errorHandler($err_severity, $err_msg, $err_file, $err_line)
			0 ignored issues – show Coding Style Naming introduced 2016-03-26 04:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this The parameter $err_severity is not named in camelCase. This check marks parameter names that have not been written in camelCase. In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes `databaseConnectionString`. Loading history... Coding Style Naming introduced 2016-03-26 04:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this The parameter $err_msg is not named in camelCase. This check marks parameter names that have not been written in camelCase. In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes `databaseConnectionString`. Loading history... Coding Style Naming introduced 2016-03-26 04:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this The parameter $err_file is not named in camelCase. This check marks parameter names that have not been written in camelCase. In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes `databaseConnectionString`. Loading history... Coding Style Naming introduced 2016-03-26 04:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this The parameter $err_line is not named in camelCase. This check marks parameter names that have not been written in camelCase. In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes `databaseConnectionString`. Loading history...
186			{
187			// call into the main exception handler above
188			throw new ErrorException($err_msg, 0, $err_severity, $err_file, $err_line);
189			}
190
191			/**
192			* Environment setup
193			*
194			* This method initialises the tool environment. If the tool cannot be initialised correctly, it will return false
195			* and shut down prematurely.
196			*
197			* @return bool
198			* @throws EnvironmentException
199			*/
200			protected function setupEnvironment()
201			{
202			// initialise global exception handler
203			set_exception_handler(array(self::class, 'exceptionHandler'));
204			set_error_handler(array(self::class, 'errorHandler'), E_RECOVERABLE_ERROR);
205
206			// start output buffering if necessary
207			if (ob_get_level() === 0) {
208			ob_start();
209			}
210
211			// initialise super-global providers
212			WebRequest::setGlobalStateProvider(new GlobalStateProvider());
213
214			if (Offline::isOffline()) {
215			print Offline::getOfflineMessage($this->isPublic());
216			ob_end_flush();
217
218			return false;
219			}
220
221			// Call parent setup
222			if (!parent::setupEnvironment()) {
223			return false;
224			}
225
226			// Start up sessions
227			Session::start();
228
229			// Check the user is allowed to be logged in still. This must be before we call any user-loading functions and
230			// get the current user cached.
231			// I'm not sure if this function call being here is particularly a good thing, but it's part of starting up a
232			// session I suppose.
233			$this->checkForceLogout();
234
235			// environment initialised!
236			return true;
237			}
238
239			/**
240			* Main application logic
241			*/
242			protected function main()
243			{
244			// Get the right route for the request
245			$page = $this->requestRouter->route();
246
247			$siteConfiguration = $this->getConfiguration();
248			$database = PdoDatabase::getDatabaseConnection('acc');
249
250			if ($siteConfiguration->getIrcNotificationsEnabled()) {
251			$notificationsDatabase = PdoDatabase::getDatabaseConnection('notifications');
252			}
253			else {
254			// @todo federated table here?
			0 ignored issues – show Coding Style Best Practice introduced 2016-03-26 04:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this Comments for TODO tasks are often forgotten in the code; it might be better to use a dedicated issue tracker. Loading history...
255			$notificationsDatabase = $database;
256			}
257
258			$this->setupHelpers($page, $siteConfiguration, $database, $notificationsDatabase);
259
260			/* @todo Remove this global statement! It's here for User.php, which does far more than it should. */
			0 ignored issues – show Coding Style Best Practice introduced 2016-03-26 04:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this Comments for TODO tasks are often forgotten in the code; it might be better to use a dedicated issue tracker. Loading history...
261			global $oauthHelper;
			0 ignored issues – show Compatibility Best Practice introduced 2016-03-26 04:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
262			$oauthHelper = $page->getOAuthHelper();
263
264			/* @todo Remove this global statement! It's here for Request.php, which does far more than it should. */
			0 ignored issues – show Coding Style Best Practice introduced 2016-03-26 04:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this Comments for TODO tasks are often forgotten in the code; it might be better to use a dedicated issue tracker. Loading history...
265			global $globalXffTrustProvider;
			0 ignored issues – show Compatibility Best Practice introduced 2016-03-26 04:10 UTC by Report Bug Copy Issue Report Show Similar Issues like this Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
266			$globalXffTrustProvider = $page->getXffTrustProvider();
267
268			// run the route code for the request.
269			$page->execute();
270			}
271
272			/**
273			* Any cleanup tasks should go here
274			*
275			* Note that we need to be very careful here, as exceptions may have been thrown and handled.
276			* This should only be for cleaning up, no logic should go here.
277			*/
278			protected function cleanupEnvironment()
279			{
280			// Clean up anything we splurged after sending the page.
281			if (ob_get_level() > 0) {
282			for ($i = ob_get_level(); $i > 0; $i--) {
283			ob_end_clean();
284			}
285			}
286			}
287
288			/**
289			* @param Exception $exception
290			*
291			* @return null\|array
292			*/
293			private static function getExceptionData($exception)
294			{
295			if ($exception == null) {
296			return null;
297			}
298
299			return array(
300			'exception' => get_class($exception),
301			'message' => $exception->getMessage(),
302			'stack' => $exception->getTraceAsString(),
303			'previous' => self::getExceptionData($exception->getPrevious()),
304			);
305			}
306
307			private function checkForceLogout()
308			{
309			$database = PdoDatabase::getDatabaseConnection('acc');
310
311			$sessionUserId = WebRequest::getSessionUserId();
312			iF ($sessionUserId === null) {
313			return;
314			}
315
316			// Note, User::getCurrent() caches it's result, which we really don't want to trigger.
317			$currentUser = User::getById($sessionUserId, $database);
318
319			if ($currentUser === false) {
320			// Umm... this user has a session cookie with a userId set, but no user exists...
321			Session::restart();
322			}
323
324			if ($currentUser->getForceLogout()) {
325			Session::restart();
326
327			$currentUser->setForceLogout(false);
328			$currentUser->save();
329			}
330			}
331
332			public function isPublic()
333			{
334			return $this->isPublic;
335			}
336
337			public function setPublic($isPublic)
338			{
339			$this->isPublic = $isPublic;
340			}
341			}
342

stwalkerster / waca

Branch — newinternal (104de7)

includes/WebStart.php (10 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

1. Pass all data via parameters

2. Create a class that maintains your state

Duplication Side-by-Side

Filter issues like