Issues in Request.php - All Issues - Inspection of "refactoring as recommended by Scrutinizer" - PrivateBin/PrivateBin - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( 81ac23...a5d5f6 )

by El

created 2017-10-08 17:19 UTC

lib/Request.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
/**
 * PrivateBin
 *
 * a zero-knowledge paste bin
 *
 * @link      https://github.com/PrivateBin/PrivateBin
 * @copyright 2012 Sébastien SAUVAGE (sebsauvage.net)
 * @license   https://www.opensource.org/licenses/zlib-license.php The zlib/libpng License
 * @version   1.1.1
 */

namespace PrivateBin;

/**
 * Request
 *
 * parses request parameters and provides helper functions for routing
 */
class Request
{
    /**
     * MIME type for JSON
     *
     * @const string
     */
    const MIME_JSON = 'application/json';

    /**
     * MIME type for HTML
     *
     * @const string
     */
    const MIME_HTML = 'text/html';

    /**
     * MIME type for XHTML
     *
     * @const string
     */
    const MIME_XHTML = 'application/xhtml+xml';

    /**
     * Input stream to use for PUT parameter parsing
     *
     * @access private
     * @var string
     */
    private static $_inputStream = 'php://input';

    /**
     * Operation to perform
     *
     * @access private
     * @var string
     */
    private $_operation = 'view';

    /**
     * Request parameters
     *
     * @access private
     * @var array
     */
    private $_params = array();

    /**
     * If we are in a JSON API context
     *
     * @access private
     * @var bool
     */
    private $_isJsonApi = false;

    /**
     * Constructor
     *
     * @access public
     */
    public function __construct()
// 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);
    }
}
    {
        // decide if we are in JSON API or HTML context
        $this->_isJsonApi = $this->_detectJsonRequest();

        // parse parameters, depending on request type
        switch (array_key_exists('REQUEST_METHOD', $_SERVER) ? $_SERVER['REQUEST_METHOD'] : 'GET') {
            case 'DELETE':
            case 'PUT':
                parse_str(file_get_contents(self::$_inputStream), $this->_params);
                break;
            case 'POST':
                $this->_params = $_POST;
                break;
            default:
                $this->_params = $_GET;
        }
        if (
            !array_key_exists('pasteid', $this->_params) &&
            !array_key_exists('jsonld', $this->_params) &&
            array_key_exists('QUERY_STRING', $_SERVER) &&
            !empty($_SERVER['QUERY_STRING'])
        ) {
            $this->_params['pasteid'] = $_SERVER['QUERY_STRING'];
        }

        // prepare operation, depending on current parameters
        if (
            (array_key_exists('data', $this->_params) && !empty($this->_params['data'])) ||
            (array_key_exists('attachment', $this->_params) && !empty($this->_params['attachment']))
        ) {
            $this->_operation = 'create';
        } elseif (array_key_exists('pasteid', $this->_params) && !empty($this->_params['pasteid'])) {
            if (array_key_exists('deletetoken', $this->_params) && !empty($this->_params['deletetoken'])) {

                $this->_operation = 'delete';
            } else {
                $this->_operation = 'read';
            }
        } elseif (array_key_exists('jsonld', $this->_params) && !empty($this->_params['jsonld'])) {

            $this->_operation = 'jsonld';
        }
    }

    /**
     * Get current operation
     *
     * @access public
     * @return string
     */
    public function getOperation()
    {
        return $this->_operation;
    }

    /**
     * Get a request parameter
     *
     * @access public
     * @param  string $param
     * @param  string $default
     * @return string
     */
    public function getParam($param, $default = '')
    {
        return array_key_exists($param, $this->_params) ? $this->_params[$param] : $default;
    }

    /**
     * If we are in a JSON API context
     *
     * @access public
     * @return bool
     */
    public function isJsonApiCall()
    {
        return $this->_isJsonApi;
    }

    /**
     * Override the default input stream source, used for unit testing
     *
     * @param string $input
     */
    public static function setInputStream($input)
    {
        self::$_inputStream = $input;
    }

    /**
     * Detect the clients supported media type and decide if its a JSON API call or not
     *
     * Adapted from: https://stackoverflow.com/questions/3770513/detect-browser-language-in-php#3771447
     *
     * @access private
     * @return bool
     */
    private function _detectJsonRequest()
