Skip to content
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
d020e05
Use Elvis instead of full ternary
frankmayer Sep 29, 2016
f8d247c
Use prefixed increment instead of assignment
frankmayer Sep 29, 2016
bcd8841
Use single quotes instead of doublequotes
frankmayer Sep 29, 2016
e1288c3
Remove unnecessary parentheses
frankmayer Sep 29, 2016
fdd996c
Replace alias function with original
frankmayer Sep 29, 2016
dfc5b98
Remove useless return
frankmayer Sep 29, 2016
aac1916
Removed unnecessary ternary operators
frankmayer Sep 29, 2016
90ffc54
Fixed callable call in loop termination condition
frankmayer Sep 29, 2016
a9d98bd
Replace is_null() with ... === null
frankmayer Sep 29, 2016
33ba088
Shortened syntax for applied operations
frankmayer Sep 29, 2016
5c7afbf
Don't use strlen() to check if string is empty.
frankmayer Sep 29, 2016
9a1a4b6
Replace strstr() with strpos()
frankmayer Sep 29, 2016
a8f2a36
Replace substr() with strpos()
frankmayer Sep 29, 2016
a158cae
Merge unset()
frankmayer Sep 29, 2016
b3da6e9
Inline one-time use variables
frankmayer Sep 29, 2016
58b8fbb
Type safe string comparison
frankmayer Sep 29, 2016
8f28982
Remove unused variable
frankmayer Sep 29, 2016
776666b
Merge if statements into parents
frankmayer Sep 29, 2016
7b70c06
Optimize non-optimal if conditions.
frankmayer Sep 29, 2016
af3e78b
More type-safe comparisons
frankmayer Sep 29, 2016
f5fc398
Fix another non-optimal if condition
frankmayer Sep 30, 2016
ac4921a
Merged another str_replace() case
frankmayer Sep 30, 2016
6f79ff1
Fix own typo
frankmayer Sep 30, 2016
943f500
Fix formatting
frankmayer Sep 30, 2016
ac0ebb4
Further optimize an if condition
frankmayer Sep 30, 2016
b9adbc5
Fix codesniffer
frankmayer Sep 30, 2016
42a5a54
Fixes...
frankmayer Oct 4, 2016
9e8063e
Changes, based on discussions with @csthomas
frankmayer Dec 9, 2016
492fb9e
One more change, based on previous discussion.
frankmayer Dec 10, 2016
475b880
Merge branch 'staging' into Performance_4
frankmayer Dec 10, 2016
7edf386
A few more minor fixes
frankmayer Dec 10, 2016
5e7202b
Fix slipped through merge conflict
frankmayer Dec 10, 2016
9581e65
Correction in comparison
frankmayer Dec 10, 2016
d0d9261
Correction in comparison with trim result
frankmayer Dec 10, 2016
bea9c0b
Merge branch 'staging' of https://github.com/joomla/joomla-cms into P…
frankmayer Dec 18, 2016
bf361ca
Merge branch 'staging' into Performance_4
frankmayer Dec 19, 2016
b7636dc
Reverted this one to not clash with @laoneo's refactoring efforts
frankmayer Dec 29, 2016
755c648
Merge branch 'staging' into Performance_4
frankmayer May 29, 2017
8afee4f
Merge branch 'staging' into Performance_4
frankmayer May 31, 2017
5f011db
Some changes according to reviewer's comments
frankmayer Jun 1, 2017
76fe7a9
Some changes according to reviewer's comments
frankmayer Jun 1, 2017
33d7bd2
Additional changes according to reviewer's comments
frankmayer Jun 2, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions plugins/authentication/cookie/cookie.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
$cookieArray = explode('.', $cookieValue);

// Check for valid cookie value
if (count($cookieArray) != 2)
if (count($cookieArray) !== 2)
{
// Destroy the cookie in the browser.
$this->app->input->cookie->set($cookieName, false, time() - 42000, $this->app->get('cookie_path', '/'), $this->app->get('cookie_domain'));
Expand Down Expand Up @@ -224,7 +224,7 @@ public function onUserAfterLogin($options)
return false;
}

