?
Referenced as Pull Request for: # 7335
avatar PhocaCz
PhocaCz
2 Jul 2015

Steps to reproduce the issue

$app = JFactory::getApplication();
$params = $app->getParams();
$param_specific = $params->get( 'param_specific', array(2,3,4) );

Joomla! 3.4.1 output
print($param_specific); // Array ( [0] => 2 [1] => 3 [2] => 4 )

Joomla! 3.4.2 output:
print($param_specific); // 0

Expected result

print($param_specific); // Array ( [0] => 2 [1] => 3 [2] => 4 )

Actual result

print($param_specific); // 0

System information (as much as possible)

Joomla! 3.4.1 (Joomla Platform 13.1.0 Stable) , 3.4.2 (Joomla Platform 13.1.0 Stable) , PHP 5.5.6, MySQL 5.6.14

Additional comments

Updating Joomla! from 3.4.1 to 3.4.2 changes the rule of reading parameters which means the highest security issue (for example because of access parameters, etc.)

Not sure what changed in the code, if the ZERO (0) was handled as FALSE and now it is handled as 0 and not as FALSE (false means, the parameter is not set and will be set by default value) but maybe such changes which completely change the behavior of system should be done between large version releases not between minor version releases.

Will be great to get info, how the parameters will be handled (if this will stay of will be fixed)

avatar PhocaCz PhocaCz - open - 2 Jul 2015
avatar brianteeman
brianteeman - comment - 2 Jul 2015

Please check #7314 and #7317


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7316.

avatar brianteeman
brianteeman - comment - 2 Jul 2015

Please check #7314 and #7317


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7316.

avatar rdeutz
rdeutz - comment - 2 Jul 2015

I tried it on staging and it works as expected. Don't understand your additional comment

avatar zero-24
zero-24 - comment - 2 Jul 2015

@PhocaCz can you try it again with staging to be sure?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7316.

avatar zero-24
zero-24 - comment - 2 Jul 2015

@PhocaCz I also can't reproduce this with 3.4.2

$app = JFactory::getApplication();
$params = $app->getParams();
$param_specific = $params->get( 'param_specific', array(2,3,4) );

echo 'Joomla! 3.4.1 output';
print_r($param_specific); // Array ( [0] => 2 [1] => 3 [2] => 4 )

echo 'Joomla! 3.4.2 output:';
print_r($param_specific); // 0

Result:

Joomla! 3.4.1 outputArray ( [0] => 2 [1] => 3 [2] => 4 ) Joomla! 3.4.2 output:Array ( [0] => 2 [1] => 3 [2] => 4 )

BTW: Only print I get a error (Array to sting ...). I have add that code to the template index.php file. Can you please retest?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7316.

avatar Fedik
Fedik - comment - 2 Jul 2015

@PhocaCz as @zero-24 already noted, print($param_specific); cannot print array :wink:
your description seems wrong, maybe you just mixed up something in your code? :wink:

I cannot reproduce this also

avatar PhocaCz
PhocaCz - comment - 2 Jul 2015

Hi, sorry for confusing non programmers, I wrote the code directly here not in text editor, so of course, I made mistake: print is wrong, it should be print_r but it does not matter which function you will use for getting the value, just use own debug function.

I thought, the core developer who changed the behavior of parameters, will describe the changes which were made in the code causing this.

The code I pasted here, is an example. If you want to test it, you need to take some existing parameter from some existing extension (if 3rd party or core extension, I think, it does not matter, I have tested it with 3rd party extension)

I think, the problem is with empty value parameters which were not loaded in 3.4.1 but are loaded in 3.4.2

Example:

In extension, there is a parameter called "our_parameter" where you can set values, for example "1,2,3" - in computer language an array --> array(1,2,3)