// 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);
    }
}
    {
        $hasAcceptHeader = array_key_exists('HTTP_ACCEPT', $_SERVER);
        $acceptHeader    = $hasAcceptHeader ? $_SERVER['HTTP_ACCEPT'] : '';

        // simple cases
        if (
            (array_key_exists('HTTP_X_REQUESTED_WITH', $_SERVER) &&
                $_SERVER['HTTP_X_REQUESTED_WITH'] == 'JSONHttpRequest') ||
            ($hasAcceptHeader &&
                strpos($acceptHeader, self::MIME_JSON) !== false &&
                strpos($acceptHeader, self::MIME_HTML) === false &&
                strpos($acceptHeader, self::MIME_XHTML) === false)
        ) {
            return true;
        }

        // advanced case: media type negotiation
        $mediaTypes = array();
        if ($hasAcceptHeader) {
            $mediaTypeRanges = explode(',', trim($acceptHeader));
            foreach ($mediaTypeRanges as $mediaTypeRange) {

                if (preg_match(
                    '#(\*/\*|[a-z\-]+/[a-z\-+*]+(?:\s*;\s*[^q]\S*)*)(?:\s*;\s*q\s*=\s*(0(?:\.\d{0,3})|1(?:\.0{0,3})))?#',
                    trim($mediaTypeRange), $match
                )) {
                    if (!isset($match[2])) {
                        $match[2] = '1.0';
                    } else {
                        $match[2] = (string) floatval($match[2]);
                    }
                    if (!isset($mediaTypes[$match[2]])) {
                        $mediaTypes[$match[2]] = array();
                    }
                    $mediaTypes[$match[2]][] = strtolower($match[1]);
                }
            }
            krsort($mediaTypes);
            foreach ($mediaTypes as $acceptedQuality => $acceptedValues) {
                if ($acceptedQuality === 0.0) {
                    continue;
                }
                foreach ($acceptedValues as $acceptedValue) {
                    if (
                        strpos($acceptedValue, self::MIME_HTML) === 0 ||
                        strpos($acceptedValue, self::MIME_XHTML) === 0
                    ) {
                        return false;
                    } elseif (strpos($acceptedValue, self::MIME_JSON) === 0) {
                        return true;
                    }
                }
            }
        }
        return false;
    }
}


