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

Don't look for IP v6 addresses if no IP v6 support complied into PHP #40821

Closed
wants to merge 1 commit into from

Conversation

walec51
Copy link

@walec51 walec51 commented Oct 8, 2023

Closes #33567

@szaimen szaimen added the 3. to review Waiting for reviews label Oct 8, 2023
@szaimen szaimen added this to the Nextcloud 28 milestone Oct 8, 2023
@szaimen szaimen requested review from ChristophWurst, Altahrim, a team, ArtificialOwl, icewind1991 and nfebe and removed request for a team October 8, 2023 19:07
Copy link
Collaborator

@Altahrim Altahrim left a comment

Choose a reason for hiding this comment

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

Well spotted :)

I think it would be better to avoid AAAA lookup if IPv6 isn't supported.
Something like this on line 78:

$dnsTypes = defined('AF_INET6') 
	? [DNS_A, DNS_AAAA, DNS_CNAME]
	: [DNS_A, DNS_CNAME];

@kesselb
Copy link
Contributor

kesselb commented Oct 9, 2023

The php constant AF_INET6 is only defined when php-sockets is enabled: https://github.com/php/php-src/blob/36b194ba299467f075e5c1ce546164702c56938a/ext/sockets/sockets_arginfo.h#L342

I assume, because the module is enabled by default, that's the case for most system packages, but we don't list sockets as required php module on https://docs.nextcloud.com/server/latest/admin_manual/installation/source_installation.html#prerequisites-for-manual-installation.

If we depend on it, we should add it there as well.

@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@georglauterbach
Copy link
Contributor

georglauterbach commented Feb 21, 2024

What's the progress on this?

In my cluster, coreDNS logs 39200 "AAAA IN <POSTGRES DNS NAME> udp 44 false 512" NOERROR qr,aa,rd 119 0.000177098s every 3 or 4 seconds, which is extremely annoying and, quite frankly, unnecessary.

On a side note: Marking and locking the corresponding issue #33567 as "too heated", when people are simply complaining (without any "heat" involved), does not help anyone. Instead, it shows how little is cared in this case, which, I think, is sad, more than anything else.

This was referenced Mar 12, 2024
@Altahrim
Copy link
Collaborator

We can detect IPv6 with inet_pton.

Following my previous comment it could be:

$dnsTypes = inet_pton('::1') === false
	? [DNS_A, DNS_CNAME]
	: [DNS_A, DNS_AAAA, DNS_CNAME];

This was referenced Mar 18, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: RuntimeException: Unable to check Ipv6. Check that PHP was not compiled with option "disable-ipv6".
7 participants