Repository navigation
573 - Check URLs are valid before attempting to perform HTTP request - #578
markfullmer wants to merge 11 commits into
Conversation
…minimal test coverage; full test converage for isInvalidUrl() already exists
…binding; Add a getValidUrlIps() wrapper to return an array of valid IPs, relevant for URLs with redirects
|
Hi @markfullmer, thank you for the quick follow-up and for being upfront about how the cURL changes were put together. Several of your ideas are exactly the right direction: pinning the validated IPs with Rewriting your branch felt wrong. I'm preparing a separate PR that builds on your ideas I re-tested
Because the fix point moves (validation in the HTTP dispatcher on the resolved absolute URI, explicit IPv4/IPv6 range tables that behave the same on PHP 7.4 to 8.5, manual redirects inside the dispatcher so oEmbed and adapter calls are covered too), rewriting your branch felt wrong. I'm preparing a separate PR that builds on your ideas, and I'll credit you in it. The JSON-LD part will go in its own smaller PR. I'd really appreciate your review when it's up, and if you'd rather take any part of it forward yourself, I'm happy to coordinate. Thanks again for pushing this forward! |
Purpose
Resolves #573
Implementation
Adds a new
isValidUrl()helper function that:FILTER_VALIDATE_URLdns_get_record()to check if a record existsdns_get_record()fails, trygethostbyname()This helper function is added to Extractor::resolveUri(), adjacent to the check for HTTP URLs.
Finally, the existing
isHttp()function is updated to use a stricter "allow-list" methodology, rejecting anything that does not match the http or https schema, as suggested in #573.Test coverage
Test coverage is added in FunctionsTest. All of its scenarios pass. There are other failing tests, but these are unchanged compared to the failing tests prior to this code change.