logHandler::getUserIP() - Code Metrics - Inspection of "write manager IP address to manager_log" - evolution-cms/evolution - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — develop ( 85bfee...36dfe9 )

by Dmytro

created 2018-03-25 21:39 UTC

logHandler::getUserIP() A

↳ Parent: Project

Complexity

Conditions	4
Paths	3

Size

Total Lines	13
Code Lines	9

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
cc	4
eloc	9
nc	3
nop	0
dl	0
loc	13
rs	9.2
c	0
b	0
f	0

<?php

/**
 * logger class.
 *
 * Usage:
 *
 * include_once "log.class.inc.php"; // include_once the class
 * $log = new logHandler;  // create the object
 * $log->initAndWriteLog($msg); // write $msg to log, and populate all other fields as best as possible
 * $log->initAndWriteLog($msg, $internalKey, $username, $action, $id, $itemname); // write $msg and other data to log
 */
class logHandler
namespace YourVendor;

class YourClass { }
{
    /**
     * Single variable for a log entry
     *
     * @var array
     */
    public $entry = array();

    /**
     * @param string $msg
     * @param string $internalKey
     * @param string $username
     * @param string $action
     * @param string $itemid
     * @param string $itemname
     *
     * @return void
     */
    public function initAndWriteLog(
// 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);
    }
}
        $msg = "",
        $internalKey = "",
        $username = "",
        $action = "",
        $itemid = "",
        $itemname = ""
    ) {
        global $modx;
        $this->entry['msg'] = $msg; // writes testmessage to the object
        $this->entry['action'] = empty($action) ? $modx->manager->action : $action;    // writes the action to the object

        // User Credentials
        $this->entry['internalKey'] = $internalKey == "" ? $modx->getLoginUserID() : $internalKey;
        $this->entry['username'] = $username == "" ? $modx->getLoginUserName() : $username;

        $this->entry['itemId'] = (empty($itemid) && isset($_REQUEST['id'])) ? (int)$_REQUEST['id'] : $itemid;  // writes the id to the object
        if ($this->entry['itemId'] == 0) {
            $this->entry['itemId'] = "-";
        } // to stop items having id 0

        $this->entry['itemName'] = ($itemname == "" && isset($_SESSION['itemname'])) ? $_SESSION['itemname'] : $itemname; // writes the id to the object
        if ($this->entry['itemName'] == "") {
            $this->entry['itemName'] = "-";
        } // to stop item name being empty

        $this->writeToLog();

        return;
    }

    /**
     * function to write to the log collects all required info, and writes it to the logging table
     *
     * @return void
     */
    public function writeToLog()
    {
        global $modx;
        $tbl_manager_log = $modx->getFullTableName('manager_log');

        if ($this->entry['internalKey'] == "") {
            $modx->webAlertAndQuit("Logging error: internalKey not set.");
        }
        if (empty($this->entry['action'])) {
            $modx->webAlertAndQuit("Logging error: action not set.");
        }
        if ($this->entry['msg'] == "") {
            include_once "actionlist.inc.php";
            $this->entry['msg'] = getAction($this->entry['action'], $this->entry['itemId']);
            if ($this->entry['msg'] == "") {
                $modx->webAlertAndQuit("Logging error: couldn't find message to write to log.");
            }
        }

        $fields['timestamp'] = time();
foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}
        $fields['internalKey'] = $modx->db->escape($this->entry['internalKey']);
        $fields['username'] = $modx->db->escape($this->entry['username']);
        $fields['action'] = $this->entry['action'];
        $fields['itemid'] = $this->entry['itemId'];
        $fields['itemname'] = $modx->db->escape($this->entry['itemName']);
        $fields['message'] = $modx->db->escape($this->entry['msg']);
        $fields['ip'] = $this->getUserIP();

        $insert_id = $modx->db->insert($fields, $tbl_manager_log);
        if (!$insert_id) {
            $modx->messageQuit("Logging error: couldn't save log to table! Error code: " . $modx->db->getLastError());
        } else {
            $limit = (isset($modx->config['manager_log_limit'])) ? (int)$modx->config['manager_log_limit'] : 3000;
            $trim = (isset($modx->config['manager_log_trim'])) ? (int)$modx->config['manager_log_trim'] : 100;
            if (($insert_id % $trim) === 0) {
                $modx->rotate_log('manager_log', $limit, $trim);
            }
        }
    }

    private function getUserIP() {
// 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( array_key_exists('HTTP_X_FORWARDED_FOR', $_SERVER) && !empty($_SERVER['HTTP_X_FORWARDED_FOR']) ) {
            if (strpos($_SERVER['HTTP_X_FORWARDED_FOR'], ',')>0) {
                $addr = explode(",",$_SERVER['HTTP_X_FORWARDED_FOR']);
                return trim($addr[0]);
            } else {
                return $_SERVER['HTTP_X_FORWARDED_FOR'];
            }
        }
        else {
            return $_SERVER['REMOTE_ADDR'];
        }
    }
}


