Store longer X-Forwarded-For header

[ihor-sviziev]

Description (*)

Header X-Forwarded-For could contains multiple IPs
This PR allow to store longer values

For instance in my DB I had value like this:
4x.xxx.xx.xx, 1xx.xxx.xx.xx, 10., where xxx - masked IP address of customer, while last IP address was just cut due to limit on the field in DB.

Related Pull Requests

N/A

Fixed Issues (if relevant)

Fixes #28693

Manual testing scenarios (*)

1. Install Magento behind reverse proxy and Varnish
2. Go to website and place order
3. Go to admin > Sales > Orders
4. Navigate to your order, see data in field "Placed from IP"

Actual result:
X-Forwarded-For header is cut

Expected result:
It shouldn't be cut

Questions or comments

Ma

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @ihor-sviziev. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

[dmytro-ch]

Nice catch, @ihor-sviziev!
The issue is relevant and must be fixed.

I guess this change can be covered by an integration test. We can either update an existing fixture or create a new one.
What do you think?

Thank you!

[ihor-sviziev]

Will check it little bit later

…

On Wed, 11 Mar 2020 at 10:24, Dmytro Cheshun ***@***.***> wrote: ***@***.**** requested changes on this pull request. Nice catch, @ihor-sviziev <https://github.com/ihor-sviziev>! The issue is relevant is must be fixed. I guess this change can be covered by an integration test. We can either update an existing fixture <https://github.com/magento/magento2/blob/2.4-develop/dev/tests/integration/testsuite/Magento/Sales/_files/order.php> or create a new one. What do you think? Thank you! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27221?email_source=notifications&email_token=AAOJOUOV6INLYACV7JJ4C73RG5DEPA5CNFSM4LEZLOX2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCY2KTLI#pullrequestreview-372550061>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOJOUJ7NV72HWPI7VULNHTRG5DEPANCNFSM4LEZLOXQ> .

[ihor-sviziev]

Unfortunately I don't have free time for implementing tests for my change.

@engcom-Golf @engcom-Charlie could you help with test coverage?

[engcom-Charlie]

@magento run all tests

[ihor-sviziev]

@engcom-Charlie maybe it's better to have more real value, like IPV6 address, or multiple ipv4 IPs?

[engcom-Charlie]

@ihor-sviziev,
In my opinion, it doesn't matter if there are IP addresses or symbols, because the PR change is the length of characters in the column.

[ihor-sviziev]

@dmytro-ch could you review again this pr?

[dmytro-ch]

The changes look good to me. Thank you!

[magento-engcom-team]

Hi @dmytro-ch, thank you for the review.
ENGCOM-7756 has been created to process this Pull Request

[engcom-Delta]

✔️ QA passed
Was checked value longer than 32 symbols is saved in x_forwarded_for in database

[m2-assistant]

Hi @ihor-sviziev, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.