Skip to content

Conversation

dilyanpalauzov
Copy link
Contributor

I have this patch since 2018, PHP-7.1. My kernel knows about IPv6, but the system has no IPv6 routing configured and PHP is configured without IPv6 support.

Fixes https://bugs.php.net/bug.php?id=75862 .

@dilyanpalauzov dilyanpalauzov changed the title 7.4 — curl/interface.c: avoid crasing, when PHP is compiled without IPv6 support, and CURL has IPv6 support 7.4 — curl/interface.c: avoid crashing, when PHP is compiled without IPv6 support, and CURL has IPv6 support Nov 23, 2021
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good to me. @derickr, since 7.4.27 is the last regular bug fix release, is this good for you?

Copy link
Member

@derickr derickr left a comment

Choose a reason for hiding this comment

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

LGTM for PHP 7.4 if the error message gets fixed.

@nikic
Copy link
Member

nikic commented Nov 24, 2021

Regarding https://bugs.php.net/bug.php?id=75862: Even if no IPv6 routing is configured, this should never result in a crash, even if PHP is built with IPv6 support. Something else must be wrong.

@dilyanpalauzov
Copy link
Contributor Author

I do not remember the details. Back in 2017 for PHP 7.1.13 this has helped. The configuration was likely PHP is ./configure’d --disable-ipv6 and CURL with IPv6 support, or maybe both without IPv6. I think the DNS resolution returned AAAA-only answer, and this could not be handled.

@dilyanpalauzov
Copy link
Contributor Author

dilyanpalauzov commented Nov 24, 2021

In fact, the original patch at https://bugs.php.net/bug.php?id=75862 does not contain #if !HAVE_IPV6 but uses instead #if !ENABLE_IPV6. As I submitted here #if !ENABLE_IPV6 all tests failed, so I replaced it with #if !HAVE_IPV6. Apparently, HAVE_IPV6 comes from PHP, while ENABLE_IPV6 comes from curl.

@cmb69 cmb69 added the Bug label Nov 29, 2021
@cmb69 cmb69 self-requested a review November 29, 2021 14:53
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Apparently, HAVE_IPV6 comes from PHP, while ENABLE_IPV6 comes from curl.

Ah, right! And as such, I think, we would need to check !ENABLE_IPV6, but that might not be available (when build without curl's config.h, what we at least do on Windows). Then again, why would that even be related to PHP's IPv6 support? Isn't that solely a libcurl issue, and ideally should be resolved there (unless it already has been resolved in the meantime)?

If we need to do something in ext/curl, I think we need to employ curl_version_info, and check whether CURL_VERSION_IPV6 is enabled. That would, of course, be a runtime, and not a compile time check.

…upport, and CURL has IPv6 support

I have this patch since 2018, PHP-7.1.  My kernel knows about IPv6, but the system has no IPv6 routing configured and PHP is configured without IPv6 support.

Fixes https://bugs.php.net/bug.php?id=75862 .
@dilyanpalauzov
Copy link
Contributor Author

I changed ×2 #if !HAVE_IPV6 with #if !ENABLE_IPV6.

@cmb69
Copy link
Member

cmb69 commented Nov 29, 2021

I changed ×2 #if !HAVE_IPV6 with #if !ENABLE_IPV6.

That can't (generally) work. ENABLE_IPV6 may not be defined at all, even though libcurl had been build with IPv6 support.

@dilyanpalauzov
Copy link
Contributor Author

Why do the test fail?

I have reverted on my system the patch and could not bring PHP to crash. I am closing the current case for now, unless I manage to reproduce the crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants