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)

Labels

Coding Style 3

Severity

Minor 3

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)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like