1			<?php
2			/**
3			* PrivateBin
4			*
5			* a zero-knowledge paste bin
6			*
7			* @link https://github.com/PrivateBin/PrivateBin
8			* @copyright 2012 Sébastien SAUVAGE (sebsauvage.net)
9			* @license https://www.opensource.org/licenses/zlib-license.php The zlib/libpng License
10			* @version 1.1.1
11			*/
12
13			namespace PrivateBin;
14
15			/**
16			* Request
17			*
18			* parses request parameters and provides helper functions for routing
19			*/
20			class Request
21			{
22			/**
23			* MIME type for JSON
24			*
25			* @const string
26			*/
27			const MIME_JSON = 'application/json';
28
29			/**
30			* MIME type for HTML
31			*
32			* @const string
33			*/
34			const MIME_HTML = 'text/html';
35
36			/**
37			* MIME type for XHTML
38			*
39			* @const string
40			*/
41			const MIME_XHTML = 'application/xhtml+xml';
42
43			/**
44			* Input stream to use for PUT parameter parsing
45			*
46			* @access private
47			* @var string
48			*/
49			private static $_inputStream = 'php://input';
50
51			/**
52			* Operation to perform
53			*
54			* @access private
55			* @var string
56			*/
57			private $_operation = 'view';
58
59			/**
60			* Request parameters
61			*
62			* @access private
63			* @var array
64			*/
65			private $_params = array();
66
67			/**
68			* If we are in a JSON API context
69			*
70			* @access private
71			* @var bool
72			*/
73			private $_isJsonApi = false;
74
75			/**
76			* Constructor
77			*
78			* @access public
79			*/
80	107		public function __construct()
			0 ignored issues – show Coding Style introduced 2017-08-10 19:58 UTC by Report Bug Copy Issue Report Show Similar Issues like this `__construct` 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... Coding Style introduced 2017-08-10 19:58 UTC by Report Bug Copy Issue Report Show Similar Issues like this `__construct` uses the super-global variable `$_POST` 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... Coding Style introduced 2017-08-10 19:58 UTC by Report Bug Copy Issue Report Show Similar Issues like this `__construct` uses the super-global variable `$_GET` 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			{
82			// decide if we are in JSON API or HTML context
83	107		$this->_isJsonApi = $this->_detectJsonRequest();
84
85			// parse parameters, depending on request type
86	107		switch (array_key_exists('REQUEST_METHOD', $_SERVER) ? $_SERVER['REQUEST_METHOD'] : 'GET') {
87	107		case 'DELETE':
88	106		case 'PUT':
89	3		parse_str(file_get_contents(self::$_inputStream), $this->_params);
90	3		break;
91	104		case 'POST':
92	50		$this->_params = $_POST;
93	50		break;
94			default:
95	54		$this->_params = $_GET;
96			}
97			if (
98	107		!array_key_exists('pasteid', $this->_params) &&
99	107		!array_key_exists('jsonld', $this->_params) &&
100	107		array_key_exists('QUERY_STRING', $_SERVER) &&
101	107		!empty($_SERVER['QUERY_STRING'])
102			) {
103	34		$this->_params['pasteid'] = $_SERVER['QUERY_STRING'];
104			}
105
106			// prepare operation, depending on current parameters
107			if (
108	107		(array_key_exists('data', $this->_params) && !empty($this->_params['data'])) \|\|
109	107		(array_key_exists('attachment', $this->_params) && !empty($this->_params['attachment']))
110			) {
111	46		$this->_operation = 'create';
112	61		} elseif (array_key_exists('pasteid', $this->_params) && !empty($this->_params['pasteid'])) {
113	47	View Code Duplication	if (array_key_exists('deletetoken', $this->_params) && !empty($this->_params['deletetoken'])) {
			0 ignored issues – show Duplication introduced 2016-07-09 10:42 UTC by Report Bug Copy Issue Report Show Similar Issues like this This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
114	20		$this->_operation = 'delete';
115			} else {
116	47		$this->_operation = 'read';
117			}
118	14	View Code Duplication	} elseif (array_key_exists('jsonld', $this->_params) && !empty($this->_params['jsonld'])) {
			0 ignored issues – show Duplication introduced 2016-07-09 10:42 UTC by Report Bug Copy Issue Report Show Similar Issues like this This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
119	5		$this->_operation = 'jsonld';
120			}
121	107		}
122
123			/**
124			* Get current operation
125			*
126			* @access public
127			* @return string
128			*/
129	107		public function getOperation()
130			{
131	107		return $this->_operation;
132			}
133
134			/**
135			* Get a request parameter
136			*
137			* @access public
138			* @param string $param
139			* @param string $default
140			* @return string
141			*/
142	98		public function getParam($param, $default = '')
143			{
144	98		return array_key_exists($param, $this->_params) ? $this->_params[$param] : $default;
145			}
146
147			/**
148			* If we are in a JSON API context
149			*
150			* @access public
151			* @return bool
152			*/
153	102		public function isJsonApiCall()
154			{
155	102		return $this->_isJsonApi;
156			}
157
158			/**
159			* Override the default input stream source, used for unit testing
160			*
161			* @param string $input
162			*/
163	3		public static function setInputStream($input)
164			{
165	3		self::$_inputStream = $input;
166	3		}
167
168			/**
169			* Detect the clients supported media type and decide if its a JSON API call or not
170			*
171			* Adapted from: https://stackoverflow.com/questions/3770513/detect-browser-language-in-php#3771447
172			*
173			* @access private
174			* @return bool
175			*/
176	107		private function _detectJsonRequest()
			0 ignored issues – show Coding Style introduced 2017-08-10 19:58 UTC by Report Bug Copy Issue Report Show Similar Issues like this `_detectJsonRequest` 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...
177			{
178	107		$hasAcceptHeader = array_key_exists('HTTP_ACCEPT', $_SERVER);
179	107		$acceptHeader = $hasAcceptHeader ? $_SERVER['HTTP_ACCEPT'] : '';
180
181			// simple cases
182			if (
183	107		(array_key_exists('HTTP_X_REQUESTED_WITH', $_SERVER) &&
184	57		$_SERVER['HTTP_X_REQUESTED_WITH'] == 'JSONHttpRequest') \|\|
185	50		($hasAcceptHeader &&
186	50		strpos($acceptHeader, self::MIME_JSON) !== false &&
187	50		strpos($acceptHeader, self::MIME_HTML) === false &&
188	107		strpos($acceptHeader, self::MIME_XHTML) === false)
189			) {
190	59		return true;
191			}
192
193			// advanced case: media type negotiation
194	48		$mediaTypes = array();
195	48		if ($hasAcceptHeader) {
196	4		$mediaTypeRanges = explode(',', trim($acceptHeader));
197	4	View Code Duplication	foreach ($mediaTypeRanges as $mediaTypeRange) {
			0 ignored issues – show Duplication introduced 2016-07-09 10:42 UTC by Report Bug Copy Issue Report Show Similar Issues like this This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
198	4		if (preg_match(
199	4		'#(\/\\|[a-z\-]+/[a-z\-+]+(?:\s;\s[^q]\S))(?:\s;\sq\s=\s*(0(?:\.\d{0,3})\|1(?:\.0{0,3})))?#',
200	4		trim($mediaTypeRange), $match
201			)) {
202	4		if (!isset($match[2])) {
203	4		$match[2] = '1.0';
204			} else {
205	4		$match[2] = (string) floatval($match[2]);
206			}
207	4		if (!isset($mediaTypes[$match[2]])) {
208	4		$mediaTypes[$match[2]] = array();
209			}
210	4		$mediaTypes[$match[2]][] = strtolower($match[1]);
211			}
212			}
213	4		krsort($mediaTypes);
214	4		foreach ($mediaTypes as $acceptedQuality => $acceptedValues) {
215	4		if ($acceptedQuality === 0.0) {
216			continue;
217			}
218	4		foreach ($acceptedValues as $acceptedValue) {
219			if (
220	4		strpos($acceptedValue, self::MIME_HTML) === 0 \|\|
221	4		strpos($acceptedValue, self::MIME_XHTML) === 0
222			) {
223	2		return false;
224	2		} elseif (strpos($acceptedValue, self::MIME_JSON) === 0) {
225	2		return true;
226			}
227			}
228			}
229			}
230	45		return false;
231			}
232			}
233

PrivateBin / PrivateBin

Push — master ( 81ac23...a5d5f6 )

lib/Request.php (7 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like