Customer edit error - data corruption

[OldNGrey]

zc158 PHP7.4 and PHP 8.2 can be reproduced on Vanilla install with no additional plugins

Edit customer error

Customer created via normal catalogue creates account.
Admin Customers fields are all populated including Company Name
Customer email address has a spelling error so
- edit and correct email address
- click Update
- the following displayed fields are now blank
Company name; Street Address; Address Line 2; Post Code; City; State
Country has not changed = Indonesia
- click cancel
- edit customer - fields are populated but email address is still incorrect

By querying the data via phpMyAdmin
the entry_state = "Bali" but has length(entry_state) = 6 and char_length(entry_state)= 5
There is one char at the front that does not display but takes up a physical space. In a hex dump this C2A0 which is a non-breaking space.
Even if I correct this one field directly in the database via PHPMyadmin the errors still occur.
I cannot detect any other errors in the data but I am assuming there must be more.
No error logs are generated with PHP7.4 but the same result occurs

Any ideas?

The error log is:
------------------
[18-May-2023 12:38:08 Australia/Brisbane] Request URI: /zc158a_DEV_PHP8.2/admin/index.php?cmd=customers&origin=index&page=1&cID=4044&action=update, IP address: 192.168.1.63, Language id 1
#0 [internal function]: zen_debug_error_handler()
#1 E:\Web\zc158A_DEV_PHP8.2\admin\customers.php(754): htmlspecialchars()
#2 E:\Web\zc158A_DEV_PHP8.2\admin\index.php(11): require('E:\\Web\\zc158A_D...')
--> PHP Deprecated: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated in E:\Web\zc158A_DEV_PHP8.2\admin\customers.php on line 754.


[18-May-2023 12:38:08 Australia/Brisbane] Request URI: /zc158a_DEV_PHP8.2/admin/index.php?cmd=customers&origin=index&page=1&cID=4044&action=update, IP address: 192.168.1.63, Language id 1
#0 [internal function]: zen_debug_error_handler()
#1 E:\Web\zc158A_DEV_PHP8.2\admin\customers.php(774): htmlspecialchars()
#2 E:\Web\zc158A_DEV_PHP8.2\admin\index.php(11): require('E:\\Web\\zc158A_D...')
--> PHP Deprecated: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated in E:\Web\zc158A_DEV_PHP8.2\admin\customers.php on line 774.


[18-May-2023 12:38:08 Australia/Brisbane] Request URI: /zc158a_DEV_PHP8.2/admin/index.php?cmd=customers&origin=index&page=1&cID=4044&action=update, IP address: 192.168.1.63, Language id 1
#0 [internal function]: zen_debug_error_handler()
#1 E:\Web\zc158A_DEV_PHP8.2\admin\customers.php(793): htmlspecialchars()
#2 E:\Web\zc158A_DEV_PHP8.2\admin\index.php(11): require('E:\\Web\\zc158A_D...')
--> PHP Deprecated: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated in E:\Web\zc158A_DEV_PHP8.2\admin\customers.php on line 793.


[18-May-2023 12:38:08 Australia/Brisbane] Request URI: /zc158a_DEV_PHP8.2/admin/index.php?cmd=customers&origin=index&page=1&cID=4044&action=update, IP address: 192.168.1.63, Language id 1
#0 [internal function]: zen_debug_error_handler()
#1 E:\Web\zc158A_DEV_PHP8.2\admin\customers.php(810): htmlspecialchars()
#2 E:\Web\zc158A_DEV_PHP8.2\admin\index.php(11): require('E:\\Web\\zc158A_D...')
--> PHP Deprecated: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated in E:\Web\zc158A_DEV_PHP8.2\admin\customers.php on line 810.


[18-May-2023 12:38:08 Australia/Brisbane] Request URI: /zc158a_DEV_PHP8.2/admin/index.php?cmd=customers&origin=index&page=1&cID=4044&action=update, IP address: 192.168.1.63, Language id 1
#0 [internal function]: zen_debug_error_handler()
#1 E:\Web\zc158A_DEV_PHP8.2\admin\customers.php(850): htmlspecialchars()
#2 E:\Web\zc158A_DEV_PHP8.2\admin\index.php(11): require('E:\\Web\\zc158A_D...')
--> PHP Deprecated: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated in E:\Web\zc158A_DEV_PHP8.2\admin\customers.php on line 850.
------------------

