Added encode diacritics breaks on rp_token

mischabraam commented 1 year ago

The change from https://github.com/magento/magento2/pull/36027/files seems to be not backwards compatible with existing customer data coming from an earlier Magento version.

This change validates input with transliteration. However, this is done on every value. Also on values which are not transliterable (is that a word?). Anyway... if we use the Magento API to change an existing customer per following REST API, this translit breaks on the existing rp_token.

PUT https://www.magento.com/rest/all/V1/customers/:id

With body to update the customer including the password like per example.

{
  "customer": {
    "email": "onno@example.com",
    "Gender": 1,
    "Lastname": "Testing",
    "Store_id": 1,
    "Firstname": "Önno",
    "addresses": [
      {
        "street": [
          "Somewhere street"
        ],
        "Lastname": "Testing",
        "Postcode": "1231 EC",
        "Telephone": "0",
        "firstname": "Önno",
        "Middlename": " ",
        "country_id": "NL",
        "Default_billing": true,
        "Default_shipping": true,
        "custom_attributes": [
          {
            "value": "236",
            "attribute_code": "house_number"
          },
          {
            "value": "D",
            "attribute_code": "house_number_addition"
          }
        ],
        "city": "TEST CITY"
      }
    ],
    "Middlename": " ",
    "Website_id": 1,
    "custom_attributes": [
      {
        "value": null,
        "attribute_code": "sales_representative"
      },
      {
        "value": "5682",
        "attribute_code": "customer_group"
      },
    ],
    "id": 12345
  },
  "password": "passwordChangeLater@11"
}

Error in logging.

[2023-03-30T13:16:19.193625+00:00] report.CRITICAL: Exception: Notice: iconv(): Detected an incomplete multibyte character in input string in /var/domains/magento.shop/magento2_staging/releases/1680163528/vendor/magento/module-ea
v/Model/Attribute/Data/Text.php on line 190 in /var/domains/magento.shop/magento2_staging/releases/1680163528/vendor/magento/framework/App/ErrorHandler.php:62
Stack trace:
#0 [internal function]: Magento\Framework\App\ErrorHandler->handler(8, 'iconv(): Detect...', '/var/domains/...', 190)
#1 /var/domains/magento.shop/magento2_staging/releases/1680163528/vendor/magento/module-eav/Model/Attribute/Data/Text.php(190): iconv('UTF-8', 'ASCII//TRANSLIT...', 'b\xA85\xBA:\xA9s\x01p\x83Pn\xA9\xE3\xEF...')
#2 /var/domains/magento.shop/magento2_staging/releases/1680163528/vendor/magento/module-eav/Model/Attribute/Data/Text.php(83): Magento\Eav\Model\Attribute\Data\Text->encodeDiacritics('b\xA85\xBA:\xA9s\x01p\x83Pn\xA9\xE3\xEF...')

Screenshot_2023-03-31_at_11_31_25

Originally posted by @mischabraam in https://github.com/magento/magento2/issues/36027#issuecomment-1493867333

m2-assistant[bot] commented 1 year ago

Hi @mischabraam. Thank you for your report. To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release
For more details, review the Magento Contributor Assistant documentation.
Add a comment to assign the issue: @magento I am working on this
To learn more about issue processing workflow, refer to the Code Contributions.

Join Magento Community Engineering Slack and ask your questions in #github channel. :warning: According to the Magento Contribution requirements, all issues must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting. :clock10: You can find the schedule on the Magento Community Calendar page. :telephone_receiver: The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

m2-assistant[bot] commented 1 year ago

Hi @engcom-Bravo. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

[ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
[ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue.
[ ] 3. Add Area: XXXXX label to the ticket, indicating the functional areas it may be related to.
[ ] 4. Verify that the issue is reproducible on 2.4-develop branch
Details
- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 5. Add label Issue: Confirmed once verification is complete.
[ ] 6. Make sure that automatic system confirms that report has been added to the backlog.

jeffyl commented 1 year ago

I've just run into this issue myself. Not even using the API, just trying to edit the customer in the Magento backend will fail. Logs for that:

[2023-04-03T20:37:52.428761+00:00] main.CRITICAL: Exception message: Notice: iconv(): Detected an incomplete multibyte character in input string in /home/forge/magento.store/vendor/magento/module-eav/Model/Attribute/Data/Text.php on line 190
Trace: <pre>#1 iconv() called at [vendor/magento/module-eav/Model/Attribute/Data/Text.php:190]
#2 Magento\Eav\Model\Attribute\Data\Text->encodeDiacritics() called at [vendor/magento/module-eav/Model/Attribute/Data/Text.php:83]
#3 Magento\Eav\Model\Attribute\Data\Text->validateValue() called at [vendor/magento/module-eav/Model/Validator/Attribute/Data.php:141]
#4 Magento\Eav\Model\Validator\Attribute\Data->isValid() called at [vendor/magento/framework/Validator/Constraint.php:51]
#5 Magento\Framework\Validator\Constraint->isValid() called at [vendor/magento/framework/Validator.php:61]
#6 Magento\Framework\Validator->isValid() called at [vendor/magento/module-customer/Model/ResourceModel/Customer.php:261]
#7 Magento\Customer\Model\ResourceModel\Customer->_validate() called at [vendor/magento/module-customer/Model/ResourceModel/Customer.php:194]
#8 Magento\Customer\Model\ResourceModel\Customer->_beforeSave() called at [vendor/magento/module-eav/Model/Entity/VersionControl/AbstractEntity.php:90]
#9 Magento\Eav\Model\Entity\VersionControl\AbstractEntity->save() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#10 Magento\Customer\Model\ResourceModel\Customer\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#11 Magento\Customer\Model\ResourceModel\Customer\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/module-checkout/Model/Plugin/RecollectQuoteOnCustomerGroupChange.php:77]
#12 Magento\Checkout\Model\Plugin\RecollectQuoteOnCustomerGroupChange->aroundSave() called at [vendor/magento/framework/Interception/Interceptor.php:135]
#13 Magento\Customer\Model\ResourceModel\Customer\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#14 Magento\Customer\Model\ResourceModel\Customer\Interceptor->___callPlugins() called at [generated/code/Magento/Customer/Model/ResourceModel/Customer/Interceptor.php:104]
#15 Magento\Customer\Model\ResourceModel\Customer\Interceptor->save() called at [vendor/magento/framework/Model/AbstractModel.php:663]
#16 Magento\Framework\Model\AbstractModel->save() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#17 Magento\Customer\Model\Backend\Customer\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#18 Magento\Customer\Model\Backend\Customer\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#19 Magento\Customer\Model\Backend\Customer\Interceptor->___callPlugins() called at [generated/code/Magento/Customer/Model/Backend/Customer/Interceptor.php:23]
#20 Magento\Customer\Model\Backend\Customer\Interceptor->save() called at [vendor/magento/module-customer/Model/ResourceModel/CustomerRepository.php:257]
#21 Magento\Customer\Model\ResourceModel\CustomerRepository->save() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#22 Magento\Customer\Model\ResourceModel\CustomerRepository\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#23 Magento\Customer\Model\ResourceModel\CustomerRepository\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/module-customer/Model/Plugin/CustomerRepository/TransactionWrapper.php:44]
#24 Magento\Customer\Model\Plugin\CustomerRepository\TransactionWrapper->aroundSave() called at [vendor/magento/framework/Interception/Interceptor.php:135]
#25 Magento\Customer\Model\ResourceModel\CustomerRepository\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#26 Magento\Customer\Model\ResourceModel\CustomerRepository\Interceptor->___callPlugins() called at [generated/code/Magento/Customer/Model/ResourceModel/CustomerRepository/Interceptor.php:23]
#27 Magento\Customer\Model\ResourceModel\CustomerRepository\Interceptor->save() called at [vendor/magento/module-customer/Controller/Adminhtml/Index/Save.php:382]
#28 Magento\Customer\Controller\Adminhtml\Index\Save->execute() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#29 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#30 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#31 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->___callPlugins() called at [generated/code/Magento/Customer/Controller/Adminhtml/Index/Save/Interceptor.php:23]
#32 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->execute() called at [vendor/magento/framework/App/Action/Action.php:111]
#33 Magento\Framework\App\Action\Action->dispatch() called at [vendor/magento/module-backend/App/AbstractAction.php:151]
#34 Magento\Backend\App\AbstractAction->dispatch() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#35 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#36 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/module-backend/App/Action/Plugin/Authentication.php:145]
#37 Magento\Backend\App\Action\Plugin\Authentication->aroundDispatch() called at [vendor/magento/framework/Interception/Interceptor.php:135]
#38 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#39 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->___callPlugins() called at [generated/code/Magento/Customer/Controller/Adminhtml/Index/Save/Interceptor.php:32]
#40 Magento\Customer\Controller\Adminhtml\Index\Save\Interceptor->dispatch() called at [vendor/magento/framework/App/FrontController.php:245]
#41 Magento\Framework\App\FrontController->getActionResponse() called at [vendor/magento/framework/App/FrontController.php:212]
#42 Magento\Framework\App\FrontController->processRequest() called at [vendor/magento/framework/App/FrontController.php:147]
#43 Magento\Framework\App\FrontController->dispatch() called at [vendor/magento/framework/Interception/Interceptor.php:58]
#44 Magento\Framework\App\FrontController\Interceptor->___callParent() called at [vendor/magento/framework/Interception/Interceptor.php:138]
#45 Magento\Framework\App\FrontController\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#46 Magento\Framework\App\FrontController\Interceptor->___callPlugins() called at [generated/code/Magento/Framework/App/FrontController/Interceptor.php:23]
#47 Magento\Framework\App\FrontController\Interceptor->dispatch() called at [vendor/magento/framework/App/Http.php:116]
#48 Magento\Framework\App\Http->launch() called at [generated/code/Magento/Framework/App/Http/Interceptor.php:23]
#49 Magento\Framework\App\Http\Interceptor->launch() called at [vendor/magento/framework/App/Bootstrap.php:264]
#50 Magento\Framework\App\Bootstrap->run() called at [pub/index.php:30]

mischabraam commented 1 year ago

GITHUB-37326_Ignore_secure_attribute_encode_diacritics__v1.composer.patch

I've created a composer patch for this issue. The patch ignores the rp_token. Probably not the way to go for a proper solution, but it resolves the issue for now.

cschuler-2024 commented 1 year ago

I was able to make the issue go away by modifying _beforeSave() in Magento\Customer\Model\ResourceModel\Customer. I just moved this:

    if (!$customer->getData('ignore_validation_flag')) {
        $this->_validate($customer);
    }

so that it is called after the next block:

     if ($customer->getData('rp_token')) {
        $rpToken = $customer->getData('rp_token');
        $customer->setRpToken($this->encryptor->encrypt($rpToken));
    }

That next block sets the token to a serialized string, which isn't going to contain any diacritics, and therefore won't cause any issues when iconv() is called against it.

engcom-Bravo commented 1 year ago

@magento give me 2.4-develop instance

magento-deployment-service[bot] commented 1 year ago

Hi @engcom-Bravo. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 1 year ago

Hi @engcom-Bravo, here is your Magento Instance: https://ee875af8bb710160ce8d86e117803225.instances-prod.magento-community.engineering Admin access: https://ee875af8bb710160ce8d86e117803225.instances-prod.magento-community.engineering/admin_7365 Login: 0ba3b989 Password: 61683d85a3c0

engcom-Bravo commented 1 year ago

Hi @mischabraam,

Thank you for reporting and collaboration.

Verified the issue on Magento 2.4-develop instance and the issue is not reproducible.Kindly refer the screenshots.

Steps to reproduce

1.Try to update existing customer using REST Api.

We are able to update successfully using REST Api.

Kindly recheck the behavior on Magento 2.4-develop instance and elaborate steps to reproduce if the issue is still reproducible.

Thanks.

cschuler-2024 commented 1 year ago

Could the failure to reproduce this error be caused by the fact that it seems to only happen on older customers that were converted from Magento V1 to V2? mischabraam mentioned that he was having this problem with customers that came from earlier versions of Magento, and the customer who brought error this to my attention was an older customer who had been converted from V1.

If I look at the rp_token field on older customers, I see a hex value, which looks something like this: b62cb2863a181348b584453c9c8d11d2

But on newer customers, the value has been encrypted differently: 0:3:pquLCjwnjqXRPAIrH8yFvgUv8IIknfEy4oN7ysOqFW5n0xJ+qXY+G8Vwdlofa/CHWAoNRCLmpRmT6RxS

Maybe this is what is causing the issue to only affect older customers?

mischabraam commented 1 year ago

@cschuler-mocap Yep that is correct. But the customers in my case aren't that old ;), I mean not coming from Magento1. This project started with Magento 2.3.5-p1 and periodically upgraded to the current version 2.4.6. Since we upgraded from 2.4.5-p1 to 2.4.6 we are having these issues. So, new customers created with 2.4.6 can be updated using the REST API (or admin grid, see @jeffyl 's comment). But, customers created with 2.4.5-p1 or an older Magento version can NOT be updated.

@engcom-Bravo did you test correct?

You should install Magento 2.4.5
Create a customer, and use it (so activate its account, do some stuff like password resets and such, maybe order something... dunno)
Upgrade Magento to 2.4.6
Update the customer using the admin grid or REST API

EDIT: I see that in my initial comment I didn't mention this, excuse moi for that.

cschuler-2024 commented 1 year ago

I found a consistent way to reproduce this error.

Manually edit the rp_token field on a customer record in customer_entity and set it to 'f37ce941d293e34d25e635ee0b0d6f16'
Edit & Save that same customer in admin.

You should now get the "we can't save the customer" error message

MageCloudTeam commented 1 year ago

We have a same issue in our projects!

engcom-Bravo commented 1 year ago

Hi @mischabraam,

Thanks for your update.

Verified the issue while upgrading from Magento 2.4.5 to Magento 2.4.6 instance and the issue is not reproducible.Kindly refer the screenshots.

Steps to reproduce

You should install Magento 2.4.5
Create a customer, and use it (so activate its account, do some stuff like password resets and such, maybe order something... dunno)
Upgrade Magento to 2.4.6
Update the customer using the admin grid or REST API

We are able to update customer successfully.

Kindly let us know if we are missing anything.

Thanks.

cschuler-2024 commented 1 year ago

Verified the issue while upgrading from Magento 2.4.5 to Magento 2.4.6 instance and the issue is not reproducible.

@engcom-Bravo Did you try setting the rp_token to f37ce941d293e34d25e635ee0b0d6f16?

As I mentioned in a previous comment, the issue here is that the code is calling the validator:

    if (!$customer->getData('ignore_validation_flag')) {
        $this->_validate($customer);
    }

before calling the function to encrpyt the rp_token:

     if ($customer->getData('rp_token')) {
        $rpToken = $customer->getData('rp_token');
        $customer->setRpToken($this->encryptor->encrypt($rpToken));
    }

Which means that the validator is is attempting to do a character conversion on raw binary data. And I think this might be why it is hard to reproduce -- the contents of that binary data are entirely random, and only certain combinations of random bytes cause an issue for the character conversion function. I had to test a dozen different customer's rp_tokens before I found one that caused the error.

engcom-Bravo commented 1 year ago

Hi @cschuler-mocap @mischabraam,

Thanks for your update.

We have tried following steps

Manually edit the rp_token field on a customer record in customer_entity and set it to 'f37ce941d293e34d25e635ee0b0d6f16'
Edit & Save that same customer in admin.

We are not able to reproduce the issue.We are able to save and update the customer successfully.

We have tried with different diacritics to update the customers using PUT method.

Kindly provide exact steps to reproduce the issue It will help us to reproduce the issue.

Thanks.

mischabraam commented 1 year ago

I'm working on a test scenario. I have a project where this error is prominent. I'll create some test data in a dump for you.

mischabraam commented 1 year ago

Ok, I have a project with this issue. I have not been able to exactly track the origin of the issue. But I have a working DB dump you can use to trace and fix it. One side note: this specific project is a Magento Commerce project, so you'll need a license for this. (I've tried to downscale the DB to OpenSource but haven't succeeded in that yet).

Steps to get it working.

Install Magento Commerce 2.4.6.
Replace the created DB with the dump to find below.
Replace the config.php with the one to find below. (Not sure if this is required)
Run bin/magento set:up and bin/magento inde:rein. (You'll need a working Elasticsearch instance running on localhost:9200 or change the config)
Use Postman to GET a customer to verify it's working.
Use Postman to PUT a change to that customer to see the error.

magento-commerce.sql.gz

config.php.zip

https://api.postman.com/collections/371535-4de80353-35ea-458b-8b7a-28e9f3e7e776?access_key=PMAT-01GYYHFGFWDH6E7WN6ZQZSYQGN

rsisco commented 1 year ago

We were facing this issue as well. One oddity we faced was that the issue was not reproducible in any other environment (staging or local) using an exact database dump and the same code base.

Our solution was to nullify all rp_token and rp_token_create_at values in the customer_entity table.

simonmaass commented 1 year ago

we can confirm this issue - we are facing the exact same problem - using magento 2.4.6!

adamstapley commented 1 year ago

Same issue here after updating from 2.4.2 to 2.4.6 [cschuler-mocap] fix worked for us

ga3003 commented 1 year ago

Hi Everyone, I am also facing the same issue. I was struggling for around 6 hours to find the exact issue, as the admin's customer edit form shows an error related to customer data in the customer-assistance module. But when I debugged further, I found it was because of this "iconv" method. Then, for the time being(as a workaround), I have deleted the rp_token from the customer_entity table so that it should work properly. So if someone wants an urgent fix, you guess can follow this.

But we need a permanent solution for this.

Zwernemann commented 1 year ago

Same issue here after updating from 2.4.5 to 2.4.6 (happened for customer with rp_token=qbAzBSoSFbzw7MWpwrprn28d1x2jshXR)

As a quick fix I edited /vendor/magento/module-eav/Model/Attribute/Data/Text.php

        if($attribute->getAttributeCode()!="rp_token") {
            $value = $this->encodeDiacritics($value);
        }

        $validateLengthResult = $this->validateLength($attribute, $value);
        $errors = array_merge($errors, $validateLengthResult);

HenKun commented 1 year ago

Same issue after updating to 2.4.6. Customers are not from Magento 1, but from Magento before 2.4.6. It does not happen on all customers, just on some. This issue also prevents checkout from succeeding, if the customer is an existing customer (from before 2.4.6) and a module tries to save customer data on placeOrder. So it is even more urgent depending on the used modules.

jonathanribas commented 1 year ago

Same issue here on 2.4.6 with rp_token fvD2CYhnbsC3WkemK3OBYwGE5aSBh64s.

maaarghk commented 1 year ago

hello,

straight off the bat I want to say the "old version" thing is a red herring and I see this with a value in the format 0:3:<base64 string>.

@magento-engcom-team / @engcom-Bravo you are saying you cannot reproduce this issue which demonstrates that you did not first try to understand the bug report.

(a) the field holds encrypted data which means the tokens that people are giving you will result in different binary data within the customer model depending on value of crypt/key - you can't expect the behaviour to be reproduced by simply entering the values provided by users in this issue without also having their crypt/key which they obviously should not be sharing. i'm not surprised by random users posting these values here as they might not be aware of the inner workings of magento, but it's a really basic oversight for someone who works for adobe

(b) the issue is obviously not going to be reproducible every time since the data being validated are by definition random bytes. not all random byte sequences result in iconv heuristics thinking it is a malformed multibyte sequence rather than random binary data. you would need to be doing password reset more than 10 times and trying to save the customer between each attempt to be guaranteed to reproduce the bug. if you don't understand, i've provided an 3v4l which demonstrates the principle

(c) even considering the above it is conceptually trivial to understand that the "text" validator should not be used on arbitrary binary data such as random keys, it is not necessary to attempt to reproduce the bug to grasp that the core has bad behaviour

I reckon a dev rather than a tester needs to have a proper look at this. For now the patch in Text provided by @mischabraam is sufficient to workaround the bug. But there is a lot more to consider for a proper fix:

from the perspective of the core, the frontend model of this being hidden is essentially a text type eav attribute which is not conceptually the same as binary data and shouldn't be using the text validateValue function anyway - it should be encouraged to have a custom model used which overrides this function to return true when there is no possible validation to be made
hidden in general automatically becoming Text when the backend_type is static is a bit lame anyway, because the type of the column on the entity table could be anything. failures_num is a hidden "static" field which is a smallint but being validated as if it's text
iconv can throw an E_NOTICE error but there is no try/catch block around the call to the function which is why the error message is opaque - in case of text where there actually is malformed UTF-8 data being provided, it would actually be a useful validation result to say so
I don't think this transliterate diacritic step is actually necessary because the strlen check uses mb_strlen which is appropriate for if the string contains diacritics. However this may be an issue of backwards compatibility being broken for custom input rules which may not use multibyte functions - and I think it's fair enough if you don't expect the ecosystem of third party module providers to properly understand this issue
In the specific case of rp_token, the transliterate step happens before calling validateLength and validateInputRule functions. Those functions both check $attribute->getValidateRules() before doing anything. In the case of rp_token, that is an empty array, so no further processing actually takes place and the validation always returns successful. So there is an opportunity to do an early return here whilst retaining the transliteration function for strings where it is actually required. personally I've patched it like this:

--- Model/Attribute/Data/Text.php   2023-07-06 14:19:18.505501382 +0100
+++ Model/Attribute/Data/Text.php   2023-07-06 14:11:21.857329218 +0100
@@ -79,6 +79,10 @@
             return $errors;
         }

+        if (empty($attribute->getValidateRules())) {
+            return true;
+        }
+
         // if string with diacritics encode it.
         $value = $this->encodeDiacritics($value);

this is a general performance microimprovement which I think fixes the bug as a side effect, and there still needs to be a more comprehensive investigation into the whole system design here.

cheers, M

m2-assistant[bot] commented 1 year ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

[ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
[ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue.
[ ] 3. Add Area: XXXXX label to the ticket, indicating the functional areas it may be related to.
[ ] 4. Verify that the issue is reproducible on 2.4-develop branch
Details
- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 5. Add label Issue: Confirmed once verification is complete.
[ ] 6. Make sure that automatic system confirms that report has been added to the backlog.

mobweb commented 1 year ago

Same issue here, happening only to some customers that have been migrated from M1 a long time ago. Error message also tells me this:

> Notice: iconv(): Detected an incomplete multibyte character in input string in vendor/magento/module-eav/Model/Attribute/Data/Text.php on line 190

If we look at the data that gets passed to this method we can see that the field in question is "rp_token", and the token looks something like this:

> %¨°ßkt^4ÔH;¨¡˜hixxAZÒ

m2-assistant[bot] commented 1 year ago

Hi @engcom-November. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

[ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
[ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue.
[ ] 3. Add Area: XXXXX label to the ticket, indicating the functional areas it may be related to.
[ ] 4. Verify that the issue is reproducible on 2.4-develop branch
Details
- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!
[ ] 5. Add label Issue: Confirmed once verification is complete.
[ ] 6. Make sure that automatic system confirms that report has been added to the backlog.

sonbui00 commented 1 year ago

I think the issue relate with this commit e0f1e46 They add encrypt on the token when saving but didn't add migrate for current tokens For simple, just remove all the tokens or create a migrate for current ones

sonbui00 commented 1 year ago

We are not able to reproduce the issue.We are able to save and update the customer successfully.

@engcom-Bravo you're updating customer (with the token) use API -> can't reproduce the issue The issue can reproduce if you update token in DB directly (like old token after upgrade), and then go to admin -> customer -> save Please follow this to reproduce the issue https://github.com/magento/magento2/issues/37326#issuecomment-1507164728

jonathanribas commented 1 year ago

Hi @sonbui00, you can use this patch to fix this issue: https://github.com/magento/quality-patches/blob/master/patches/os/ACSD-52133_2.4.6.patch

sonbui00 commented 1 year ago

@jonathanribas Thank you. The patch should work. But there is another issue, the token is still not encrypted (expect from here https://github.com/magento/magento2/commit/e0f1e4653fb6f3a6eaf7816a59b21d4a6ae14688). So customers can't use the link with the token that was sent before. I think it's better to migrate all token to be encrypted

jonathanribas commented 1 year ago

@sonbui00, I see but rp_token should have a few days validity for security reasons.

sonbui00 commented 1 year ago

Thank @jonathanribas . I think we can close this issue

engcom-Hotel commented 1 year ago

Hello @maaarghk,

Thanks for the detailed explanation of the issue https://github.com/magento/magento2/issues/37326#issuecomment-1623679106 here!

We have tried multiple values as well as multiple diacritics words in order to reproduce the issue, and tried with multiple password strings as well. But the issue is still unable to reproduce. Also tried to hit the rest API multiple times as mentioned here:

(b) the issue is obviously not going to be reproducible every time since the data being validated are by definition random bytes. not all random byte sequences result in iconv heuristics thinking it is a malformed multibyte sequence rather than random binary data. you would need to be doing password reset more than 10 times and trying to save the customer between each attempt to be guaranteed to reproduce the bug. if you don't understand, i've provided an 3v4l which demonstrates the principle

It seems the issue is reproducible for specific diacritics words or characters. I am keeping this issue on Needs Update for more suggestions or activities.

Thanks

maaarghk commented 1 year ago

@engcom-Hotel any possibility of getting me access to one of your deployment service app instances to try to come up with something reproducible - don't have much time to set up a stock 2.4 instance myself rn

engcom-Hotel commented 1 year ago

Hello @maaarghk,

Thanks for the reply!

You can deploy a test instance via below command:

@magento give me 2.4-develop instance

And also please go through with the below devdocs to deploy the instances;

https://developer.adobe.com/commerce/contributor/guides/code-contributions/#deploy-vanilla-magento-open-source-instance

But in the above instances, you are not able to SSH the server.

Thanks

magento-deployment-service[bot] commented 1 year ago

Hi @engcom-Hotel. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 1 year ago

Hi @engcom-Hotel, here is your Magento Instance: https://ee875af8bb710160ce8d86e117803225.instances-prod.magento-community.engineering Admin access: https://ee875af8bb710160ce8d86e117803225.instances-prod.magento-community.engineering/admin_bf8e Login: 0937363f Password: 7904186658ff

engcom-Hotel commented 1 year ago

Hello @maaarghk,

Have you had a chance to check this comment https://github.com/magento/magento2/issues/37326#issuecomment-1681658296?

Thanks

ljr95 commented 1 year ago

Hello, In my case I needed to transfer the ecommerce data to a new clean M2.4.6 install and forgot to update the encryption key in env.php based on the first install. that fixed the problem. Good day

engcom-Hotel commented 1 year ago

Hello @mischabraam @maaarghk,

As there is no activity on this issue, we assume that the issue has been fixed for you. Hence closing this issue for now. Please feel free to comment here, if you have further information on this issue.

We will be happy to help you.

Thanks

andriydeveloper commented 1 year ago

Hello. I had the same issue after updating from 2.4.3 to 2.4.6 Changing the crypt value in env.php solved this issue.

PascalBrouwers commented 1 year ago

Can't believe this issue has been closed. @mischabraam can you reopen it?

maaarghk commented 1 year ago

@engcom-Hotel I was on holiday for most of the time, otherwise I'd have commented "oops I was wrong sorry please close".

@magento give me 2.4-develop instance

magento-deployment-service[bot] commented 1 year ago

Hi @maaarghk. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 1 year ago

Hi @maaarghk, here is your Magento Instance: https://ee875af8bb710160ce8d86e117803225.instances-prod.magento-community.engineering Admin access: https://ee875af8bb710160ce8d86e117803225.instances-prod.magento-community.engineering/admin_7611 Login: ec4e81e1 Password: 42fc3a6a5416

maaarghk commented 1 year ago

Ah, a workaround was made here: https://github.com/magento/magento2/commit/b2f10630626a162934398e4d8225d08d4f38fa70 which was merged to 2.4-develop here https://github.com/magento/magento2/commit/eb34e3477994257e0601bd1a92a04f92152c1919 on September 1st. So hopefully this will drop in the next release.

It uses the patch suggested in this thread of not calling encode diacritics, unfortunately ignoring the advice where translit shouldn't be called anyway - the appropriate bug fix for #12075 was actually just to remove an override on Symfony\Mailer (from memory) - but that's a discussion for another thread I suppose. For now this is fine closed as it is worked around.

HenKun commented 1 year ago

Can confirm https://github.com/magento/magento2/commit/b2f10630626a162934398e4d8225d08d4f38fa70 works so far. I am using it as a patch to 2.4.6-p2

magento / magento2

Added encode diacritics breaks on rp_token #37326

To learn more about issue processing workflow, refer to the Code Contributions.