Issues in EnvironmentChecker.php - Fixed Issues - Inspection of "Merge pull request #22 from helpfulrobot/convert-t..." - silverstripe/silverstripe-environmentcheck - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( e090e4...18d79e )

by Damian

created 2015-11-22 21:28 UTC

code/EnvironmentChecker.php (7 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

/**
 * Provides an interface for checking the given EnvironmentCheckSuite.
 */
class EnvironmentChecker extends RequestHandler {

	/**
	 * @var array
	 */
	private static $url_handlers = array(
		'' => 'index',
	);

	/**
	 * @var string
	 */
	protected $checkSuiteName;

	/**
	 * @var string
	 */
	protected $title;

	/**
	 * @var int
	 */
	protected $errorCode = 500;

	/**
	 * @var null|string
	 */
	private static $to_email_address = null;

	/**
	 * @var null|string
	 */
	private static $from_email_address = null;

	/**
	 * @var bool
	 */
	private static $email_results = false;

	/**
	 * @var bool Log results via {@link SS_Log}
	 */
	private static $log_results_warning = false;

	/**
	 * @var int Maps to {@link Zend_Log} levels. Defaults to Zend_Log::WARN
	 */
	private static $log_results_warning_level = 4;

	/**
	 * @var bool Log results via {@link SS_Log}
	 */
	private static $log_results_error = false;

	/**
	 * @var int Maps to {@link Zend_Log} levels. Defaults to Zend_Log::ALERT
	 */
	private static $log_results_error_level = 1;

	/**
	 * @param string $checkSuiteName
	 * @param string $title
	 */
	function __construct($checkSuiteName, $title) {

		parent::__construct();
		
		$this->checkSuiteName = $checkSuiteName;
		$this->title = $title;
	}

	/**
	 * @param string $permission
	 *
	 * @throws SS_HTTPResponse_Exception
	 */
	function init($permission = 'ADMIN') {
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
		// if the environment supports it, provide a basic auth challenge and see if it matches configured credentials
		if(defined('ENVCHECK_BASICAUTH_USERNAME') && defined('ENVCHECK_BASICAUTH_PASSWORD')) {
			if(isset($_SERVER['PHP_AUTH_USER']) && isset($_SERVER['PHP_AUTH_PW'])) {
				// authenticate the input user/pass with the configured credentials
				if(
					!(
						$_SERVER['PHP_AUTH_USER'] == ENVCHECK_BASICAUTH_USERNAME
						&& $_SERVER['PHP_AUTH_PW'] == ENVCHECK_BASICAUTH_PASSWORD
					)
				) {
					$response = new SS_HTTPResponse(null, 401);
					$response->addHeader('WWW-Authenticate', "Basic realm=\"Environment check\"");
					// Exception is caught by RequestHandler->handleRequest() and will halt further execution
					$e = new SS_HTTPResponse_Exception(null, 401);
					$e->setResponse($response);
					throw $e;
				}
			} else {
				$response = new SS_HTTPResponse(null, 401);
				$response->addHeader('WWW-Authenticate', "Basic realm=\"Environment check\"");
				// Exception is caught by RequestHandler->handleRequest() and will halt further execution
				$e = new SS_HTTPResponse_Exception(null, 401);
				$e->setResponse($response);
				throw $e;
			}
		} else {
			if(!$this->canAccess(null, $permission)) return $this->httpError(403);
		}
	}

	/**
	 * @param null|int|Member $member
	 * @param string $permission
	 *
	 * @return bool
	 *
	 * @throws SS_HTTPResponse_Exception
	 */
	function canAccess($member = null, $permission = "ADMIN") {

		if(!$member) {
			$member = Member::currentUser();
		}

		if(!$member) {
			$member = BasicAuth::requireLogin('Environment Checker', $permission, false);
		}

		// We allow access to this controller regardless of live-status or ADMIN permission only
		// if on CLI.  Access to this controller is always allowed in "dev-mode", or of the user is ADMIN.
		if(
			Director::isDev() 
			|| Director::is_cli()
			|| empty($permission)
			|| Permission::checkMember($member, $permission)
		) {
			return true;
		}

		// Extended access checks.
		// "Veto" style, return NULL to abstain vote.
		$canExtended = null;
		$results = $this->extend('canAccess', $member);
		if($results && is_array($results)) {
			if(!min($results)) return false;
			else return true;
		}

		return false;
	}

	/**
	 * @return SS_HTTPResponse
	 */
	function index() {

		$response = new SS_HTTPResponse;
		$result = EnvironmentCheckSuite::inst($this->checkSuiteName)->run();

		if(!$result->ShouldPass()) {
			$response->setStatusCode($this->errorCode);
		}

		$resultText = $result->customise(array(
			"URL" => Director::absoluteBaseURL(),
			"Title" => $this->title,
			"Name" => $this->checkSuiteName,
			"ErrorCode" => $this->errorCode,
		))->renderWith("EnvironmentChecker");

		if ($this->config()->email_results && !$result->ShouldPass()) {
			$email = new Email($this->config()->from_email_address, $this->config()->to_email_address, $this->title, $resultText);
			$email->send();
		}

		// Optionally log errors and warnings individually
		foreach($result->Details() as $detail) {
			if($this->config()->log_results_warning && $detail->StatusCode == EnvironmentCheck::WARNING) {
				$this->log(
					sprintf('EnvironmentChecker warning at "%s" check. Message: %s', $detail->Check, $detail->Message),
					$this->config()->log_results_warning_level
				);
			} elseif($this->config()->log_results_error && $detail->StatusCode == EnvironmentCheck::ERROR) {
				$this->log(
					sprintf('EnvironmentChecker error at "%s" check. Message: %s', $detail->Check, $detail->Message),
					$this->config()->log_results_error_level
				);
			}
		}

		// output the result as JSON if requested
		if(
			$this->getRequest()->getExtension() == 'json'
			|| strpos($this->getRequest()->getHeader('Accept'), 'application/json') !== false
		) {
			$response->setBody($result->toJSON());
			$response->addHeader('Content-Type', 'application/json');
			return $response;
		}

		$response->setBody($resultText);
		
		return $response;
	}

	/**
	 * @param string $message
	 * @param int $level
	 */
	public function log($message, $level) {
		SS_Log::log($message, $level);
	}

	/**
	 * Set the HTTP status code that should be returned when there's an error.
	 *
	 * @param int $errorCode
	 */
	function setErrorCode($errorCode) {

		$this->errorCode = $errorCode;
	}

	/**
	 * @deprecated
	 * @param string $from
	 */
	public static function set_from_email_address($from) {
		Deprecation::notice('2.0', 'Use config API instead');
		Config::inst()->update('EnvironmentChecker', 'from_email_address', $from);
	}

	/**
	 * @deprecated
	 * @return null|string
	 */
	public static function get_from_email_address() {
		Deprecation::notice('2.0', 'Use config API instead');
		return Config::inst()->get('EnvironmentChecker', 'from_email_address');
	}

	/**
	 * @deprecated
	 * @param string $to
	 */
	public static function set_to_email_address($to) {
		Deprecation::notice('2.0', 'Use config API instead');
		Config::inst()->update('EnvironmentChecker', 'to_email_address',  $to);
	}

	/**
	 * @deprecated
	 * @return null|string
	 */
	public static function get_to_email_address() {
		Deprecation::notice('2.0', 'Use config API instead');
		return Config::inst()->get('EnvironmentChecker', 'to_email_address');
	}

	/**
	 * @deprecated
	 * @param bool $results
	 */
	public static function set_email_results($results) {
		Deprecation::notice('2.0', 'Use config API instead');
		Config::inst()->update('EnvironmentChecker', 'email_results', $results);
	}

	/**
	 * @deprecated
	 * @return bool
	 */
	public static function get_email_results() {
		Deprecation::notice('2.0', 'Use config API instead');
		return Config::inst()->get('EnvironmentChecker', 'email_results');
	}
}


1			<?php
2
3			/**
4			* Provides an interface for checking the given EnvironmentCheckSuite.
5			*/
6			class EnvironmentChecker extends RequestHandler {
			0 ignored issues – show Coding Style introduced 2015-11-20 23:50 UTC by Report Bug Copy Issue Report Show Similar Issues like this As per PSR2, the opening brace for this class should be on a new line. Loading history...
7			/**
8			* @var array
9			*/
10			private static $url_handlers = array(
11			'' => 'index',
12			);
13
14			/**
15			* @var string
16			*/
17			protected $checkSuiteName;
18
19			/**
20			* @var string
21			*/
22			protected $title;
23
24			/**
25			* @var int
26			*/
27			protected $errorCode = 500;
28
29			/**
30			* @var null\|string
31			*/
32			private static $to_email_address = null;
33
34			/**
35			* @var null\|string
36			*/
37			private static $from_email_address = null;
38
39			/**
40			* @var bool
41			*/
42			private static $email_results = false;
43
44			/**
45			* @var bool Log results via {@link SS_Log}
46			*/
47			private static $log_results_warning = false;
48
49			/**
50			* @var int Maps to {@link Zend_Log} levels. Defaults to Zend_Log::WARN
51			*/
52			private static $log_results_warning_level = 4;
53
54			/**
55			* @var bool Log results via {@link SS_Log}
56			*/
57			private static $log_results_error = false;
58
59			/**
60			* @var int Maps to {@link Zend_Log} levels. Defaults to Zend_Log::ALERT
61			*/
62			private static $log_results_error_level = 1;
63
64			/**
65			* @param string $checkSuiteName
66			* @param string $title
67			*/
68			function __construct($checkSuiteName, $title) {
			0 ignored issues – show Best Practice introduced 2015-11-20 23:50 UTC by Report Bug Copy Issue Report Show Similar Issues like this It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
69			parent::__construct();
70
71			$this->checkSuiteName = $checkSuiteName;
72			$this->title = $title;
73			}
74
75			/**
76			* @param string $permission
77			*
78			* @throws SS_HTTPResponse_Exception
79			*/
80			function init($permission = 'ADMIN') {
			0 ignored issues – show Best Practice introduced 2015-11-20 23:50 UTC by Report Bug Copy Issue Report Show Similar Issues like this It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history... Coding Style introduced 2015-11-20 23:50 UTC by Report Bug Copy Issue Report Show Similar Issues like this `init` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
81			// if the environment supports it, provide a basic auth challenge and see if it matches configured credentials
82			if(defined('ENVCHECK_BASICAUTH_USERNAME') && defined('ENVCHECK_BASICAUTH_PASSWORD')) {
83			if(isset($_SERVER['PHP_AUTH_USER']) && isset($_SERVER['PHP_AUTH_PW'])) {
84			// authenticate the input user/pass with the configured credentials
85			if(
86			!(
87			$_SERVER['PHP_AUTH_USER'] == ENVCHECK_BASICAUTH_USERNAME
88			&& $_SERVER['PHP_AUTH_PW'] == ENVCHECK_BASICAUTH_PASSWORD
89			)
90			) {
91			$response = new SS_HTTPResponse(null, 401);
92			$response->addHeader('WWW-Authenticate', "Basic realm=\"Environment check\"");
93			// Exception is caught by RequestHandler->handleRequest() and will halt further execution
94			$e = new SS_HTTPResponse_Exception(null, 401);
95			$e->setResponse($response);
96			throw $e;
97			}
98			} else {
99			$response = new SS_HTTPResponse(null, 401);
100			$response->addHeader('WWW-Authenticate', "Basic realm=\"Environment check\"");
101			// Exception is caught by RequestHandler->handleRequest() and will halt further execution
102			$e = new SS_HTTPResponse_Exception(null, 401);
103			$e->setResponse($response);
104			throw $e;
105			}
106			} else {
107			if(!$this->canAccess(null, $permission)) return $this->httpError(403);
108			}
109			}
110
111			/**
112			* @param null\|int\|Member $member
113			* @param string $permission
114			*
115			* @return bool
116			*
117			* @throws SS_HTTPResponse_Exception
118			*/
119			function canAccess($member = null, $permission = "ADMIN") {
			0 ignored issues – show Best Practice introduced 2015-11-20 23:50 UTC by Report Bug Copy Issue Report Show Similar Issues like this It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
120			if(!$member) {
121			$member = Member::currentUser();
122			}
123
124			if(!$member) {
125			$member = BasicAuth::requireLogin('Environment Checker', $permission, false);
126			}
127
128			// We allow access to this controller regardless of live-status or ADMIN permission only
129			// if on CLI. Access to this controller is always allowed in "dev-mode", or of the user is ADMIN.
130			if(
131			Director::isDev()
132			\|\| Director::is_cli()
133			\|\| empty($permission)
134			\|\| Permission::checkMember($member, $permission)
135			) {
136			return true;
137			}
138
139			// Extended access checks.
140			// "Veto" style, return NULL to abstain vote.
141			$canExtended = null;
142			$results = $this->extend('canAccess', $member);
143			if($results && is_array($results)) {
144			if(!min($results)) return false;
145			else return true;
146			}
147
148			return false;
149			}
150
151			/**
152			* @return SS_HTTPResponse
153			*/
154			function index() {
			0 ignored issues – show Best Practice introduced 2015-11-20 23:50 UTC by Report Bug Copy Issue Report Show Similar Issues like this It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
155			$response = new SS_HTTPResponse;
156			$result = EnvironmentCheckSuite::inst($this->checkSuiteName)->run();
157
158			if(!$result->ShouldPass()) {
159			$response->setStatusCode($this->errorCode);
160			}
161
162			$resultText = $result->customise(array(
163			"URL" => Director::absoluteBaseURL(),
164			"Title" => $this->title,
165			"Name" => $this->checkSuiteName,
166			"ErrorCode" => $this->errorCode,
167			))->renderWith("EnvironmentChecker");
168
169			if ($this->config()->email_results && !$result->ShouldPass()) {
170			$email = new Email($this->config()->from_email_address, $this->config()->to_email_address, $this->title, $resultText);
171			$email->send();
172			}
173
174			// Optionally log errors and warnings individually
175			foreach($result->Details() as $detail) {
176			if($this->config()->log_results_warning && $detail->StatusCode == EnvironmentCheck::WARNING) {
177			$this->log(
178			sprintf('EnvironmentChecker warning at "%s" check. Message: %s', $detail->Check, $detail->Message),
179			$this->config()->log_results_warning_level
180			);
181			} elseif($this->config()->log_results_error && $detail->StatusCode == EnvironmentCheck::ERROR) {
182			$this->log(
183			sprintf('EnvironmentChecker error at "%s" check. Message: %s', $detail->Check, $detail->Message),
184			$this->config()->log_results_error_level
185			);
186			}
187			}
188
189			// output the result as JSON if requested
190			if(
191			$this->getRequest()->getExtension() == 'json'
192			\|\| strpos($this->getRequest()->getHeader('Accept'), 'application/json') !== false
193			) {
194			$response->setBody($result->toJSON());
195			$response->addHeader('Content-Type', 'application/json');
196			return $response;
197			}
198
199			$response->setBody($resultText);
200
201			return $response;
202			}
203
204			/**
205			* @param string $message
206			* @param int $level
207			*/
208			public function log($message, $level) {
209			SS_Log::log($message, $level);
210			}
211
212			/**
213			* Set the HTTP status code that should be returned when there's an error.
214			*
215			* @param int $errorCode
216			*/
217			function setErrorCode($errorCode) {
			0 ignored issues – show Best Practice introduced 2015-11-20 23:50 UTC by Report Bug Copy Issue Report Show Similar Issues like this It is generally recommended to explicitly declare the visibility for methods. Adding explicit visibility (`private`, `protected`, or `public`) is generally recommend to communicate to other developers how, and from where this method is intended to be used. Loading history...
218			$this->errorCode = $errorCode;
219			}
220
221			/**
222			* @deprecated
223			* @param string $from
224			*/
225			public static function set_from_email_address($from) {
226			Deprecation::notice('2.0', 'Use config API instead');
227			Config::inst()->update('EnvironmentChecker', 'from_email_address', $from);
228			}
229
230			/**
231			* @deprecated
232			* @return null\|string
233			*/
234			public static function get_from_email_address() {
235			Deprecation::notice('2.0', 'Use config API instead');
236			return Config::inst()->get('EnvironmentChecker', 'from_email_address');
237			}
238
239			/**
240			* @deprecated
241			* @param string $to
242			*/
243			public static function set_to_email_address($to) {
244			Deprecation::notice('2.0', 'Use config API instead');
245			Config::inst()->update('EnvironmentChecker', 'to_email_address', $to);
246			}
247
248			/**
249			* @deprecated
250			* @return null\|string
251			*/
252			public static function get_to_email_address() {
253			Deprecation::notice('2.0', 'Use config API instead');
254			return Config::inst()->get('EnvironmentChecker', 'to_email_address');
255			}
256
257			/**
258			* @deprecated
259			* @param bool $results
260			*/
261			public static function set_email_results($results) {
262			Deprecation::notice('2.0', 'Use config API instead');
263			Config::inst()->update('EnvironmentChecker', 'email_results', $results);
264			}
265
266			/**
267			* @deprecated
268			* @return bool
269			*/
270			public static function get_email_results() {
271			Deprecation::notice('2.0', 'Use config API instead');
272			return Config::inst()->get('EnvironmentChecker', 'email_results');
273			}
274			}
275

silverstripe / silverstripe-environmentcheck

Push — master ( e090e4...18d79e )

code/EnvironmentChecker.php (7 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like