[pilou2]

Hi,
I had a similar problem when trying to update Date of Birth field. I started an issue on GitHub last December but was not able to fix it or get help for it.
See https://github.com/zencart/zencart/issues/5482.
My understanding was that some fields data that were populated using POST variables are displayed correctly, but those which are calculated/retrieved from database when opening page just get blank after an update causing an error an preventing the update...
Customer.php file does a lot of things and its code is complex. The 'update' part certainly needs to be rewritten.
Errors in the Log are due to PHP 8.2 not liking to transmit null parameters to functions, they are not the problem, just a consequence of fields being blank.
Unfortunately, I have no solution...

[swguy]

The system really does not expect you to have an address with street address, city and state blank. Don't they have actual addresses in Bali?

I guess if you think this is a valid configuration you can fix those logs in the normal way this issue is fixed - for the first one, as an example, change


htmlspecialchars($cInfo->street_address, ENT_COMPAT, CHARSET, true),


to

htmlspecialchars($cInfo->street_address ?? '', ENT_COMPAT, CHARSET, true),

[OldNGrey]

Admin Customers fields are all populated including Company Name

It is when editing the customer record that the problem occurs. I believe the real issue is the original data entry on the catalogue side has not filtered out some control characters that breaks the admin customer program when updating. The error logs are from PHP8.2 and helps to identify the problem.

I can fix this one record through an export and reimport of data but the problem remains on the filtering of data going into the database.

[OldNGrey]

zc158
The original issue that I thought was due to control characters or non-breaking spaces in the address fields is actually having a duplicate email address.

In admin / customers.php


In zc157d the code does a set of checks on the email address.
If an error occurs the $error flag is set and a separate specific error flag is also set.
In the section that outputs the form-group for email address, at line 682 the entry_email_address errors are checked and appropriate messages displayed eg "This email address already exists!" (from ENTRY_EMAIL_ADDRESS_ERROR_EXISTS)


In zc158 the code does a set of checks on the email address (approx line 156).
If an error occurs the $error flag is set and a separate specific error flag is also set.
In the section that outputs the form-group for email address (line 648) there is no check for the entry_email_address errors and therefore there are no error messages displayed.

The screen is refreshed;
The original email address remains and the company and address fields are displayed as blank but are not written to the database
if the cancel button is clicked and the entry reedited the fields are read from the data base and displayed

[pilou2]

Same behavior with DOB field.
I added some validation and got some error message when entered data are incorrect but then still got blank fields (mainly address).
It seems that when page is refreshed after an update with error, fields that need data to be red from database are blank.
A solution might be to duplicate loading from database when no error (lines 193 to 371) in the section that manage errors (lines 372 to 374). I will try something like that and come back here after.

[pilou2]

Well, my idea was not that good but, trying it I found the origin of the problem and fixed it!
After an error occurred there is not much happening, a new object $cInfo is created, an associative array with fields name as key and parameters as values. So I checked if it was populated and all was looking OK except I found out some fields names/keys were different from when you first open (edit) this page. And guess which fields it was, those that got blank!
I did a quick fix by making old and new fields names equal. Around line 372 where you have

   } else {
                if ($error == true) {
                    $cInfo = new objectInfo($_POST);
                    $processed = true; 

I changed it to:

   } else {
                if ($error == true) {
                    $cInfo = new objectInfo($_POST);
                $cInfo->company = $cInfo->entry_company;
            $cInfo->street_address = $cInfo->entry_street_address;
            $cInfo->suburb = $cInfo->entry_suburb;
            $cInfo->postcode = $cInfo->entry_postcode;
            $cInfo->city =  $cInfo->entry_city;
                $cInfo->state = $cInfo->entry_state;
                    $processed = true; 

Blank fields problem is fixed but there were still other problems. DOB and email address need some checking and error message if necessary. Around lines 647 and 676 (after above fix) there is a line like this

);?> 

that I changed to this for DOB:

                            );
                            echo($error == true && $entry_date_of_birth_error == true ? ' ' . ENTRY_DATE_OF_BIRTH_ERROR : '');?> 

And this for email address:

                            );
                            echo($error == true && $entry_email_address_check_error == true ? ' ' . ENTRY_EMAIL_ADDRESS_ERROR : '');?> 

