Skip to content
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

net: fix IPv4 validation regex #40174

Closed
wants to merge 1 commit into from

Conversation

VoltrexKeyva
Copy link
Member

Fixed the IPv4's validation regex so it also matches IPv4 octets that are prefixed by a 0 as they're also considered valid octets.

Fixes: #40173

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels Sep 21, 2021
@VoltrexKeyva VoltrexKeyva force-pushed the patch-5 branch 2 times, most recently from 4dc46b6 to 7e8cb99 Compare September 21, 2021 22:04
Copy link
Contributor

@mscdex mscdex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good idea considering some software can/will interpret zero-prefixed numbers as octal instead of base 10, which I don't think anyone intends to have happen these days.

@VoltrexKeyva
Copy link
Member Author

@mscdex that is quite interesting, although I don't it is really necessary to worry about those since most of the software will interpret them as base-10, I don't think those said softwares would even support these builds whatsoever, maybe we should also wait for other TSC members or collabrators reviews on this as well.

Fixed the IPv4's validation regex so it also matches IPv4 octets that
are prefixed by a `0` as they're also considered valid octets.

Fixes: nodejs#40173
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VoltrexKeyva
Copy link
Member Author

Oh didn't realize that, closing.

@7c
Copy link

7c commented Sep 22, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

net.isIP detection issue
5 participants