cast variable #27618

astridx · 2020-01-23T22:06:54Z

Pull Request for Issue in the German forum.

Summary of Changes

Cast a default Custom Field value of 0 to -0.

Testing Instructions

Create a Custom Field of type radio and set the default value to 0 but add no value with 0.
Save the Custom Field.

Expected result

You see an error if you want to save a custom field with a default value, that is not possible.

Actual result

Saving is possible.

Documentation Changes Required

No

Note

I cast this value because the function empty is used twice: Here and here in the function test.

N6REJ · 2020-01-27T11:08:48Z

I have tested this item ✅ successfully on ecb7260

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27618.}

jwaisner · 2020-01-28T04:45:44Z

I have tested this item ✅ successfully on ecb7260

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27618.}

Quy · 2020-01-30T17:42:09Z

Not able to reproduce with v3.9.15.

astridx · 2020-01-31T21:47:10Z

Strange, I am able to reproduce with v3.9.15 today.

Quy · 2020-02-04T16:42:25Z

Sorry false alarm. I had applied your PR when testing. Duh! To me, it feels hackish fix.

…efaultvalue

astridx · 2020-02-04T21:54:59Z

You are right, this is hackish!
It would be more correct to change empty() to isset().

The problem is that emty("0") is interpreted as "there is no default value". Even "0" as a string! That is not correct. There is a default value in our case!

Do you like my new version better?

Quy · 2020-02-05T23:29:13Z

Now it is always invalid.

…efaultvalue

astridx · 2020-02-08T22:38:51Z

@Quy The last change is embarrassing to me.

Now it should fit.

The following things are considered to be empty (https://www.php.net/empty):

"" (an empty string)
0 (0 as an integer)
0.0 (0 as a float)
"0" (0 as a string)
NULL
FALSE
array() (an empty array)
and
var $var; (a variable declared, but without a value in a class)

If I am correct, we have to treat the case string "0" differently.

astridx · 2020-02-08T22:40:31Z

@N6REJ I would be happy if you could test again

administrator/components/com_fields/models/field.php

libraries/src/Form/Rule/OptionsRule.php

SharkyKZ · 2020-02-09T15:03:15Z

libraries/src/Form/Rule/OptionsRule.php

 		$required = ((string) $element['required'] == 'true' || (string) $element['required'] == 'required');

-		if (!$required && empty($value))
+		if (!$required && empty($value) && !($data['default_value'] !== "0"))


$data is undefined here.

Thanks, corrected

astridx · 2020-02-12T22:16:12Z

Requested updates are done.

Quy · 2020-02-13T21:58:13Z

I have tested this item ✅ successfully on 8a61ac9

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27618.}

SharkyKZ · 2020-02-13T22:16:15Z

Having a closer look, integer 0 and float 0.0 should also pass. So it would probably be better to compare against actually empty values (null, empty string and maybe empty array) rather than do empty() check. And we might need this for other validation rules as well.

astridx · 2020-02-14T10:36:51Z

@SharkyKZ Having a closer look, integer 0 and float 0.0 should also pass. So it would probably be better to compare against actually empty values (null, empty string and maybe empty array) rather than do empty() check.

I hope I understand you correctly. 0.0 does not pass as we use it as a string.

Do you mean this?

@SharkyKZ And we might need this for other validation rules as well.

You're right. If I find a solution here that is good, I would look at more. I think it makes this PR more complicated if I try to includ this here.

SharkyKZ · 2020-02-16T12:08:48Z

This issue affects core form validation rules, so not just custom fields but any forms using them.

astridx · 2020-02-16T13:21:25Z

@SharkyKZ I do not understand what you mean. I know that this rule does not just apply to custom fields.
I prevent a string '0' from being considered as no default value. In my opinion, this is also correct in the core. Don't you see it that way? Can you give me an example of where this is wrong.

ReLater · 2020-02-16T14:16:13Z

I haven't tested it but what's about is_numeric ?
https://www.php.net/manual/en/function.is-numeric.php

ReLater · 2020-02-16T14:23:27Z

$tests = array(
-0,
0,
0.0,
.0,
'-0',
'0',
'0.0',
'.0',
'',
null
);

foreach ($tests as $element) {
 if (is_numeric($element)) {
  echo gettype($element)  . ':', PHP_EOL;
  echo var_export($element, true) . " is numeric" . PHP_EOL;
 } else {
  echo gettype($element)  . ':', PHP_EOL;
  echo var_export($element, true) . " is NOT numeric", PHP_EOL;
 }
}
exit;

integer:
0 is numeric
integer:
0 is numeric
double:
0.0 is numeric
double:
0.0 is numeric
string:
'-0' is numeric
string:
'0' is numeric
string:
'0.0' is numeric
string:
'.0' is numeric
string:
'' is NOT numeric
NULL:
NULL is NOT numeric

if (empty($val) && !is_numeric($val))
{
 --- DO sth. ---
}

…efaultvalue

astridx · 2020-02-16T17:27:25Z

