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

[5.3] Catch dns_get_record possible warnings in validateActiveUrl #15979

Merged
merged 1 commit into from
Oct 18, 2016

Conversation

litvinchuk
Copy link
Contributor

In some cases dns_get_record can throw warnings

  • An unexpected server failure occurred.
  • A temporary server error occurred.
  • DNS Query failed

In some cases dns_get_record can throw warnings
- An unexpected server failure occurred.
- A temporary server error occurred.
- DNS Query failed
@taylorotwell taylorotwell merged commit 6f21736 into laravel:5.3 Oct 18, 2016
@GrahamCampbell GrahamCampbell changed the title Catch dns_get_record possible warnings in validateActiveUrl [5.3] Catch dns_get_record possible warnings in validateActiveUrl Oct 18, 2016
@cyrossignol
Copy link

cyrossignol commented Oct 21, 2016

@litvinchuk @taylorotwell @GrahamCampbell Thanks for the change. Unfortunately, this PR doesn't fix a certain scenario:

If the IPv6 stack isn't configured properly somewhere between the server and the internet, PHP warns, dns_get_record(): Unable to parse DNS data received on line 1, probably because we're attempting to fetch AAAA records as well as A.

As a result, validation fails for valid IPv4 lookups as well.

Arguably, we could suggest that users get their IPv6 working correctly, but this seems unreasonable, especially for applications deployed on an IPv4 intranet. Docker users will likely encounter confusion as well because containers disable IPv6 by default, and nothing in the docs or error hint at this limitation. This may also be the reason why we see some homestead users having problems with active_url validation.

I'm not sure if this warrants a separate issue ticket. Perhaps we can separate the validation checks for IPv4 and IPv6:

if ($url = parse_url($value, PHP_URL_HOST)) {
    try {
        if (count(dns_get_record($url, DNS_A)) > 0) {
            return true;
        }
    } catch (Exception $e) {
    }

    try {
        if (count(dns_get_record($url, DNS_AAAA)) > 0)
            return true;
        }
    } catch (Exception $e) {
    }

    return false;
}

The extra lookup is expensive, but adds flexibility.

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

Successfully merging this pull request may close these issues.

None yet

3 participants