Joomla! 3.4.1
We store options in administration of the extension and "our_parameter" does not have any value (we didn't set any value), what will happen:

  • this parameter is not loaded
  • when we need the parameter value, we get it from default value: $params->get( 'our_parameter', array(2,3,4) );
  • so "our_parameter" will get value: array(2,3,4)

Joomla! 3.4.2
We store options and "our_parameter" does not have any value, what will happen:

  • this parameter is loaded (this is different to 3.4.1) and include this value: array() (it is empty array)
  • when we need the parameter value, we don't get it from default value: $params->get( 'our_parameter', array(2,3,4) );
  • so "our_parameter" will get value: array()

So in 3.4.1 the parameter was not loaded so the value was taken from default value, in 3.4.2 the parameter is loaded with empty value so then the empty value is taken and not the default value is used (In Joomla! when the parameter has empty value, normally the default value is taken but this changed)

What I mean with "is loaded, it is not loaded"

// Getting parameters from extension
$params = $app->getParams();
print_r($params);

In Joomla! 3.4.1, empty parameters (parameters where we didn't set the value) were not loaded, in Joomla! 3.4.2, they are loaded with empty values which then cause NOT USING a default value in this function: $params->get( 'our_parameter', array(2,3,4) )

I hope, you understand. Thank you, Jan


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7316.

avatar PhocaCz
PhocaCz - comment - 2 Jul 2015

Tested now in 3.4.3 and there is the same problem like in 3.4.2

3.4.1 - empty value ( Array ( ) ) allows loading of default value --> $params->get( parameter, default value )
3.4.2(3) - empty value ( Array() ) is not handled as empty value and does not allow to load default value --> $params->get( parameter, default value )


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7316.

avatar Fedik
Fedik - comment - 2 Jul 2015

if i understand you right here is testing code:

$params = new Joomla\Registry\Registry;
$params->set('some_val', array());
$val = $params->get('some_val', array(2,3,4) );
var_dump($val);

but for me it works the same for both 3.4.1 and 3.4.3,
it return empty array, as should

avatar PhocaCz
PhocaCz - comment - 2 Jul 2015

Hi Fedik, in fact you are testing the code after the error. The problem is coming before this code - when loading all parameters from Options.

Your test:

  • you set the parameter so it is included in object "$params" - so you get the same results in 3.4.1 and 3.4.3

But the problem is different handling of loading the parameters in 3.4.1 and 3.4.3:

  • 3.4.1 - when the parameter is empty, it is not loaded to object "$params"
  • 3.4.3 - when the parameter is empty, it is loaded to object "$params"

Example:
1) Extension has 3 parameters: parameter1, parameter2, parameter3
2) We set values for parameter1 and parameter 3, we don't set value for parameter2

