Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CS after merge https://github.com/joomla/joomla-cms/pull/7540 #11843

Merged
merged 2 commits into from Aug 30, 2016
Merged

CS after merge https://github.com/joomla/joomla-cms/pull/7540 #11843

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a61989
Update modules.php
zero-24 Aug 29, 2016
072d19d
Update modules.php
zero-24 Aug 30, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 17 additions & 18 deletions components/com_config/model/modules.php
View file
Open in desktop
Expand Up @@ -12,13 +12,10 @@
/**
* Config Module model.
*
* @package Joomla.Site
* @subpackage com_config
* @since 3.2
* @since 3.2
*/
class ConfigModelModules extends ConfigModelForm
{

/**
* Method to auto-populate the model state.
*
Expand Down Expand Up @@ -120,13 +117,13 @@ protected function preprocessForm(JForm $form, $data, $group = 'content')
/**
* Method to get list of module positions in current template
*
* @return array
* @return array
*
* @since 3.2
* @since 3.2
*/
public function getPositions()
{
$lang = JFactory::getLanguage();
$lang = JFactory::getLanguage();
$templateName = JFactory::getApplication()->getTemplate();

// Load templateDetails.xml file
Expand Down Expand Up @@ -165,7 +162,7 @@ public function getPositions()
// Add custom position to options
$customGroupText = JText::_('COM_MODULES_CUSTOM_POSITION');

$editPositions = true;
$editPositions = true;
$customPositions = self::getActivePositions(0, $editPositions);
$templateGroups[$customGroupText] = self::createOptionGroup($customGroupText, $customPositions);

Expand All @@ -179,15 +176,17 @@ public function getPositions()
* @param boolean $editPositions Allow to edit the positions
*
* @return array A list of positions
*
* @since __DEPLOY_VERSION__
*/
public static function getActivePositions($clientId, $editPositions = false)
{
$db = JFactory::getDbo();
$query = $db->getQuery(true)
->select('DISTINCT(position)')
->from('#__modules')
->where($db->quoteName('client_id') . ' = ' . (int) $clientId)
->order('position');
$db = JFactory::getDbo();
$query = $db->getQuery(true)
->select('DISTINCT position')
->from($db->quoteName('#__modules'))
->where($db->quoteName('client_id') . ' = ' . $db->quote($clientId))
Copy link
Contributor

Choose a reason for hiding this comment

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

why quote an integer?

Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't be quoting int's. In one of the db languages i remember that caused massive problems

Copy link
Contributor

Choose a reason for hiding this comment

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

I've heard that there is a performance cost to quoting ints in that mysql needs to do a type conversion from string to the datatype of the column. Typically quoting is related to reducing SQL injection risks.

If I remember right Quoting * (an asterisk) in a select causes issues on MySQL but I'm unaware of quoting ints causing problems with the non-MySQL db languages.

reference http://www.sigmainfy.com/blog/sql-single-quotes-for-numeric-values-is-it-really-good-practice.html

"Do not use quote around numeric values in SQL query for performance. Rather, make sure in your say PHP code, make sure the numeric values are indeed valid numeric ones before making up the complete SQL query to protect your db from SQL injection."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed..

->order($db->quoteName('position'));

$db->setQuery($query);

Expand All @@ -210,11 +209,11 @@ public static function getActivePositions($clientId, $editPositions = false)
{
if (!$position && !$editPositions)
{
$options[] = JHtml::_('select.option', 'none', ':: ' . JText::_('JNONE') . ' ::');
$options[] = JHtml::_('select.option', 'none', ':: ' . JText::_('JNONE') . ' ::');
}
else
{
$options[] = JHtml::_('select.option', $position, $position);
$options[] = JHtml::_('select.option', $position, $position);
}
}

Expand All @@ -229,7 +228,7 @@ public static function getActivePositions($clientId, $editPositions = false)
*
* @return object The option as an object (stdClass instance)
*
* @since 3.0
* @since __DEPLOY_VERSION__
*/
private static function createOption($value = '', $text = '')
{
Expand All @@ -253,7 +252,7 @@ private static function createOption($value = '', $text = '')
*
* @return array Return the new group as an array
*
* @since 3.0
* @since __DEPLOY_VERSION__
*/
private static function createOptionGroup($label = '', $options = array())
{
Expand Down