From 4a30f88f2c19f3e0a81f80125b553c769e742c8d Mon Sep 17 00:00:00 2001 From: Cameron Date: Thu, 10 Dec 2020 18:02:28 -0800 Subject: [PATCH] check_class() code optimization --- class2.php | 73 ++++++++++++---------------- e107_handlers/admin_ui.php | 2 + e107_tests/tests/unit/class2Test.php | 21 ++++++++ 3 files changed, 55 insertions(+), 41 deletions(-) diff --git a/class2.php b/class2.php index f56aaea18..47fe62d28 100755 --- a/class2.php +++ b/class2.php @@ -77,13 +77,14 @@ if(!isset($_E107['cli'])) // // Can't be moved to e107, required here for e107_config vars security -$register_globals = true; +/*$register_globals = true; if(function_exists('ini_get')) { $register_globals = ini_get('register_globals'); -} +}*/ // Destroy! (if we need to) +/* if($register_globals === true) { if(isset($_REQUEST['_E107'])) { unset($_E107); } @@ -95,26 +96,8 @@ if($register_globals === true) } } unset($global); -} +}*/ -// MOVED TO $e107->prepare_request() -// TODO - better ajax detection method (headers when possible) -//define('e_AJAX_REQUEST', isset($_REQUEST['ajax_used'])); -//unset($_REQUEST['ajax_used']); // removed because it's auto-appended from JS (AJAX), could break something... -// -//if(isset($_E107['minimal']) || e_AJAX_REQUEST) -//{ -// $_e107vars = array('forceuserupdate', 'online', 'theme', 'menus', 'prunetmp'); -// foreach($_e107vars as $v) -// { -// $noname = 'no_'.$v; -// if(!isset($_E107[$v])) -// { -// $_E107[$noname] = 1; -// } -// unset($_E107[$v]); -// } -//} // Set Absolute file-path of directory containing class2.php if(!defined('e_ROOT')) @@ -1310,7 +1293,7 @@ $e107 = e107::getInstance(); // Is this needed now? $dbg->logTime('IP Handler and Ban Check'); e107::getIPHandler()->ban(); -if(USER && !isset($_E107['no_forceuserupdate']) && varset($pref['force_userupdate']) && $_SERVER['QUERY_STRING'] !== 'logout') +if(USER && !isset($_E107['no_forceuserupdate']) && $_SERVER['QUERY_STRING'] !== 'logout' && varset($pref['force_userupdate'])) { if(isset($currentUser) && force_userupdate($currentUser)) { @@ -1660,7 +1643,8 @@ if (!file_exists(FOOTERF)) //define('LOGINMESSAGE', ''); - not needed, breaks login messages define('OPEN_BASEDIR', (ini_get('open_basedir') ? true : false)); -define('SAFE_MODE', (ini_get('safe_mode') ? true : false)); +define('SAFE_MODE', false); + define('FILE_UPLOADS', (ini_get('file_uploads') ? true : false)); define('INIT', true); if(isset($_SERVER['HTTP_REFERER'])) @@ -1734,43 +1718,48 @@ function check_email($email) function check_class($var, $userclass = USERCLASS_LIST, $uid = 0) { $e107 = e107::getInstance(); - if($var === e_LANGUAGE) + if ($var === e_LANGUAGE) { - return TRUE; + return true; } - if(e107::isCli()) + if (e107::isCli()) { global $_E107; - if(empty($_E107['phpunit'])) + if (empty($_E107['phpunit'])) { return true; } } - if(is_numeric($uid) && $uid > 0) - { // userid has been supplied, go build that user's class list + if (is_numeric($uid) && $uid > 0) + { // userid has been supplied, go build that user's class list $userclass = class_list($uid); } - if (empty($userclass)) + if ($userclass == '') { - return FALSE; + return false; } $class_array = !is_array($userclass) ? explode(',', $userclass) : $userclass; $varList = !is_array($var) ? explode(',', $var) : $var; - $latchedAccess = FALSE; + $latchedAccess = false; - foreach($varList as $v) + foreach ($varList as $v) { $v = trim($v); - $invert = FALSE; + $invert = false; //value to test is a userclass name (or garbage, of course), go get the id - if(!is_numeric($v)) + if (!is_numeric($v)) { - if (strncmp($v, '-', 1) === 0) + if ($v === '') + { + return false; + } + + if ($v[0] === '-') { $invert = TRUE; $v = substr($v, 1); @@ -1779,27 +1768,29 @@ function check_class($var, $userclass = USERCLASS_LIST, $uid = 0) } elseif ($v < 0) { - $invert = TRUE; + $invert = true; $v = -$v; } if ($v !== FALSE) { + // var_dump($v); // Ignore non-valid userclass names - if (($v === '0') || ($v === 0) || in_array($v, $class_array, true)) + if (($v === '0') || ($v === 0) || in_array($v, $class_array)) { if ($invert) { - return FALSE; + return false; } $latchedAccess = TRUE; } - elseif ($invert && count($varList) === 1) + elseif ($invert && count($varList) == 1) { // Handle scenario where only an 'exclude' class is passed - $latchedAccess = TRUE; + $latchedAccess = true; } } } + return $latchedAccess; } diff --git a/e107_handlers/admin_ui.php b/e107_handlers/admin_ui.php index fca3c3a23..984d5632d 100755 --- a/e107_handlers/admin_ui.php +++ b/e107_handlers/admin_ui.php @@ -1152,11 +1152,13 @@ class e_admin_dispatcher { if(isset($this->access[$route]) && !e107::getUser()->checkClass($this->access[$route], false)) { + e107::getMessage()->addDebug("Userclass Permissions Failed: ".$this->access[$route]); return false; } if(is_array($this->perm) && isset($this->perm[$route]) && !e107::getUser()->checkAdminPerms($this->perm[$route])) { + e107::getMessage()->addDebug("Admin Permissions Failed.".$this->perm[$route]); return false; } diff --git a/e107_tests/tests/unit/class2Test.php b/e107_tests/tests/unit/class2Test.php index eafb88c01..124a711fe 100644 --- a/e107_tests/tests/unit/class2Test.php +++ b/e107_tests/tests/unit/class2Test.php @@ -54,6 +54,15 @@ $result = check_class(0, "253,254,250,251,0"); $this->assertTrue($result); + $result = check_class('NEWSLETTER', "253,254,250,251,0"); + $this->assertFalse($result); + + $result = check_class('NEWSLETTER', "253,254,250,251,3,0"); // NEWSLETTER = 3 + $this->assertTrue($result); + + $result = check_class('-NEWSLETTER', "253,254,250,251,0"); + $this->assertTrue($result); + $result = check_class(254, "253,254,250,251,0"); $this->assertTrue($result); @@ -63,6 +72,18 @@ $result = check_class(null, "253,254,250,251,0"); $this->assertFalse($result); + $result = check_class('-254', "253,254,250,251,0"); + $this->assertFalse($result); + + $result = check_class('-254', "253,250,251,0"); + $this->assertTrue($result); + + $result = check_class(-254, "253,250,251,0"); + $this->assertTrue($result); + + $result = check_class(-254, "254,253,250,251,0"); + $this->assertFalse($result); + $result = check_class(e_UC_NOBODY, "253,254,250,251,0"); $this->assertFalse($result);