XoopsModules25x /
smartpartner
This project does not seem to handle request data directly as such no vulnerable execution paths were found.
include, or for example
via PHP's auto-loading mechanism.
These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more
| 1 | <?php |
||
|
0 ignored issues
–
show
|
|||
| 2 | // |
||
| 3 | // ------------------------------------------------------------------------ // |
||
| 4 | // XOOPS - PHP Content Management System // |
||
| 5 | // Copyright (c) 2000-2016 XOOPS.org // |
||
| 6 | // <http://xoops.org/> // |
||
| 7 | // ------------------------------------------------------------------------ // |
||
| 8 | // defined('XOOPS_ROOT_PATH') || exit('XOOPS root path not defined'); |
||
| 9 | include_once XOOPS_ROOT_PATH . '/modules/smartpartner/include/common.php'; |
||
| 10 | require_once(SMARTPARTNER_ROOT_PATH . 'class/baseObjectHandler.php'); |
||
| 11 | |||
| 12 | /** |
||
| 13 | * smartpartnerMimetype class |
||
| 14 | * |
||
| 15 | * Information about an individual mimetype |
||
| 16 | * |
||
| 17 | * <code> |
||
| 18 | * $hMime = xoops_getModuleHandler('mimetype', 'smartpartner'); |
||
| 19 | * $mimetype =& $hMime->get(1); |
||
| 20 | * $mime_id = $mimetype->getVar('id'); |
||
| 21 | * </code> |
||
| 22 | * |
||
| 23 | * @author Eric Juden <[email protected]> |
||
| 24 | * @access public |
||
| 25 | * @package smartpartner |
||
| 26 | */ |
||
| 27 | class smartpartnerMimetype extends XoopsObject |
||
|
0 ignored issues
–
show
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...
|
|||
| 28 | { |
||
| 29 | /** |
||
| 30 | * smartpartnerMimetype constructor. |
||
| 31 | * @param null $id |
||
| 32 | */ |
||
| 33 | public function __construct($id = null) |
||
| 34 | { |
||
| 35 | $this->initVar('mime_id', XOBJ_DTYPE_INT, null, false); |
||
| 36 | $this->initVar('mime_ext', XOBJ_DTYPE_TXTBOX, null, true, 60); |
||
| 37 | $this->initVar('mime_types', XOBJ_DTYPE_TXTAREA, null, false, 1024); |
||
| 38 | $this->initVar('mime_name', XOBJ_DTYPE_TXTBOX, null, true, 255); |
||
| 39 | $this->initVar('mime_admin', XOBJ_DTYPE_INT, null, false); |
||
| 40 | $this->initVar('mime_user', XOBJ_DTYPE_INT, null, false); |
||
| 41 | |||
| 42 | if (isset($id)) { |
||
| 43 | if (is_array($id)) { |
||
| 44 | $this->assignVars($id); |
||
| 45 | } |
||
| 46 | } else { |
||
| 47 | $this->setNew(); |
||
| 48 | } |
||
| 49 | } |
||
| 50 | } // end of class |
||
| 51 | |||
| 52 | /** |
||
| 53 | * Class smartpartnerMimetypeHandler |
||
| 54 | */ |
||
| 55 | class smartpartnerMimetypeHandler extends SmartpartnerBaseObjectHandler |
||
|
0 ignored issues
–
show
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...
|
|||
| 56 | { |
||
| 57 | /** |
||
| 58 | * Name of child class |
||
| 59 | * |
||
| 60 | * @var string |
||
| 61 | * @access private |
||
| 62 | */ |
||
| 63 | public $classname = 'smartpartnermimetype'; |
||
| 64 | |||
| 65 | /** |
||
| 66 | * DB Table Name |
||
| 67 | * |
||
| 68 | * @var string |
||
| 69 | * @access private |
||
| 70 | */ |
||
| 71 | public $_dbtable = 'smartpartner_mimetypes'; |
||
| 72 | |||
| 73 | /** |
||
| 74 | * Constructor |
||
| 75 | * |
||
| 76 | * @param object|XoopsDatabase $db reference to a xoopsDB object |
||
| 77 | */ |
||
| 78 | public function __construct(XoopsDatabase $db) |
||
| 79 | { |
||
| 80 | parent::init($db); |
||
|
0 ignored issues
–
show
It seems like you call parent on a different method (
init() instead of __construct()). Are you sure this is correct? If so, you might want to change this to $this->init().
This check looks for a call to a parent method whose name is different than the method from which it is called. Consider the following code: class Daddy
{
protected function getFirstName()
{
return "Eidur";
}
protected function getSurName()
{
return "Gudjohnsen";
}
}
class Son
{
public function getFirstName()
{
return parent::getSurname();
}
}
The Loading history...
|
|||
| 81 | } |
||
| 82 | |||
| 83 | /** |
||
| 84 | * retrieve a mimetype object from the database |
||
| 85 | * @param int $id ID of mimetype |
||
| 86 | * @return object {@link smartpartnerMimetype} |
||
|
0 ignored issues
–
show
|
|||
| 87 | * @access public |
||
| 88 | */ |
||
| 89 | View Code Duplication | public function &get($id) |
|
|
0 ignored issues
–
show
This method seems to be duplicated in 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...
|
|||
| 90 | { |
||
| 91 | $id = (int)$id; |
||
| 92 | if ($id > 0) { |
||
| 93 | $sql = $this->_selectQuery(new Criteria('mime_id', $id)); |
||
| 94 | if (!$result = $this->_db->query($sql)) { |
||
| 95 | return false; |
||
| 96 | } |
||
| 97 | $numrows = $this->_db->getRowsNum($result); |
||
| 98 | if ($numrows == 1) { |
||
| 99 | $obj = new $this->classname($this->_db->fetchArray($result)); |
||
| 100 | |||
| 101 | return $obj; |
||
| 102 | } |
||
| 103 | } |
||
| 104 | |||
| 105 | return false; |
||
| 106 | } |
||
| 107 | |||
| 108 | /** |
||
| 109 | * retrieve objects from the database |
||
| 110 | * |
||
| 111 | * @param object $criteria {@link CriteriaElement} conditions to be met |
||
|
0 ignored issues
–
show
Should the type for parameter
$criteria not be object|null?
This check looks for It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. Loading history...
|
|||
| 112 | * @return array array of {@link smartpartnerMimetype} objects |
||
| 113 | * @access public |
||
| 114 | */ |
||
| 115 | public function &getObjects($criteria = null) |
||
| 116 | { |
||
| 117 | $ret = array(); |
||
| 118 | $limit = $start = 0; |
||
| 119 | $sql = $this->_selectQuery($criteria); |
||
| 120 | if (isset($criteria)) { |
||
| 121 | $limit = $criteria->getLimit(); |
||
| 122 | $start = $criteria->getStart(); |
||
| 123 | } |
||
| 124 | //echo "<br>$sql<br>";exit; |
||
|
0 ignored issues
–
show
Unused Code
Comprehensibility
introduced
by
84% of this comment could be valid code. Did you maybe forget this after debugging?
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it. The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production. This check looks for comments that seem to be mostly valid code and reports them. Loading history...
|
|||
| 125 | $result = $this->_db->query($sql, $limit, $start); |
||
| 126 | // if no records from db, return empty array |
||
| 127 | if (!$result) { |
||
| 128 | return $ret; |
||
| 129 | } |
||
| 130 | |||
| 131 | // Add each returned record to the result array |
||
| 132 | while ($myrow = $this->_db->fetchArray($result)) { |
||
| 133 | $obj = new $this->classname($myrow); |
||
| 134 | $ret[] =& $obj; |
||
| 135 | unset($obj); |
||
| 136 | } |
||
| 137 | |||
| 138 | return $ret; |
||
| 139 | } |
||
| 140 | |||
| 141 | /** |
||
| 142 | * Format mime_types into array |
||
| 143 | * |
||
| 144 | * @param null $mime_ext |
||
| 145 | * @return array array of mime_types |
||
| 146 | * @access public |
||
| 147 | */ |
||
| 148 | public function getArray($mime_ext = null) |
||
| 149 | { |
||
| 150 | /*global $smartPartnerIsAdmin, $xoopsUser, $xoopsModule; |
||
|
0 ignored issues
–
show
Unused Code
Comprehensibility
introduced
by
56% of this comment could be valid code. Did you maybe forget this after debugging?
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it. The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production. This check looks for comments that seem to be mostly valid code and reports them. Loading history...
|
|||
| 151 | |||
| 152 | $ret = array(); |
||
| 153 | if ($xoopsUser && !$smartPartnerIsAdmin) { |
||
| 154 | // For user uploading |
||
| 155 | $crit = new CriteriaCompo(new Criteria('mime_user', 1)); //$sql = sprintf("SELECT * FROM %s WHERE mime_user=1", $xoopsDB->prefix('smartpartner_mimetypes')); |
||
| 156 | } elseif ($xoopsUser && $smartPartnerIsAdmin) { |
||
| 157 | // For admin uploading |
||
| 158 | $crit = new CriteriaCompo(new Criteria('mime_admin', 1)); //$sql = sprintf("SELECT * FROM %s WHERE mime_admin=1", $xoopsDB->prefix('smartpartner_mimetypes')); |
||
| 159 | } else { |
||
| 160 | return $ret; |
||
| 161 | } |
||
| 162 | if ($mime_ext) { |
||
| 163 | $crit->add(new Criteria('mime_ext', $mime_ext)); |
||
| 164 | } |
||
| 165 | $result = $this->getObjects($crit); |
||
| 166 | |||
| 167 | // if no records from db, return empty array |
||
| 168 | if (!$result) { |
||
| 169 | return $ret; |
||
| 170 | } |
||
| 171 | |||
| 172 | foreach ($result as $mime) { |
||
| 173 | $line = explode(" ", $mime->getVar('mime_types')); |
||
| 174 | foreach ($line as $row) { |
||
| 175 | $allowed_mimetypes[] = array('type'=>$row, 'ext'=>$mime->getVar('mime_ext')); |
||
| 176 | } |
||
| 177 | }*/ |
||
| 178 | global $xoopsModuleConfig; |
||
|
0 ignored issues
–
show
Compatibility
Best Practice
introduced
by
Use of
global functionality is not recommended; it makes your code harder to test, and less reusable.
Instead of relying on 1. Pass all data via parametersfunction myFunction($a, $b) {
// Do something
}
2. Create a class that maintains your stateclass MyClass {
private $a;
private $b;
public function __construct($a, $b) {
$this->a = $a;
$this->b = $b;
}
public function myFunction() {
// Do something
}
}
Loading history...
|
|||
| 179 | $mymetypesArray = include_once(SMARTPARTNER_ROOT_PATH . '/include/mimetypes.inc.php'); |
||
| 180 | foreach (explode('|', $xoopsModuleConfig['allowed_ext']) as $ext) { |
||
| 181 | $allowed_mimetypes[] = array('type' => $mymetypesArray[$ext], 'ext' => $ext); |
||
|
0 ignored issues
–
show
Coding Style
Comprehensibility
introduced
by
$allowed_mimetypes was never initialized. Although not strictly required by PHP, it is generally a good practice to add $allowed_mimetypes = 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 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...
|
|||
| 182 | } |
||
| 183 | |||
| 184 | return $allowed_mimetypes; |
||
|
0 ignored issues
–
show
The variable
$allowed_mimetypes does not seem to be defined for all execution paths leading up to this point.
If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) {
switch ($a) {
case 'foo':
$x = 1;
break;
case 'bar':
$x = 2;
break;
}
// $x is potentially undefined here.
echo $x;
}
In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined. Available Fixes
Loading history...
|
|||
| 185 | } |
||
| 186 | |||
| 187 | /** |
||
| 188 | * Checks to see if the user uploading the file has permissions to upload this mimetype |
||
| 189 | * @param $post_field file being uploaded |
||
| 190 | * @return false if no permission, return mimetype if has permission |
||
|
0 ignored issues
–
show
|
|||
| 191 | * @access public |
||
| 192 | */ |
||
| 193 | public function checkMimeTypes($post_field) |
||
|
0 ignored issues
–
show
checkMimeTypes uses the super-global variable $_FILES 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...
|
|||
| 194 | { |
||
| 195 | $fname = $_FILES[$post_field]['name']; |
||
| 196 | $farray = explode('.', $fname); |
||
| 197 | $fextension = strtolower($farray[count($farray) - 1]); |
||
|
0 ignored issues
–
show
$fextension is not used, you could remove the assignment.
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently. $myVar = 'Value';
$higher = false;
if (rand(1, 6) > 3) {
$higher = true;
} else {
$higher = false;
}
Both the Loading history...
|
|||
| 198 | |||
| 199 | $allowed_mimetypes = $this->getArray(); |
||
| 200 | if (empty($allowed_mimetypes)) { |
||
| 201 | return false; |
||
| 202 | } |
||
| 203 | foreach ($allowed_mimetypes as $mime) { |
||
|
0 ignored issues
–
show
The expression
$allowed_mimetypes of type array<integer,array<stri...:"?","ext":"?"}>>|false is not guaranteed to be traversable. How about adding an additional type check?
There are different options of fixing this problem.
Loading history...
|
|||
| 204 | if ($mime['type'] == $_FILES[$post_field]['type']) { |
||
| 205 | $allowed_mimetypes = $mime['type']; |
||
| 206 | break; |
||
| 207 | } else { |
||
| 208 | $allowed_mimetypes = false; |
||
| 209 | } |
||
| 210 | } |
||
| 211 | |||
| 212 | return $allowed_mimetypes; |
||
| 213 | } |
||
| 214 | |||
| 215 | /** |
||
| 216 | * Create a "select" SQL query |
||
| 217 | * @param object $criteria {@link CriteriaElement} to match |
||
|
0 ignored issues
–
show
Should the type for parameter
$criteria not be object|null?
This check looks for It makes a suggestion as to what type it considers more descriptive. Most often this is a case of a parameter that can be null in addition to its declared types. Loading history...
|
|||
| 218 | * @param bool $join |
||
| 219 | * @return string SQL query |
||
|
0 ignored issues
–
show
|
|||
| 220 | * @access private |
||
| 221 | */ |
||
| 222 | View Code Duplication | public function _selectQuery($criteria = null, $join = false) |
|
|
0 ignored issues
–
show
This method seems to be duplicated in 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...
|
|||
| 223 | { |
||
| 224 | if (!$join) { |
||
| 225 | $sql = sprintf('SELECT * FROM %s', $this->_db->prefix($this->_dbtable)); |
||
| 226 | } else { |
||
| 227 | echo 'no need for join...'; |
||
| 228 | exit; |
||
|
0 ignored issues
–
show
The method
_selectQuery() contains an exit expression.
An exit expression should only be used in rare cases. For example, if you write a short command line script. In most cases however, using an Loading history...
|
|||
| 229 | } |
||
| 230 | if (isset($criteria) && is_subclass_of($criteria, 'criteriaelement')) { |
||
|
0 ignored issues
–
show
|
|||
| 231 | $sql .= ' ' . $criteria->renderWhere(); |
||
| 232 | if ($criteria->getSort() != '') { |
||
| 233 | $sql .= ' ORDER BY ' . $criteria->getSort() . ' ' . $criteria->getOrder(); |
||
| 234 | } |
||
| 235 | } |
||
| 236 | |||
| 237 | return $sql; |
||
| 238 | } |
||
| 239 | |||
| 240 | /** |
||
| 241 | * @param $obj |
||
| 242 | * @return string |
||
| 243 | */ |
||
| 244 | View Code Duplication | public function _insertQuery($obj) |
|
|
0 ignored issues
–
show
This method seems to be duplicated in 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...
|
|||
| 245 | { |
||
| 246 | // Copy all object vars into local variables |
||
| 247 | foreach ($obj->cleanVars as $k => $v) { |
||
| 248 | ${$k} = $v; |
||
| 249 | } |
||
| 250 | |||
| 251 | $sql = sprintf('INSERT INTO %s (mime_id, mime_ext, mime_types, mime_name, mime_admin, mime_user) VALUES |
||
| 252 | (%u, %s, %s, %s, %u, %u)', $this->_db->prefix($this->_dbtable), $mime_id, $this->_db->quoteString($mime_ext), $this->_db->quoteString($mime_types), $this->_db->quoteString($mime_name), |
||
|
0 ignored issues
–
show
|
|||
| 253 | $mime_admin, $mime_user); |
||
|
0 ignored issues
–
show
|
|||
| 254 | |||
| 255 | return $sql; |
||
|
0 ignored issues
–
show
The return type of
return $sql; (string) is incompatible with the return type of the parent method SmartpartnerBaseObjectHandler::_insertQuery of type boolean.
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design. Let’s take a look at an example: class Author {
private $name;
public function __construct($name) {
$this->name = $name;
}
public function getName() {
return $this->name;
}
}
abstract class Post {
public function getAuthor() {
return 'Johannes';
}
}
class BlogPost extends Post {
public function getAuthor() {
return new Author('Johannes');
}
}
class ForumPost extends Post { /* ... */ }
function my_function(Post $post) {
echo strtoupper($post->getAuthor());
}
Our function Loading history...
|
|||
| 256 | } |
||
| 257 | |||
| 258 | /** |
||
| 259 | * @param $obj |
||
| 260 | * @return string |
||
| 261 | */ |
||
| 262 | View Code Duplication | public function _updateQuery($obj) |
|
|
0 ignored issues
–
show
This method seems to be duplicated in 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...
|
|||
| 263 | { |
||
| 264 | // Copy all object vars into local variables |
||
| 265 | foreach ($obj->cleanVars as $k => $v) { |
||
| 266 | ${$k} = $v; |
||
| 267 | } |
||
| 268 | |||
| 269 | $sql = sprintf('UPDATE %s SET mime_ext = %s, mime_types = %s, mime_name = %s, mime_admin = %u, mime_user = %u WHERE |
||
| 270 | mime_id = %u', $this->_db->prefix($this->_dbtable), $this->_db->quoteString($mime_ext), $this->_db->quoteString($mime_types), $this->_db->quoteString($mime_name), $mime_admin, |
||
|
0 ignored issues
–
show
|
|||
| 271 | $mime_user, $mime_id); |
||
|
0 ignored issues
–
show
|
|||
| 272 | |||
| 273 | return $sql; |
||
|
0 ignored issues
–
show
The return type of
return $sql; (string) is incompatible with the return type of the parent method SmartpartnerBaseObjectHandler::_updateQuery of type boolean.
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design. Let’s take a look at an example: class Author {
private $name;
public function __construct($name) {
$this->name = $name;
}
public function getName() {
return $this->name;
}
}
abstract class Post {
public function getAuthor() {
return 'Johannes';
}
}
class BlogPost extends Post {
public function getAuthor() {
return new Author('Johannes');
}
}
class ForumPost extends Post { /* ... */ }
function my_function(Post $post) {
echo strtoupper($post->getAuthor());
}
Our function Loading history...
|
|||
| 274 | } |
||
| 275 | |||
| 276 | /** |
||
| 277 | * @param $obj |
||
| 278 | * @return string |
||
| 279 | */ |
||
| 280 | public function _deleteQuery($obj) |
||
| 281 | { |
||
| 282 | $sql = sprintf('DELETE FROM %s WHERE mime_id = %u', $this->_db->prefix($this->_dbtable), $obj->getVar('mime_id')); |
||
| 283 | |||
| 284 | return $sql; |
||
|
0 ignored issues
–
show
The return type of
return $sql; (string) is incompatible with the return type of the parent method SmartpartnerBaseObjectHandler::_deleteQuery of type boolean.
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design. Let’s take a look at an example: class Author {
private $name;
public function __construct($name) {
$this->name = $name;
}
public function getName() {
return $this->name;
}
}
abstract class Post {
public function getAuthor() {
return 'Johannes';
}
}
class BlogPost extends Post {
public function getAuthor() {
return new Author('Johannes');
}
}
class ForumPost extends Post { /* ... */ }
function my_function(Post $post) {
echo strtoupper($post->getAuthor());
}
Our function Loading history...
|
|||
| 285 | } |
||
| 286 | } // end class |
||
| 287 | |||
| 288 |
The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file.
The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place.
To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1.