Now, in case of wrong entry a message will appear under concerned field.
But it is not over yet! Date validation is buggy and will bring an error 500 if you make an error in filling any field of the form. PHP checkdate function needs integers as arguments. I added '(int)' in lines 143, 144 and 145 to solve that.
No more crash, well nearly... The order of parameters sent to checkdate function is designed for dates coming from database and already sanitized to this format 'yyyymmdd'. When updating with American date mm/dd/yyyy everything got messed up! I added a condition in the if test to accept this format as well. Code is now like this:

    if (ACCOUNT_DOB == 'true') {
                if (checkdate(
                    (int)substr(zen_date_raw($customers_dob), 4, 2),
                    (int)substr(zen_date_raw($customers_dob), 6, 2),
                    (int)substr(zen_date_raw($customers_dob), 0, 4)
                ) || checkdate(
                    (int)substr(zen_date_raw($customers_dob), 0, 2),
                    (int)substr(zen_date_raw($customers_dob), 3, 2),
                    (int)substr(zen_date_raw($customers_dob), 6, 4)
                )) {
                    $entry_date_of_birth_error = false; 

To make it work with Great Britain and few other European countries date format dd/mm/yyyy, following code should be added too:

        ) || checkdate(
                    (int)substr(zen_date_raw($customers_dob), 3, 2),
                    (int)substr(zen_date_raw($customers_dob), 0, 2),
                    (int)substr(zen_date_raw($customers_dob), 6, 4) 

There was a lot to fix and there is still few things to improve too in this file... For example First name is not mandatory, design or bug I don't know but I think it should be, I changed this around line 600:

                    ) . ' class="form-control" id="customers_firstname" minlength="' . ENTRY_FIRST_NAME_MIN_LENGTH . '"' 

To

                    ) . ' class="form-control" id="customers_firstname" minlength="' . ENTRY_FIRST_NAME_MIN_LENGTH . '"',
                 true 

That should be all for today, I will post this on GitHub issue tomorrow.

[swguy]

If you would PR that would be even better. Thanks for driving this to conclusion.

[pilou2]

OK I will, but I take a little more time to check everything.;)

[pilou2]

I was right to check again. DOB messed up when error was not due to validation problem as I thought but more display problem. DOB is displayed using zen_date_short function which return short date in local format from a 'raw date'. Problem is when updating date used as entry is coming from field and is not raw anymore.
Using a condition before display around line 637 solved it.
Old code:

        ((empty($cInfo->customers_dob) || $cInfo->customers_dob <= '0001-01-01' || $cInfo->customers_dob == '0001-01-01 00:00:00') ? '' : zen_date_short(
                $cInfo->customers_dob
                )), 

New code:

        ((empty($cInfo->customers_dob) || $cInfo->customers_dob <= '0001-01-01' || $cInfo->customers_dob == '0001-01-01 00:00:00') ? '' :
        (($action == 'edit') ? zen_date_short($cInfo->customers_dob) : $cInfo->customers_dob)
                ), 

Please ignore precedent fix after line 145 which is not necessary:

if (ACCOUNT_DOB == 'true') {
                if (checkdate(
                    (int)substr(zen_date_raw($customers_dob), 4, 2),
                    (int)substr(zen_date_raw($customers_dob), 6, 2),
                    (int)substr(zen_date_raw($customers_dob), 0, 4)
                ) || checkdate(
                    (int)substr(zen_date_raw($customers_dob), 0, 2),
                    (int)substr(zen_date_raw($customers_dob), 3, 2),
                    (int)substr(zen_date_raw($customers_dob), 6, 4)
                )) {
                    $entry_date_of_birth_error = false;