OK. Now I saw it. $value is of type mixed - although I cannot find a place for thus OptionsRule that contains anything other than string.

But I think we can use the empty() function to make it easier. I just updated the branch.

What do you all mean?

SharkyKZ · 2020-02-17T06:41:43Z

This doesn't cover negative values. If you want to keep logic this way, use is_numeric() like suggested by @ReLater.

astridx · 2020-02-17T20:51:37Z

What is not covered?
Signed 0 (-0) is in PHP equal to not signed 0 for float and int: https://stackoverflow.com/questions/31250241/is-minus-negative-zero-equivalent-to-0-in-php?answertab=votes#tab-top
Am I missing something?

SharkyKZ · 2020-02-17T21:03:49Z

No, you're right. My bad. This works fine.

ReLater · 2020-02-26T15:52:16Z

This test is irrelevant! My fault.

I have tested this item 🔴 unsuccessfully on 3ccf188

Tested like instructed with today's nightly.

Radio Value: 1 => 1

Default Values:
0
0.0
.0
-0

"Save failed with the following error: The default value is invalid."

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27618.}

ReLater · 2020-02-26T15:55:17Z

I have tested this item ✅ successfully on 3ccf188

Ah sorry. There shall be the error if 0 entered as default value.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27618.}

jwaisner · 2020-03-04T01:33:22Z

I have tested this item ✅ successfully on 3ccf188

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27618.}

jwaisner · 2020-03-04T01:33:35Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/27618.}

HLeithner · 2020-03-04T20:45:49Z

Thanks

cast variable

ecb7260

joomla-cms-bot added the PR-staging label Jan 23, 2020

astridx added 2 commits February 4, 2020 22:47

Merge branch 'staging' of https://github.com/joomla/joomla-cms into d…

d02cc61

…efaultvalue

not so hackish

2e3943e

astridx added 3 commits February 7, 2020 18:50

Merge branch 'staging' of https://github.com/joomla/joomla-cms into d…

0232e8e

…efaultvalue

check 0

737133a

correction

f7f2783

Quy reviewed Feb 9, 2020

View reviewed changes

administrator/components/com_fields/models/field.php Outdated Show resolved Hide resolved

Quy reviewed Feb 9, 2020

View reviewed changes

libraries/src/Form/Rule/OptionsRule.php Outdated Show resolved Hide resolved

SharkyKZ reviewed Feb 9, 2020

View reviewed changes

astridx added 2 commits February 9, 2020 16:52

wip

02305c3

wip

8a61ac9

Merge branch 'staging' of https://github.com/joomla/joomla-cms into d…

f3adbff

…efaultvalue

float and integer

3ccf188

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 4, 2020

HLeithner merged commit fcd1885 into joomla:staging Mar 4, 2020

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 4, 2020

HLeithner added this to the Joomla! 3.9.16 milestone Mar 4, 2020

astridx deleted the defaultvalue branch August 9, 2020 12:30

Uh oh!

cast variable #27618

cast variable #27618

Uh oh!

Conversation

astridx commented Jan 23, 2020

Summary of Changes

Testing Instructions

Expected result

Actual result

Documentation Changes Required

Note

Uh oh!

N6REJ commented Jan 27, 2020

Uh oh!

jwaisner commented Jan 28, 2020

Uh oh!

Quy commented Jan 30, 2020

Uh oh!

astridx commented Jan 31, 2020

Uh oh!

Quy commented Feb 4, 2020

Uh oh!

astridx commented Feb 4, 2020

Uh oh!

Quy commented Feb 5, 2020

Uh oh!

astridx commented Feb 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astridx commented Feb 8, 2020

Uh oh!

Uh oh!

Uh oh!

SharkyKZ Feb 9, 2020

Choose a reason for hiding this comment

Uh oh!

astridx Feb 9, 2020

Choose a reason for hiding this comment

Uh oh!

astridx commented Feb 12, 2020

Uh oh!

Quy commented Feb 13, 2020

Uh oh!

SharkyKZ commented Feb 13, 2020

Uh oh!

astridx commented Feb 14, 2020

Uh oh!

SharkyKZ commented Feb 16, 2020

Uh oh!

astridx commented Feb 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ReLater commented Feb 16, 2020

Uh oh!

ReLater commented Feb 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astridx commented Feb 16, 2020

Uh oh!

SharkyKZ commented Feb 17, 2020

Uh oh!

astridx commented Feb 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SharkyKZ commented Feb 17, 2020

Uh oh!

ReLater commented Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This test is irrelevant! My fault.

Uh oh!

ReLater commented Feb 26, 2020

Uh oh!

jwaisner commented Mar 4, 2020

Uh oh!

jwaisner commented Mar 4, 2020

Uh oh!

HLeithner commented Mar 4, 2020

Uh oh!

Reviewers

astridx commented Feb 8, 2020 •

edited

Loading

astridx commented Feb 16, 2020 •

edited

Loading

ReLater commented Feb 16, 2020 •

edited

Loading

astridx commented Feb 17, 2020 •

edited

Loading

ReLater commented Feb 26, 2020 •

edited

Loading