Skip to content

Normalize missing DNS server ports to 53 - #124

Open
yankewei wants to merge 1 commit into
amphp:2.xfrom
yankewei:codex/fix-nameserver-port-validation
Open

Normalize missing DNS server ports to 53#124
yankewei wants to merge 1 commit into
amphp:2.xfrom
yankewei:codex/fix-nameserver-port-validation

Conversation

@yankewei

@yankewei yankewei commented Jul 28, 2026

Copy link
Copy Markdown

Summary

  • normalize custom IPv4 and IPv6 nameserver addresses before storing them
  • add the standard DNS port 53 when callers omit a port
  • preserve explicit custom ports and reject malformed IPv4 port values
  • verify the normalized endpoints returned by DnsConfig::getNameservers()

Root cause

DnsConfig used port 53 while validating a nameserver without a port, but discarded that local default and stored the original address. The resolver later passed an incomplete endpoint such as udp://1.1.1.1 to stream_socket_client(), which requires a port.

This change writes the validated, normalized endpoint back into the configuration, so callers can continue omitting the standard DNS port while the socket layer always receives a complete address.

Fixes #123.

Validation

  • vendor/bin/phpunit test/DnsConfigTest.php test/Rfc1035StubResolverTest.php (29 tests, 29 assertions)
  • composer cs
  • vendor/bin/psalm.phar --no-cache
  • GitHub Actions: PHP 8.1–8.5, Windows, and macOS (7/7 jobs passed)

The complete test suite was also run locally. Three tests that depend on the local proxy DNS and timeout behavior still fail in this environment, as they did before this revision; the GitHub Actions matrix passes on all seven PHP and OS jobs.

@yankewei yankewei closed this Jul 29, 2026
@yankewei yankewei changed the title Require ports in custom DNS server addresses Normalize missing DNS server ports to 53 Jul 29, 2026
@yankewei yankewei reopened this Jul 29, 2026
@yankewei
yankewei force-pushed the codex/fix-nameserver-port-validation branch from 47fd387 to 542ad65 Compare July 29, 2026 14:58
@yankewei
yankewei marked this pull request as ready for review July 29, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

DnsConfig: $nameservers must contain port number

1 participant