1			<?php
2
3			/**
4			* logger class.
5			*
6			* Usage:
7			*
8			* include_once "log.class.inc.php"; // include_once the class
9			* $log = new logHandler; // create the object
10			* $log->initAndWriteLog($msg); // write $msg to log, and populate all other fields as best as possible
11			* $log->initAndWriteLog($msg, $internalKey, $username, $action, $id, $itemname); // write $msg and other data to log
12			*/
13			class logHandler
			0 ignored issues – show Coding Style Compatibility introduced 2018-02-25 17:05 UTC by Report Bug Copy Issue Report PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions. You can fix this by adding a namespace to your class: namespace YourVendor; class YourClass { } When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries. Loading history... Coding Style introduced 2018-02-25 17:05 UTC by Report Bug Copy Issue Report This class is not in CamelCase format. Classes in PHP are usually named in CamelCase. In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. The whole name starts with a capital letter as well. Thus the name database provider becomes `DatabaseProvider`. Loading history...
14			{
15			/**
16			* Single variable for a log entry
17			*
18			* @var array
19			*/
20			public $entry = array();
21
22			/**
23			* @param string $msg
24			* @param string $internalKey
25			* @param string $username
26			* @param string $action
27			* @param string $itemid
28			* @param string $itemname
29			*
30			* @return void
31			*/
32			public function initAndWriteLog(
			0 ignored issues – show Coding Style introduced 2018-02-25 17:05 UTC by Report Bug Copy Issue Report `initAndWriteLog` uses the super-global variable `$_REQUEST` 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 2018-02-25 17:05 UTC by Report Bug Copy Issue Report `initAndWriteLog` uses the super-global variable `$_SESSION` 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...
33			$msg = "",
34			$internalKey = "",
35			$username = "",
36			$action = "",
37			$itemid = "",
38			$itemname = ""
39			) {
40			global $modx;
41			$this->entry['msg'] = $msg; // writes testmessage to the object
42			$this->entry['action'] = empty($action) ? $modx->manager->action : $action; // writes the action to the object
43
44			// User Credentials
45			$this->entry['internalKey'] = $internalKey == "" ? $modx->getLoginUserID() : $internalKey;
46			$this->entry['username'] = $username == "" ? $modx->getLoginUserName() : $username;
47
48			$this->entry['itemId'] = (empty($itemid) && isset($_REQUEST['id'])) ? (int)$_REQUEST['id'] : $itemid; // writes the id to the object
49			if ($this->entry['itemId'] == 0) {
50			$this->entry['itemId'] = "-";
51			} // to stop items having id 0
52
53			$this->entry['itemName'] = ($itemname == "" && isset($_SESSION['itemname'])) ? $_SESSION['itemname'] : $itemname; // writes the id to the object
54			if ($this->entry['itemName'] == "") {
55			$this->entry['itemName'] = "-";
56			} // to stop item name being empty
57
58			$this->writeToLog();
59
60			return;
61			}
62
63			/**
64			* function to write to the log collects all required info, and writes it to the logging table
65			*
66			* @return void
67			*/
68			public function writeToLog()
69			{
70			global $modx;
71			$tbl_manager_log = $modx->getFullTableName('manager_log');
72
73			if ($this->entry['internalKey'] == "") {
74			$modx->webAlertAndQuit("Logging error: internalKey not set.");
75			}
76			if (empty($this->entry['action'])) {
77			$modx->webAlertAndQuit("Logging error: action not set.");
78			}
79			if ($this->entry['msg'] == "") {
80			include_once "actionlist.inc.php";
81			$this->entry['msg'] = getAction($this->entry['action'], $this->entry['itemId']);
82			if ($this->entry['msg'] == "") {
83			$modx->webAlertAndQuit("Logging error: couldn't find message to write to log.");
84			}
85			}
86
87			$fields['timestamp'] = time();
			0 ignored issues – show Coding Style Comprehensibility introduced 2018-02-21 15:53 UTC by Report Bug Copy Issue Report `$fields` was never initialized. Although not strictly required by PHP, it is generally a good practice to add `$fields = array();` before regardless. Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code. Let’s take a look at an example: foreach ($collection as $item) { $myArray['foo'] = $item->getFoo(); if ($item->hasBar()) { $myArray['bar'] = $item->getBar(); } // do something with $myArray } As you can see in this example, the array `$myArray` is initialized the first time when the foreach loop is entered. You can also see that the value of the `bar` key is only written conditionally; thus, its value might result from a previous iteration. This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization `$myArray = array()` either outside or inside the foreach loop. Loading history...
88			$fields['internalKey'] = $modx->db->escape($this->entry['internalKey']);
89			$fields['username'] = $modx->db->escape($this->entry['username']);
90			$fields['action'] = $this->entry['action'];
91			$fields['itemid'] = $this->entry['itemId'];
92			$fields['itemname'] = $modx->db->escape($this->entry['itemName']);
93			$fields['message'] = $modx->db->escape($this->entry['msg']);
94			$fields['ip'] = $this->getUserIP();
95
96			$insert_id = $modx->db->insert($fields, $tbl_manager_log);
97			if (!$insert_id) {
98			$modx->messageQuit("Logging error: couldn't save log to table! Error code: " . $modx->db->getLastError());
99			} else {
100			$limit = (isset($modx->config['manager_log_limit'])) ? (int)$modx->config['manager_log_limit'] : 3000;
101			$trim = (isset($modx->config['manager_log_trim'])) ? (int)$modx->config['manager_log_trim'] : 100;
102			if (($insert_id % $trim) === 0) {
103			$modx->rotate_log('manager_log', $limit, $trim);
104			}
105			}
106			}
107
108			private function getUserIP() {
			0 ignored issues – show Documentation introduced 2018-03-25 21:45 UTC by Report Bug Copy Issue Report The return type could not be reliably inferred; please add a `@return` annotation. Our type inference engine in quite powerful, but sometimes the code does not provide enough clues to go by. In these cases we request you to add a `@return` annotation as described here. Loading history... Coding Style introduced 2018-03-25 21:45 UTC by Report Bug Copy Issue Report `getUserIP` 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...
109			if( array_key_exists('HTTP_X_FORWARDED_FOR', $_SERVER) && !empty($_SERVER['HTTP_X_FORWARDED_FOR']) ) {
110			if (strpos($_SERVER['HTTP_X_FORWARDED_FOR'], ',')>0) {
111			$addr = explode(",",$_SERVER['HTTP_X_FORWARDED_FOR']);
112			return trim($addr[0]);
113			} else {
114			return $_SERVER['HTTP_X_FORWARDED_FOR'];
115			}
116			}
117			else {
118			return $_SERVER['REMOTE_ADDR'];
119			}
120			}
121			}
122

evolution-cms / evolution

Push — develop ( 85bfee...36dfe9 )

logHandler::getUserIP() A

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like