Topic: Bug due to commit

This commit has created a bug shown in image.

I could have caught just one. May be this is affecting other places too.

Please rectify.

www.boxygen.pk

Re: Bug due to commit

Please try to give more details about this. I don't know how to fix it.

Joe

Re: Bug due to commit

Which version of PHP are you using?
Make a test script with empty array and see what appears in var_dump on using the new function.

In the old dispensation, the argument could be either an array or a scalar and the count() faithfully returned an acceptable result - was it a 0 or just false? In the current codebase, it will return a 0 if "it is a scalar" and also "even if it is an array and if the array has no elements".

Re: Bug due to commit

The following test code was executed in PHP 5.3.3:

    $a = 0; // Scalar
    $b = count($a);
    echo var_dump($b); // returns int(1)

    $a = array(); // Array
    $b = count($a);
    echo var_dump($b); // returns int(0)

Hence we should return a 1 if it is a scalar and not a 0!

Post's attachments

Array_Count_Output_and_Fix.zip 36.3 kb, 2 downloads since 2018-12-15 

You don't have the permssions to download the attachments of this post.

Re: Bug due to commit

Yes changing the return to 1 has worked.

www.boxygen.pk

Re: Bug due to commit

@joe: Please commit this and verify that all files that partook of this change and the ones before this function came in are all okay.

Re: Bug due to commit

I guess you are right guys. Sorry that we didn't detect this before the minor release. However now it seems to work.
Please help me test it. You can make a global search on count_array and try to run the routines involved. It should be 6 files.

Committed to stable repo. New file can be downloaded here.

Joe

8 (edited by boxygen 12/16/2018 10:50:05 am)

Re: Bug due to commit

Found this bug after new commit

And I doubt this one also have logical error after this new commit.

www.boxygen.pk

Re: Bug due to commit

yes, function count_array() is for the purpose as of its name.
all places that are checking positive intergers need a different function.

function is_positive_int($input)
{
    if(ctype_digit($input) && (int)$input > 0)
        return true;

    return false;
}

and line 371 of sales_db.inc needs to be modified to :

if (!is_positive_int($trans_no))

also roll back the last commit.

Phuong

Re: Bug due to commit

@notrinos

The function is_positive_int doesn't work as you provide in php 7.2.

However changing

if (!is_positive_int($trans_no))
to
if (!trans_no)
do work.

what do you think?

joe

Re: Bug due to commit

@joe
my function is not working because it is called inside function get_sales_child_documents which use 3 times in file view_sales_order.php validating for both number and array.

your solution is working in this case but seems we still need a function to check both array element and number

Phuong

Re: Bug due to commit

before the function count_array() was deployed we used PHP build in function count() for validating both array and number
since the count() has been changed to validate array, object only so we need to create new function in FA for checking both number and array.
The current function count_array() should be modified to:

function fa_count($input)
{
    if(is_array($input))
        return count($input);
    elseif(ctype_digit($input) && (int)$input > 0)
        return (int)$input;
    else
        return 0;
}

Do this we could change every curently count() to fa_count() without worry about errors that may arise somewhere.

Phuong

Re: Bug due to commit

ok, I will just test this before committing.

Joe

Re: Bug due to commit

Hello again,

I have now tested your example, @notrinos, and it doesn't work as intended.
I have now changed the function count_array($array) in /includes/ui/ui_globals.inc to this:

function count_array($array)
{
    return (is_array($array)) ? count($array) : (($array === NULL) ? 0 : 1);
}

and it works for PHP versions 7.2.12, 7.1.9, 7.0.23, 5.6.31, 5.5.25 and 5.4.10.

It gives the same return as the old count() without errors or warnings.
We only have to convert more counts() if we detect more errors. We can wait until next major to do this.

This has now been committed and a new file can be downloaded here:

/Joe.

Re: Bug due to commit

It works in PHP 5.3.1 on WinXP3 as well (XAMPP 1.7.3).