Issues in TrafficLimiter.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/Persistence/TrafficLimiter.php (3 issues)

Showing only issues like (Show All)

super-globals are not used.

Coding Style Minor

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\Persistence;

use PrivateBin\Configuration;

/**
 * TrafficLimiter
 *
 * Handles traffic limiting, so no user does more than one call per 10 seconds.
 */
class TrafficLimiter extends AbstractPersistence
{
    /**
     * time limit in seconds, defaults to 10s
     *
     * @access private
     * @static
     * @var    int
     */
    private static $_limit = 10;

    /**
     * key to fetch IP address
     *
     * @access private
     * @static
     * @var    string
     */
    private static $_ipKey = 'REMOTE_ADDR';

    /**
     * set the time limit in seconds
     *
     * @access public
     * @static
     * @param  int $limit
     */
    public static function setLimit($limit)
    {
        self::$_limit = $limit;
    }

    /**
     * set configuration options of the traffic limiter
     *
     * @access public
     * @static
     * @param Configuration $conf
     */
    public static function setConfiguration(Configuration $conf)
// 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);
    }
}
    {
        self::setLimit($conf->getKey('limit', 'traffic'));
        self::setPath($conf->getKey('dir', 'traffic'));
        if (($option = $conf->getKey('header', 'traffic')) !== null) {
            $httpHeader = 'HTTP_' . $option;
            if (array_key_exists($httpHeader, $_SERVER) && !empty($_SERVER[$httpHeader])) {
                self::$_ipKey = $httpHeader;
            }
        }
    }

    /**
     * get a HMAC of the current visitors IP address
     *
     * @access public
     * @static
     * @param  string $algo
     * @return string
     */
    public static function getHash($algo = 'sha512')
// 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);
    }
}
    {
        return hash_hmac($algo, $_SERVER[self::$_ipKey], ServerSalt::get());
    }

    /**
     * traffic limiter
     *
     * Make sure the IP address makes at most 1 request every 10 seconds.
     *
     * @access public
     * @static
     * @throws Exception
     * @return bool
     */
    public static function canPass()
// 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);
    }
}
    {
        // disable limits if set to less then 1
        if (self::$_limit < 1) {
            return true;
        }

        $file = 'traffic_limiter.php';
        if (!self::_exists($file)) {
            self::_store(
                $file,
                '<?php' . PHP_EOL .
                '$GLOBALS[\'traffic_limiter\'] = array();' . PHP_EOL
            );
        }

        $path = self::getPath($file);
        require $path;
        $now = time();
        $tl  = $GLOBALS['traffic_limiter'];

        // purge file of expired hashes to keep it small
        foreach ($tl as $key => $time) {
            if ($time + self::$_limit < $now) {
                unset($tl[$key]);
            }
        }

        // this hash is used as an array key, hence a shorter hash is used
        $hash = self::getHash('sha256');
        if (array_key_exists($hash, $tl) && ($tl[$hash] + self::$_limit >= $now)) {
            $result = false;
        } else {
            $tl[$hash] = time();
            $result    = true;
        }
        self::_store(
            $file,
            '<?php' . PHP_EOL .
            '$GLOBALS[\'traffic_limiter\'] = ' .
            var_export($tl, true) . ';' . PHP_EOL
        );
        return $result;
    }
}


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\Persistence;
14
15		use PrivateBin\Configuration;
16
17		/**
18		* TrafficLimiter
19		*
20		* Handles traffic limiting, so no user does more than one call per 10 seconds.
21		*/
22		class TrafficLimiter extends AbstractPersistence
23		{
24		/**
25		* time limit in seconds, defaults to 10s
26		*
27		* @access private
28		* @static
29		* @var int
30		*/
31		private static $_limit = 10;
32
33		/**
34		* key to fetch IP address
35		*
36		* @access private
37		* @static
38		* @var string
39		*/
40		private static $_ipKey = 'REMOTE_ADDR';
41
42		/**
43		* set the time limit in seconds
44		*
45		* @access public
46		* @static
47		* @param int $limit
48		*/
49	45	public static function setLimit($limit)
50		{
51	45	self::$_limit = $limit;
52	45	}
53
54		/**
55		* set configuration options of the traffic limiter
56		*
57		* @access public
58		* @static
59		* @param Configuration $conf
60		*/
61	44	public static function setConfiguration(Configuration $conf)
		0 ignored issues – show Coding Style introduced 2016-07-09 10:42 UTC by Report Bug Copy Issue Report Show Similar Issues like this `setConfiguration` 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...
62		{
63	44	self::setLimit($conf->getKey('limit', 'traffic'));
64	44	self::setPath($conf->getKey('dir', 'traffic'));
65	44	if (($option = $conf->getKey('header', 'traffic')) !== null) {
66	2	$httpHeader = 'HTTP_' . $option;
67	2	if (array_key_exists($httpHeader, $_SERVER) && !empty($_SERVER[$httpHeader])) {
68	2	self::$_ipKey = $httpHeader;
69		}
70		}
71	44	}
72
73		/**
74		* get a HMAC of the current visitors IP address
75		*
76		* @access public
77		* @static
78		* @param string $algo
79		* @return string
80		*/
81	20	public static function getHash($algo = 'sha512')
		0 ignored issues – show Coding Style introduced 2016-08-10 16:12 UTC by Report Bug Copy Issue Report Show Similar Issues like this `getHash` 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...
82		{
83	20	return hash_hmac($algo, $_SERVER[self::$_ipKey], ServerSalt::get());
84		}
85
86		/**
87		* traffic limiter
88		*
89		* Make sure the IP address makes at most 1 request every 10 seconds.
90		*
91		* @access public
92		* @static
93		* @throws Exception
94		* @return bool
95		*/
96	45	public static function canPass()
		0 ignored issues – show Coding Style introduced 2016-07-09 10:42 UTC by Report Bug Copy Issue Report Show Similar Issues like this `canPass` uses the super-global variable `$GLOBALS` 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...
97		{
98		// disable limits if set to less then 1
99	45	if (self::$_limit < 1) {
100	38	return true;
101		}
102
103	9	$file = 'traffic_limiter.php';
104	9	if (!self::_exists($file)) {
105	9	self::_store(
106	9	$file,
107	9	'<?php' . PHP_EOL .
108	9	'$GLOBALS[\'traffic_limiter\'] = array();' . PHP_EOL
109		);
110		}
111
112	9	$path = self::getPath($file);
113	9	require $path;
114	9	$now = time();
115	9	$tl = $GLOBALS['traffic_limiter'];
116
117		// purge file of expired hashes to keep it small
118	9	foreach ($tl as $key => $time) {
119	3	if ($time + self::$_limit < $now) {
120	3	unset($tl[$key]);
121		}
122		}
123
124		// this hash is used as an array key, hence a shorter hash is used
125	9	$hash = self::getHash('sha256');
126	9	if (array_key_exists($hash, $tl) && ($tl[$hash] + self::$_limit >= $now)) {
127	3	$result = false;
128		} else {
129	9	$tl[$hash] = time();
130	9	$result = true;
131		}
132	9	self::_store(
133	9	$file,
134	9	'<?php' . PHP_EOL .
135	9	'$GLOBALS[\'traffic_limiter\'] = ' .
136	9	var_export($tl, true) . ';' . PHP_EOL
137		);
138	9	return $result;
139		}
140		}
141

PrivateBin / PrivateBin

Push — master ( 81ac23...a5d5f6 )

lib/Persistence/TrafficLimiter.php (3 issues)

Showing only issues like (Show All)

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like