search

php / php-src

The PHP Interpreter
https://www.php.net
Other
38.13k stars 7.74k forks source link

Reconsider usages of `gethostbyname_r()` #15531

Open Girgias opened 2 months ago

Girgias commented 2 months ago

Description

According to some sources, this function is obsolete and should be replaced with:

Applications should use getaddrinfo(3), getnameinfo(3), and gai_strerror(3) instead.

Moreover, getting rid of them would allow us to get remove some conditionally defined globals: https://github.com/php/php-src/blob/69d9c12df64e829befd843175bfc9617aabb7450/ext/standard/file.h#L106-L110

Initially found while working on #15511

devnexen commented 2 months ago

yes, in the same vein of the previous inet_aton removals.

NattyNarwhal commented 2 months ago

Looks like we're wrapping both getaddrinfo and gethostbyname_(r) as php_network_getaddresses and php_network_gethostbyname respectively, with the former wrapping the latter if getaddrinfo doesn't exist.

It does seem that all the gethostbyname_r calls get wrapped by php_network_gethostbyname, but getaddrinfo usage in exts/SAPIs is inconsistent if it calls php_network_getaddresses or getaddrinfo directly.

For the functions like gethostbyaddr and gethostbyname PHP functions, they can probably be changed to call the better interface.

NattyNarwhal commented 2 months ago

As for getting rid of gethostbyaddr, it should simplify the build system, because the _r variant is non-standard with different number of parameters. getaddrinfo is in POSIX since 2004 and is in almost everything with an IPv6 stack (i.e. anything modern enough to run PHP on).

NattyNarwhal commented 2 months ago

I'm also realizing that PHP gethostbyname and friends are documented as returning v4 addresses only. We should probably make these return v6 addresses, or preserve the existing behaviour (even with backing function change) and offer a new function based around getaddrinfo that exposes everything it does (edit: or extend the existing interface in next release).

Girgias commented 2 months ago

Hum, looks there is quite a bit of cruft in that area.

Probably creating some new php_* functions, using those consistently, and deprecating the "old" php_* functions is the best step forward?

NattyNarwhal commented 2 months ago

I think php_network_getaddresses is probably OK as-is, and php_network_gethostbyname can act as a wrapper for it instead of the other way around to keep ABI compatibility (then we can remove php_network_gethostbyname in 8.5/9.0).