Skip to content

Fix ip2long in AIX to treat IPs with leading zeros as invalid like LINUX#21296

Open
reshmavk wants to merge 4 commits intophp:PHP-8.4from
reshmavk:gh-21295
Open

Fix ip2long in AIX to treat IPs with leading zeros as invalid like LINUX#21296
reshmavk wants to merge 4 commits intophp:PHP-8.4from
reshmavk:gh-21295

Conversation

@reshmavk
Copy link
Copy Markdown

@reshmavk reshmavk commented Feb 25, 2026 •
edited
Loading

Fixes #21295

@reshmavk reshmavk requested a review from bukka as a code owner February 25, 2026 17:20
devnexen
devnexen reviewed Feb 25, 2026
View reviewed changes
ext/standard/basic_functions.c Outdated
For consistency, we convert back the IP to a string and check if it is equal to
the original string. If not, the IP should be considered invalid.
*/
if (strcmp(addr, inet_ntoa(ip)) != 0) {
Copy link
Copy Markdown
Member

@devnexen devnexen Feb 25, 2026

Choose a reason for hiding this comment

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

quick question: is inet_ntop available on AIX ?

Copy link
Copy Markdown
Author

@reshmavk reshmavk Feb 25, 2026

Choose a reason for hiding this comment

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

Yes, inet_ntop is available

Copy link
Copy Markdown
Member

@devnexen devnexen Feb 25, 2026

Choose a reason for hiding this comment

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

I think it is preferable over inet_ntoa. I know it s easier, but it is also deprecated.

Copy link
Copy Markdown
Author

@reshmavk reshmavk Feb 27, 2026

Choose a reason for hiding this comment

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

I will update the code to use inet_ntop instead of inet_ntoa.

mvorisek
mvorisek reviewed Feb 27, 2026
View reviewed changes
ext/standard/basic_functions.c Outdated
Comment on lines +603 to +604
AIX accepts IP strings with excedentary 0 (192.168.042.42 will be treated as
192.168.42.42), while Linux don't.
Copy link
Copy Markdown
Contributor

@mvorisek mvorisek Feb 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

If such functionality is expected, it should be tested.

@devnexen
Copy link
Copy Markdown
Member

devnexen commented Mar 11, 2026

Looks like you accidentally swallowed your changes with the last commit. There is a slight indentation issue with the new var declaration. You would probably need to guard the said var to be aix exclusive to avoid unused var build warnings for everything else.

@devnexen
Copy link
Copy Markdown
Member

devnexen commented Mar 11, 2026

Also as @mvorisek mentioned a new test for provability sake would be nice ;)

@reshmavk
Copy link
Copy Markdown
Author

reshmavk commented Mar 12, 2026

There is an existing testcase that covers IPs with leading zeros - ext/standard/tests/network/ip2long_variation2_x64.phpt. This was failing earlier and is now passing with the fix.
Please let me know if any other scenarios need to be covered.
Output of testcase

  1. Without patch
--TEST--
Test ip2long() function : usage variation 2, 64 bit
--SKIPIF--
--FILE--
int(16845579)
int(2130706433)
int(16860999)
int(0)
int(16863569)
int(3232235520)
bool(false)
bool(false)
--EXPECT--
bool(false)
int(2130706433)
bool(false)
int(0)
bool(false)
int(3232235520)
bool(false)
bool(false)
  1. with patch
--TEST--
Test ip2long() function : usage variation 2, 64 bit
--SKIPIF--
--FILE--
bool(false)
int(2130706433)
bool(false)
int(0)
bool(false)
int(3232235520)
bool(false)
bool(false)
--EXPECT--
bool(false)
int(2130706433)
bool(false)
int(0)
bool(false)
int(3232235520)
bool(false)
bool(false)

devnexen
devnexen reviewed Mar 12, 2026
View reviewed changes
ext/standard/basic_functions.c Outdated
RETURN_FALSE;
}
#ifdef _AIX
/*
Copy link
Copy Markdown
Member

@devnexen devnexen Mar 12, 2026

Choose a reason for hiding this comment

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

the diff show indentation misalignment (see the line above).

devnexen
devnexen reviewed Mar 12, 2026
View reviewed changes
ext/standard/basic_functions.c Outdated
RETURN_FALSE;
}
#ifdef _AIX
/*
Copy link
Copy Markdown
Member

@devnexen devnexen Mar 12, 2026

Choose a reason for hiding this comment

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

sorry to annoy you again but the indentation is still off.

Copy link
Copy Markdown
Author

@reshmavk reshmavk Mar 13, 2026

Choose a reason for hiding this comment

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

Thanks for your patience. I’ll update the indentation so that #ifdef and #endif have no indentation, and ensure the lines within that block align with the code around line 600.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devnexen devnexen devnexen left review comments

@bukka bukka Awaiting requested review from bukka bukka is a code owner

+1 more reviewer

@mvorisek mvorisek mvorisek left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Extension: standard

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@reshmavk @devnexen @mvorisek @iluuu1994