Skip to content

REMOTE_IP gets saved partially when using IPV6 #10395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
keevitaja opened this issue Aug 1, 2017 · 12 comments
Closed

REMOTE_IP gets saved partially when using IPV6 #10395

keevitaja opened this issue Aug 1, 2017 · 12 comments
Assignees
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line help wanted Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@keevitaja
Copy link

keevitaja commented Aug 1, 2017

When customer is using IPV6 address, the remote_ip might get saved only partially into sales_order and quote tables depending on the address str length.

Problem is in the remote_ip column length which currently is VARCHAR(32)

https://en.wikipedia.org/wiki/IPv6

Preconditions

  1. tested on 2.1.5, but InstallSchema in 2.1.7 has also VARCHAR(32)

Steps to reproduce

  1. Customer creates new order

Expected result

  1. 2001:7d0:84ae:8d80:704b:bca4:b3a8:197

Actual result

  1. 2001:7d0:84ae:8d80:704b:bca4:b3a

To fix this, the remote_ip column in sales_order and quote tables must be VARCHAR(45)

@andypieters
Copy link

Same issue here.
We need a valid enduser ip address for starting a payment.
If the ipaddress is invalid, customers cannot pay the order

@magento-engcom-team magento-engcom-team added G1 Passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Oct 1, 2017
@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 2, 2017
@magento-engcom-team
Copy link
Contributor

@keevitaja, thank you for your report.
We've created internal ticket(s) MAGETWO-83202 to track progress on the issue.

@OmarAssadi
Copy link

Didn't know about this until it almost burned me earlier in the week. This really needs resolving. More and more customers are using IPv6. And, had we not had access logs on, I'm not really sure what we would have done to catch this particular fraudulent customer.

@magento-engcom-team magento-engcom-team added the Event: distributed-cd Distributed Contribution Day label Mar 19, 2018
@Zifius
Copy link
Member

Zifius commented Mar 23, 2018

I'll take this, field length needs to be updated in the quote table as well

@luismaldonado91
Copy link

@Zifius I've updated both sales_order and quote tables. It's a must for signifyd in order to work properly.

@georgeschiopu
Copy link
Member

@Zifius hope you don't mind if I take this, since there hasn't been an update in over a month? Have some spare time this evening and seems it's a quick one.

@Zifius
Copy link
Member

Zifius commented May 4, 2018 via email

georgeschiopu pushed a commit to georgeschiopu/magento2 that referenced this issue May 4, 2018
@magento-engcom-team
Copy link
Contributor

Hi @keevitaja. Thank you for your report.
The issue has been fixed in #14976 by @georgeschiopu in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.5 release.

@magento-engcom-team
Copy link
Contributor

Hi @keevitaja. Thank you for your report.
The issue has been fixed in #15142 by @dmytro-ch in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.0 release.

@melamity
Copy link

melamity commented May 26, 2018

I saw the commits in question here and I am left wondering about the x_forwarded_for field too - checking my Magento 2 installation here, it does seem that this has the same schema definition that remote_ip once had, which would cause some problems if you have a IPv6 "X-Forwarded-For" IP.

About the new size of the field being set to 45 - wouldn't it be more efficient setting the field to a size of 39, considering that an IPv6 address has 8 groups of 4 hexadecimal numbers, and 7 colons to separate them which makes (8 * 4) = 32 + 7 = 39 characters?

I can re-submit a PR for this later if this is something you guys want, with the above changes that I have suggested.

@keevitaja
Copy link
Author

@megubyte 45 is correct!

https://www.google.ee/search?q=ipv6+max+length

@maartjegroen
Copy link

@megubyte I am left wondering about some other fields too. A quick search in my Magento (2.1) installation resulted in the following list.

table name field name
admin_user_session ip
password_reset_request_event ip
quote remote_ip
rating_option_vote remote_ip
sales_order remote_ip
sales_order x_forwarded_for
sendfriend_log ip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line help wanted Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

10 participants