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

only return valid IPs #181

Closed
wants to merge 2 commits into from
Closed

Conversation

remicollet
Copy link

On recent linux distro (e.g. Fedora 33+) Network Manager generates /etc/Resolv.conf with interface references

Example:

# Generated by NetworkManager
search remirepo.net
nameserver 192.168.1.1
nameserver fe80::7e94:2aff:fe85:5cce%eno1

Last address is not valid, but not filtered by loadResolvConfBlocking, so create unusable configuration
Ex: running react/socket test suite

There were 8 errors:

1) React\Tests\Socket\ConnectorTest::testConnectorUsesTcpAsDefaultScheme
InvalidArgumentException: Invalid nameserver address given

/usr/share/php/React/Dns/Query/UdpTransportExecutor.php:115
...

This trivial fix filter to only return valid IPs

@WyriHaximus WyriHaximus added this to the v1.7.1 milestone Jun 29, 2021
@clue clue removed this from the v1.7.1 milestone Jul 10, 2021
@clue
Copy link
Member

clue commented Nov 26, 2021

@remicollet Thank you for looking into this and filing this PR!

I've also looked into this and have just filed #187 to apply a slightly different logic. Instead of ignoring the entire nameserver entry, it will only ignore the IPv6 zone ID, so it would at least still fall back to fe80::7e94:2aff:fe85:5cce as a secondary DNS server in your case.

In either case, it will no longer throw an InvalidArgumentException and should work just fine by using the primary DNS server first.

@remicollet remicollet closed this Nov 26, 2021
@remicollet remicollet deleted the issue-validip branch November 26, 2021 14:34
@remicollet
Copy link
Author

nice

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.

None yet

3 participants