if (isset($options['responseType']) && $options['responseType'] == 'Cookie')
if (isset($options['responseType']) && $options['responseType'] === 'Cookie')
{
// Logged in using a cookie
$cookieName = 'joomla_remember_me_' . JUserHelper::getShortHashedUserAgent();
Expand Down Expand Up @@ -269,7 +269,7 @@ public function onUserAfterLogin($options)
{
$results = $this->db->setQuery($query)->loadResult();

if (is_null($results))
if ($results === null)
{
$unique = true;
}
Expand All @@ -279,7 +279,7 @@ public function onUserAfterLogin($options)
$errorCount++;

// We'll let this query fail up to 5 times before giving up, there's probably a bigger issue at this point
if ($errorCount == 5)
if ($errorCount === 5)
{
return false;
}
Expand Down
6 changes: 3 additions & 3 deletions plugins/authentication/gmail/gmail.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
$this->loadLanguage();

// No backend authentication
if (JFactory::getApplication()->isAdmin() && !$this->params->get('backendLogin', 0))
if (!$this->params->get('backendLogin', 0) && JFactory::getApplication()->isAdmin())
{
return;
}
Expand Down Expand Up @@ -65,7 +65,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
}

// Check if we have a username and password
if (strlen($credentials['username']) == 0 || strlen($credentials['password']) == 0)
if ($credentials['username'] === '' || $credentials['password'] === '')
{
$response->type = 'GMail';
$response->status = JAuthentication::STATUS_FAILURE;
Expand Down Expand Up @@ -191,7 +191,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
foreach ($localUsers as $localUser)
{
// Local user exists with same username but different email address
if ($email != $localUser->email)
if ($email !== $localUser->email)
{
$response->status = JAuthentication::STATUS_FAILURE;
$response->error_message = JText::sprintf('JGLOBAL_AUTH_FAILED', JText::_('PLG_GMAIL_ERROR_LOCAL_USERNAME_CONFLICT'));
Expand Down
6 changes: 3 additions & 3 deletions plugins/authentication/joomla/joomla.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
}

// Check the two factor authentication
if ($response->status == JAuthentication::STATUS_SUCCESS)
if ($response->status === JAuthentication::STATUS_SUCCESS)
{
$methods = JAuthenticationHelper::getTwoFactorMethods();

Expand All @@ -115,7 +115,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
}

// Check if the user has enabled two factor authentication
if (empty($otpConfig->method) || ($otpConfig->method == 'none'))
if (empty($otpConfig->method) || ($otpConfig->method === 'none'))
{
// Warn the user if they are using a secret code but they have not
// enabed two factor auth in their account.
Expand Down Expand Up @@ -166,7 +166,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
// Did the user use an OTEP instead?
if (empty($otpConfig->otep))
{
if (empty($otpConfig->method) || ($otpConfig->method == 'none'))
if (empty($otpConfig->method) || ($otpConfig->method === 'none'))
{
// Two factor authentication is not enabled on this account.
// Any string is assumed to be a valid OTEP.
Expand Down
10 changes: 5 additions & 5 deletions plugins/authentication/ldap/ldap.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
{
// Bind using Connect Username/password
// Force anon bind to mitigate misconfiguration like [#7119]
if (strlen($this->params->get('username')))
if ($this->params->get('username') !== '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$this->params is an intance of Registry. Method get if key not exists may returns null.
I would only change to if ($this->params->get('username')).
Otherwise you have to be 100% sure that this won't break anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would be better. Thanks, will change.

{
$bindtest = $ldap->bind();
}
Expand All @@ -82,9 +82,9 @@ public function onUserAuthenticate($credentials, $options, &$response)
if ($bindtest)
{
// Search for users DN
$binddata = $ldap->simple_search(str_replace("[search]", $credentials['username'], $this->params->get('search_string')));
$binddata = $ldap->simple_search(str_replace('[search]', $credentials['username'], $this->params->get('search_string')));

if (isset($binddata[0]) && isset($binddata[0]['dn']))
if (isset($binddata[0], $binddata[0]['dn']))
{
// Verify Users Credentials
$success = $ldap->bind($binddata[0]['dn'], $credentials['password'], 1);
Expand Down Expand Up @@ -112,7 +112,7 @@ public function onUserAuthenticate($credentials, $options, &$response)

if ($success)
{
$userdetails = $ldap->simple_search(str_replace("[search]", $credentials['username'], $this->params->get('search_string')));
$userdetails = $ldap->simple_search(str_replace('[search]', $credentials['username'], $this->params->get('search_string')));
}
else
{
Expand All @@ -126,7 +126,7 @@ public function onUserAuthenticate($credentials, $options, &$response)
{
$response->status = JAuthentication::STATUS_FAILURE;

if (!strlen($response->error_message))
if ($response->error_message === '')
{
$response->error_message = JText::_('JGLOBAL_AUTH_INCORRECT');
}
Expand Down
16 changes: 8 additions & 8 deletions plugins/captcha/recaptcha/recaptcha.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ public function onInit($id = 'dynamic_recaptcha_1')
{
$pubkey = $this->params->get('public_key', '');

if ($pubkey == null || $pubkey == '')
if ($pubkey === null || $pubkey === '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: can pubkey be null? when in line above default to '' if doesn't exist

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it can't... changing

{
throw new Exception(JText::_('PLG_RECAPTCHA_ERROR_NO_PUBLIC_KEY'));
}

if ($this->params->get('version', '1.0') == '1.0')
if ($this->params->get('version', '1.0') === '1.0')
{
JHtml::_('jquery.framework');

Expand Down Expand Up @@ -82,7 +82,7 @@ public function onInit($id = 'dynamic_recaptcha_1')
*/
public function onDisplay($name = null, $id = 'dynamic_recaptcha_1', $class = '')
{
if ($this->params->get('version', '1.0') == '1.0')
if ($this->params->get('version', '1.0') === '1.0')
{
return '<div id="' . $id . '" ' . $class . '></div>';
}
Expand Down Expand Up @@ -117,13 +117,13 @@ public function onCheckAnswer($code = null)
case '1.0':
$challenge = $input->get('recaptcha_challenge_field', '', 'string');
$response = $input->get('recaptcha_response_field', '', 'string');
$spam = ($challenge == null || strlen($challenge) == 0 || $response == null || strlen($response) == 0);
$spam = ($challenge === null || $challenge === '' || $response === null || $response === '');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

break;
case '2.0':
// Challenge Not needed in 2.0 but needed for getResponse call
$challenge = null;
$response = $input->get('g-recaptcha-response', '', 'string');
$spam = ($response == null || strlen($response) == 0);
$spam = ($response === null || $response === '');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

break;
}

Expand Down Expand Up @@ -226,7 +226,7 @@ private function getResponse($privatekey, $remoteip, $response, $challenge = nul
*/
private function _recaptcha_qsencode($data)
{
$req = "";
$req = '';

foreach ($data as $key => $value)
{
Expand Down Expand Up @@ -258,14 +258,14 @@ private function _recaptcha_http_post($host, $path, $data, $port = 80)
$http_request = "POST $path HTTP/1.0\r\n";
$http_request .= "Host: $host\r\n";
$http_request .= "Content-Type: application/x-www-form-urlencoded;\r\n";
$http_request .= "Content-Length: " . strlen($req) . "\r\n";
$http_request .= 'Content-Length: ' . strlen($req) . "\r\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again why use ' when you use " in all others?

$http_request .= "User-Agent: reCAPTCHA/PHP\r\n";
$http_request .= "\r\n";
$http_request .= $req;

$response = '';

if (($fs = @fsockopen($host, $port, $errno, $errstr, 10)) == false )
if (($fs = @fsockopen($host, $port, $errno, $errstr, 10)) === false )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove space after false.

{
die('Could not open socket');
}
Expand Down
19 changes: 9 additions & 10 deletions plugins/captcha/recaptcha/recaptchalib.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ class JReCaptchaResponse

class JReCaptcha
{
private static $_signupUrl = "https://www.google.com/recaptcha/admin";
private static $_siteVerifyUrl = "https://www.google.com/recaptcha/api/siteverify";
private static $_signupUrl = 'https://www.google.com/recaptcha/admin';
private static $_siteVerifyUrl = 'https://www.google.com/recaptcha/api/siteverify';
private $_secret;
private static $_version = "php_1.0";
private static $_version = 'php_1.0';

/**
* Constructor.
Expand All @@ -53,10 +53,10 @@ class JReCaptcha
*/
public function __construct($secret)
{
if ($secret == null || $secret == "")
if ($secret === null || $secret === '')
{
die("To use reCAPTCHA you must get an API key from <a href='"
. self::$_signupUrl . "'>" . self::$_signupUrl . "</a>");
. self::$_signupUrl . "'>" . self::$_signupUrl . '</a>');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again why use ' when you use " in all others?

}
$this->_secret = $secret;
}
Expand All @@ -70,7 +70,7 @@ public function __construct($secret)
*/
private function _encodeQS($data)
{
$req = "";
$req = '';
foreach ($data as $key => $value)
{
$req .= $key . '=' . urlencode(stripslashes($value)) . '&';
Expand All @@ -94,9 +94,8 @@ private function _submitHTTPGet($path, $data)
{
$req = $this->_encodeQS($data);
$http = JHttpFactory::getHttp();
$response = $http->get($path . '?' . $req)->body;

return $response;
return $http->get($path . '?' . $req)->body;
}

/**
Expand All @@ -111,7 +110,7 @@ private function _submitHTTPGet($path, $data)
public function verifyResponse($remoteIp, $response)
{
// Discard empty solution submissions
if ($response == null || strlen($response) == 0)
if ($response === null || $response === '')
{
$recaptchaResponse = new JReCaptchaResponse();
$recaptchaResponse->success = false;
Expand All @@ -132,7 +131,7 @@ public function verifyResponse($remoteIp, $response)
$answers = json_decode($getResponse, true);
$recaptchaResponse = new JReCaptchaResponse();

if (trim($answers['success']) == true)
if (trim($answers['success']) === true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this is my last error note :)

If am I right function trim returns string.
I would replace by if (trim($answers['success']))

After you fix it I can mark test as success.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was being overzealous... 😄 Will do.

{
$recaptchaResponse->success = true;
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/content/contact/contact.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ protected function getContactId($created_by)
$query->where('contact.published = 1');
$query->where('contact.user_id = ' . (int) $created_by);

if (JLanguageMultilang::isEnabled() == 1)
if (JLanguageMultilang::isEnabled() === 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work because JLanguageMultilang::isEnabled() return boolean!
Should be:
if (JLanguageMultilang::isEnabled() === true) or better if (JLanguageMultilang::isEnabled())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, this one slipped through... :)
I will go for if (JLanguageMultilang::isEnabled()).

{
$query->where('(contact.language in '
. '(' . $this->db->quote(JFactory::getLanguage()->getTag()) . ',' . $this->db->quote('*') . ') '
Expand Down
18 changes: 9 additions & 9 deletions plugins/content/emailcloak/emailcloak.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class PlgContentEmailcloak extends JPlugin
public function onContentPrepare($context, &$row, &$params, $page = 0)
{
// Don't run this plugin when the content is being indexed
if ($context == 'com_finder.indexer')
if ($context === 'com_finder.indexer')
{
return true;
}
Expand Down Expand Up @@ -68,13 +68,13 @@ protected function _getPattern ($link, $text)
*/
protected function _addAttributesToEmail($jsEmail, $before, $after)
{
if ($before !== "")
if ($before !== '')
{
$before = str_replace("'", "\'", $before);
$jsEmail = str_replace(".innerHTML += '<a '", ".innerHTML += '<a {$before}'", $jsEmail);
}

if ($after !== "")
if ($after !== '')
{
$after = str_replace("'", "\'", $after);
$jsEmail = str_replace("'\'>'", "'\'{$after}>'", $jsEmail);
Expand Down Expand Up @@ -123,7 +123,7 @@ protected function _cloak(&$text, &$params)
$searchText = '((?:[\x20-\x7f]|[\xA1-\xFF]|[\xC2-\xDF][\x80-\xBF]|[\xE0-\xEF][\x80-\xBF]{2}|[\xF0-\xF4][\x80-\xBF]{3})[^<>]+)';

// Any Image link
$searchImage = "(<img[^>]+>)";
$searchImage = '(<img[^>]+>)';

// Any Text with <span or <strong
$searchTextSpan = '(<span[^>]+>|<span>|<strong>|<strong><span[^>]+>|<strong><span>)' . $searchText . '(</span>|</strong>|</span></strong>)';
Expand Down Expand Up @@ -283,12 +283,12 @@ protected function _cloak(&$text, &$params)
* Search for derivatives of link code <a href="mailto:[email protected]">
* <img anything>[email protected]</a>
*/
$pattern = $this->_getPattern($searchEmail, ($searchImage . $searchEmail));
$pattern = $this->_getPattern($searchEmail, $searchImage . $searchEmail);

while (preg_match($pattern, $text, $regs, PREG_OFFSET_CAPTURE))
{
$mail = $regs[2][0];
$mailText = $regs[4][0] . ($regs[5][0]);
$mailText = $regs[4][0] . $regs[5][0];

$replacement = JHtml::_('email.cloak', $mail, $mode, $mailText);

Expand All @@ -303,7 +303,7 @@ protected function _cloak(&$text, &$params)
* Search for derivatives of link code <a href="mailto:[email protected]">
* <img anything>any text</a>
*/
$pattern = $this->_getPattern($searchEmail, ($searchImage . $searchText));
$pattern = $this->_getPattern($searchEmail, $searchImage . $searchText);

while (preg_match($pattern, $text, $regs, PREG_OFFSET_CAPTURE))
{
Expand Down Expand Up @@ -435,7 +435,7 @@ protected function _cloak(&$text, &$params)
* Search for derivatives of link code
* <a href="mailto:[email protected]?subject=Text"><img anything>[email protected]</a>
*/
$pattern = $this->_getPattern($searchEmailLink, ($searchImage . $searchEmail));
$pattern = $this->_getPattern($searchEmailLink, $searchImage . $searchEmail);

while (preg_match($pattern, $text, $regs, PREG_OFFSET_CAPTURE))
{
Expand All @@ -459,7 +459,7 @@ protected function _cloak(&$text, &$params)
* Search for derivatives of link code
* <a href="mailto:[email protected]?subject=Text"><img anything>any text</a>
*/
$pattern = $this->_getPattern($searchEmailLink, ($searchImage . $searchText));
$pattern = $this->_getPattern($searchEmailLink, $searchImage . $searchText);

while (preg_match($pattern, $text, $regs, PREG_OFFSET_CAPTURE))
{
Expand Down
4 changes: 2 additions & 2 deletions plugins/content/joomla/joomla.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class PlgContentJoomla extends JPlugin
public function onContentAfterSave($context, $article, $isNew)
{
// Check we are handling the frontend edit form.
if ($context != 'com_content.form')
if ($context !== 'com_content.form')
{
return true;
}
Expand Down Expand Up @@ -108,7 +108,7 @@ public function onContentAfterSave($context, $article, $isNew)
public function onContentBeforeDelete($context, $data)
{
// Skip plugin if we are deleting something other than categories
if ($context != 'com_categories.category')
if ($context !== 'com_categories.category')
{
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/content/loadmodule/loadmodule.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class PlgContentLoadmodule extends JPlugin
public function onContentPrepare($context, &$article, &$params, $page = 0)
{
// Don't run this plugin when the content is being indexed
if ($context == 'com_finder.indexer')
if ($context === 'com_finder.indexer')
{
return true;
}
Expand Down
Loading