On the page, this is loaded in parameter object:
3.4.1: Joomla\Registry\Registry Object ( [data:protected] => stdClass Object ( [parameter1] => array(1,2) [parameter3] => array(1,2)
3.4.3: Joomla\Registry\Registry Object ( [data:protected] => stdClass Object ( [parameter1] => array(1,2) [parameter2] => array() [parameter3] => array(1,2)

So what will happen in 3.4.1:
$val = $params->get('some_val', array(2,3,4) ); // $val become array(2,3,4) - we don't have the parameter2 set, so we get the default value
What will happen in 3.4.3:
$val = $params->get('some_val', array(2,3,4) ); // $val become array() - we have the parameter2 "set", so we get the value set in options (even it is empty)

So in fact the solution in 3.4.1 can be "BUG" but it can be "FIX" and I as extension developer need to know, it is a bug or it is a fix. If bug, I will wait for fix. If fix, I need to change my extensions ASAP (as for example, parameters like access rights which values are stored in arrays - exactly examples I made - are fatally affected by changed behavior - which happened between 3.4.1 and 3.4.2)

Jan

avatar Fedik
Fedik - comment - 2 Jul 2015

hm, strange ... as I know it from JSON string
I tried

$params = new Joomla\Registry\Registry('{"parameter1":[1,2,3], "parameter2":[], "parameter3":true}');
// and
$params = new Joomla\Registry\Registry('{"parameter1":[1,2,3], "parameter2":null, "parameter3":true}');

but in all cases I have all 3 parameters in $params for 3.4.1 and 3.4.3 ...

well, hope someone know better :smile:

avatar PhocaCz
PhocaCz - comment - 2 Jul 2015

Ok, I have found the part which I think, changes the behavior.

The problem is not when loading the parameters but even earlier - when saving the parameters

libraries/joomla/form/form.php line cca 238
Joomla! 3.4.1

// Get the field value from the data input.
            if ($group)
            {
                // Filter the value if it exists.
                if ($input->exists($group . '.' . $name))
                {
                    $output->set($group . '.' . $name, $this->filterField($field, $input->get($group . '.' . $name, (string) $field['default'])));
                }
            }
            else
            {
                // Filter the value if it exists.
                if ($input->exists($name))
                {
                    $output->set($name, $this->filterField($field, $input->get($name, (string) $field['default'])));
                }
            }

Joomla! 3.4.3

$key = $group ? $group . '.' . $name : $name;

            // Filter the value if it exists.
            if ($input->exists($key))
            {



                $output->set($key, $this->filterField($field, $input->get($key, (string) $field['default'])));

            }


            // Get the JFormField object for this field, only it knows if it is supposed to be multiple.
            $jfield = $this->getField($name, $group);

            // Fields supporting multiple values must be stored as empty arrays when no values are selected.
            // If not, they will appear to be unset and then revert to their default value.
            if ($jfield && $jfield->multiple && !$output->exists($key))
            {





                $output->set($key, array());
            }

This change (maybe, the problem is in some of the called function) in fact do the following:

  • in 3.4.1 - it does not save parameters with empty values
  • in 3.4.3 - it saves them and this means, they are stored but with empty values.

And this has effect on default values - because if the parameter is NOT included (3.4.1), default value is set. But if it is included, e.g. with empty value, default value is NOT set as the empty value is used.

Maybe this has effect on multiple values only (not sure, didn't test the standard - e.g. text values) but multiple values are used by access rights for example, so not it is very important to know, it is a bug or a fix.

Fedik

  • Try to test it only with multiple values (so all parameters have multiple values)

EDIT:

  • Try to load (3.4.1):
{"param1":[1,2],"param3":[1,2]}
  • and then (3.4.3):
{"param1":[1,2],"param2":[],"param3":[1,2]}

Param2 is not saved in database in 3.4.1 and this is the difference between 3.4.1 and 3.4.3

Thank you, Jan

avatar PhocaCz
PhocaCz - comment - 2 Jul 2015

In short, so it can be more understandable:

  • Joomla! 3.4.1 does not store parameters with empty values
  • Joomla! 3.4.3 stores them

This code: $params->get( parameter, default value ) applies the default value when it does not find the parameter (3.4.1).
But it does not apply the default value when it finds the parameter (even it is stored with empty value) (3.4.3)

Jan

avatar Fedik
Fedik - comment - 3 Jul 2015

this changes was introduced by this PR #2617

@PhocaCz can you please provide sample for XML form/fields, so it will be more simple to confirm and test this issue

avatar PhocaCz
PhocaCz - comment - 3 Jul 2015

Hi, see:

<field name="param2" type="accesslevel" label="Param 2 Label" description="Param 2 Description" multiple="true"  />

(only for info, default value ist not set in XML, because it does not accept multiple values: 1,2 or [1,2] or ["1","2"])

But it does not matter, important is the different handling of empty values:

  • in 3.4.1 empty value means the parameter is NOT set so default value from code will be set
  • in 3.4.3 empty value means the parameter is SET but with empty value so default value in code will be ignored

I cannot say which way is right but the change has fatal impact on security because it changes the way how - for example - access rights will be handled.

In extensions I have tested, "opened doors will be closed" after update (3.4.1 -> 3.4.2) but it can happen that somewhere "closed doors will be open" - access to some protected part of the site will be open.

hypothetical example:
User groups: 2,4,6 do not have access to some part of the site. When administrator saves the options and access level parameter will leave unchanged then:

  • in 3.4.1 the default values will be set (2,4,6) and the part will be protected
  • in 3.4.3 the default value will be NOT set (as empty value is taken as value) and the part will be NOT more protected by default values.

And this will happen only when administrator updates the system (without knowing this change)

avatar Fedik
Fedik - comment - 3 Jul 2015

ok, I finally understand :smile:

Joomla 3.4.1 save <select name="param2" multiple="multiple"> as {"param2": null},
Joomla 3.4.2 save <select name="param2" multiple="multiple"> as {"param2": []}

This changed in #2617.
From some point of view it is valid behavior.

And as for null JRegistry return default, and for [] return empty array, you got such issue.

I agree, such thing can make unexpected effect, for extensions that expect default value from params if value empty.
But I not sure what solution can be here, let's wait someone smarter :smile:

avatar Bakual
Bakual - comment - 3 Jul 2015

Sounds like the issue would be located in our Registry class. @mbabker @wilsonge want to have a look?
https://github.com/joomla-framework/registry/blob/master/src/Registry.php#L235-L238 returns the default value only if the path is not found, null or an empty string. Is there a reason an empty array doesn't trigger the default?

avatar Bakual
Bakual - comment - 3 Jul 2015

Thinking about it it is probably expected behavior. Otherwise you couldn't have a multiple option field where nothing is selected, the default would be applied in that case again. Which is what the original PR wanted to fix.

avatar rdeutz
rdeutz - comment - 3 Jul 2015

@Bakual looking at the lines 245ff in #2617 I think this must change the behaviour because the last part only does something for "multiple" formfields and before this part was executed for all fields in $input. The question is does that matter, I don't know and I have my doubts that the author of the change knows. But maybe I am wrong.

avatar Fedik
Fedik - comment - 3 Jul 2015

I think if we revert #2617 we lost nothing, actually if User select nothing the value still equal null, no matter what the field type there, multiple or not (from some point of view :smile:)

Not sure what effect can be if we change JRegistry current behavior.

avatar Bakual
Bakual - comment - 3 Jul 2015

Yeah, I think the original PR was wrong.
Default values always apply when nothing is selected. For example in a text field or color field when you delete the content, the default value will be applied if there is one. If you don't want that behavior, you should not define a default value. Having a default value specified implies that the field cannot be empty.
Elin wrote that in the JC item already and I think she was right.

So I'd say revert #2617.

avatar wilsonge
wilsonge - comment - 3 Jul 2015

I think it's dicey. I can see a situation where on the creation of an item you might want some options to be checked by default. but if a user wants to disable them then that's not a problem...

avatar Bakual
Bakual - comment - 3 Jul 2015

Yep, but that's not how our default values work.
You would have to write own logic which only applies on "create" and not on "edit".
Our default values don't care if it's a new item or an edited one.

avatar Fedik
Fedik - comment - 3 Jul 2015

seems everyone agree that revert #2617 is best solution here, so here it is #7335 :smile:

avatar wilsonge
wilsonge - comment - 3 Jul 2015

@Bakual I agree with that for everything except checkboxes. which imho is a special case.

avatar Bakual
Bakual - comment - 3 Jul 2015

True. For checkboxes I don't think you can really support default values. The way the spec is, you can't distinguish between a non-checked or non-present checkbox. Since only the values of those who are checked are sent.
I don't even know if JFormFieldCheckboxes currently works with defaults set. Last time I checked it didn't work as expected and I resolved it in a different way :smile:

avatar Fedik
Fedik - comment - 3 Jul 2015

checkboxes problem I usually fix by adding one more hidden input with same name on front of the checkboxes inputs,
so if checkboxes are unchecked then will be submitted hidden input with empty value, and if checked then the checked value will override that hidden input :smile:

avatar PhocaCz
PhocaCz - comment - 3 Jul 2015

Hi Fedik, one small correction:

It is not:

Joomla 3.4.1 save <select name="param2" multiple="multiple"> as {"param2": null},
Joomla 3.4.2 save <select name="param2" multiple="multiple"> as {"param2": []}

but:

Joomla 3.4.1 save <select name="param2" multiple="multiple"> as {},
Joomla 3.4.2 save <select name="param2" multiple="multiple"> as {"param2": []}

When the parameter is not set in Options (Joomla! 3.4.1) it is not loaded as a whole (parameter including its value).

But maybe there is next problem (which can be independent to this), I am not able to set default value for multiple select box in XML:

This WORKS:

<field name="param2" type="accesslevel" label="Param 2 Label" description="Param 2 Description" multiple="true" default="1" />

This DOES NOT WORK:

<field name="param2" type="accesslevel" label="Param 2 Label" description="Param 2 Description" multiple="true" default="1,2" />
<field name="param2" type="accesslevel" label="Param 2 Label" description="Param 2 Description" multiple="true" default="[1,2]" />
<field name="param2" type="accesslevel" label="Param 2 Label" description="Param 2 Description" multiple="true" default="['1','2']" />
<field name="param2" type="accesslevel" label="Param 2 Label" description="Param 2 Description" multiple="true" default="array(1,2)" />

When you cannot set default value in XML, then the only one place is to set it in code - $params->get(parameter, default value) and when this does not work, it is really not good situation.

Yes, the question for some of the next "large" version of Joomla! is how to deal with empty values ( array() ): When the parameter gets array():

a) handle it as a value and use array()
b) or handle it as missing value and set the default value in php code?

Jan

avatar Fedik
Fedik - comment - 4 Jul 2015

@Bakual @wilsonge what do you think about that staging...Fedik:checkbox-empty-value as alternative fix for #2617 , worth to make a pull? :neckbeard:

avatar Bakual
Bakual - comment - 4 Jul 2015

Depends how the behavior will be. I never tried it :)

avatar Fedik
Fedik - comment - 5 Jul 2015

Here quick explanation:

// When checkbox checked
$post = array();
$post['checkbox'] = ''; // hidden input
$post['checkbox'] = 1; // real checkbox(es) input

// When checkbox unchecked
$post = array();
$post['checkbox'] = ''; // hidden input

Longer explanation:
In PHP you cannot have 2 same key in the array,
so when PHP parse the request with two fields with same name, then it replace value of the first field by value from the second field.
In case with unchecked checkboxes will be submitted only the hidden input, that will emulate "empty value" for checkboxes.

I already use such thing in my custom fields :smile:

avatar Fedik
Fedik - comment - 5 Jul 2015

well, this some kind of "hack", there is better solution I think https://groups.google.com/forum/#!topic/joomla-dev-cms/lwvSE4c6W94

avatar brianteeman
brianteeman - comment - 5 Apr 2016

Trying to work out if this is still a valid issue - for something marked as critical it is almost 9 months old


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7316.

avatar Fedik
Fedik - comment - 5 Apr 2016

I thought it already fixed in #7381 and #7806 no?

avatar brianteeman
brianteeman - comment - 5 Apr 2016

I dont know that is why I am asking ;)

On 5 April 2016 at 09:28, Fedir Zinchuk notifications@github.com wrote:

I thought it already fixed in #7381
#7381 and #7806
#7806 no?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#7316 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar Fedik
Fedik - comment - 5 Apr 2016

for me it is fixed :smiley:

avatar brianteeman
brianteeman - comment - 5 Apr 2016

OK thanks I will close it then

avatar brianteeman brianteeman - change - 5 Apr 2016
Status New Closed
Closed_Date 0000-00-00 00:00:00 2016-04-05 08:45:56
Closed_By brianteeman
avatar brianteeman brianteeman - close - 5 Apr 2016

Add a Comment

Login with GitHub to post a comment