Fix Str::isUrl() returning false for single-char domain names (#58538)#58686
Conversation
|
The test failing appears unrelated to the code change, must be a flaky test. |
|
Maybe, this would be enough: public static function isUrl($value, array $protocols = [])
{
if (empty($protocols)) {
try {
return Uri::new($value) !== null;
} catch () {
return false;
}
}
// …
}But maybe, this is more like something to be included in #58132 🤔 |
|
@shaedrich what do you think about this approach? All tests pass with it and it hands off validation to League URI instead of doing our massive regex. I believe #58132 will be a larger refactor and land on 13.x branch. public static function isUrl($value, array $protocols = [])
{
if (! is_string($value)) {
return false;
}
try {
$uri = \League\Uri\Uri::new($value);
} catch (\Throwable) {
return false;
}
$scheme = $uri->getScheme();
$host = $uri->getHost();
if ($scheme === null || $host === null || $host === '' || preg_match('/^\.+$/', $host)) {
return false;
}
$protocolList = empty($protocols)
? 'aaa|aaas|about|acap|acct|acd|acr|adiumxtra|adt|afp|afs|aim|amss|android|appdata|apt|ark|attachment|aw|barion|beshare|bitcoin|bitcoincash|blob|bolo|browserext|calculator|callto|cap|cast|casts|chrome|chrome-extension|cid|coap|coap\+tcp|coap\+ws|coaps|coaps\+tcp|coaps\+ws|com-eventbrite-attendee|content|conti|crid|cvs|dab|data|dav|diaspora|dict|did|dis|dlna-playcontainer|dlna-playsingle|dns|dntp|dpp|drm|drop|dtn|dvb|ed2k|elsi|example|facetime|fax|feed|feedready|file|filesystem|finger|first-run-pen-experience|fish|fm|ftp|fuchsia-pkg|geo|gg|git|gizmoproject|go|gopher|graph|gtalk|h323|ham|hcap|hcp|http|https|hxxp|hxxps|hydrazone|iax|icap|icon|im|imap|info|iotdisco|ipn|ipp|ipps|irc|irc6|ircs|iris|iris\.beep|iris\.lwz|iris\.xpc|iris\.xpcs|isostore|itms|jabber|jar|jms|keyparc|lastfm|ldap|ldaps|leaptofrogans|lorawan|lvlt|magnet|mailserver|mailto|maps|market|message|mid|mms|modem|mongodb|moz|ms-access|ms-browser-extension|ms-calculator|ms-drive-to|ms-enrollment|ms-excel|ms-eyecontrolspeech|ms-gamebarservices|ms-gamingoverlay|ms-getoffice|ms-help|ms-infopath|ms-inputapp|ms-lockscreencomponent-config|ms-media-stream-id|ms-mixedrealitycapture|ms-mobileplans|ms-officeapp|ms-people|ms-project|ms-powerpoint|ms-publisher|ms-restoretabcompanion|ms-screenclip|ms-screensketch|ms-search|ms-search-repair|ms-secondary-screen-controller|ms-secondary-screen-setup|ms-settings|ms-settings-airplanemode|ms-settings-bluetooth|ms-settings-camera|ms-settings-cellular|ms-settings-cloudstorage|ms-settings-connectabledevices|ms-settings-displays-topology|ms-settings-emailandaccounts|ms-settings-language|ms-settings-location|ms-settings-lock|ms-settings-nfctransactions|ms-settings-notifications|ms-settings-power|ms-settings-privacy|ms-settings-proximity|ms-settings-screenrotation|ms-settings-wifi|ms-settings-workplace|ms-spd|ms-sttoverlay|ms-transit-to|ms-useractivityset|ms-virtualtouchpad|ms-visio|ms-walk-to|ms-whiteboard|ms-whiteboard-cmd|ms-word|msnim|msrp|msrps|mss|mtqp|mumble|mupdate|mvn|news|nfs|ni|nih|nntp|notes|ocf|oid|onenote|onenote-cmd|opaquelocktoken|openpgp4fpr|pack|palm|paparazzi|payto|pkcs11|platform|pop|pres|prospero|proxy|pwid|psyc|pttp|qb|query|redis|rediss|reload|res|resource|rmi|rsync|rtmfp|rtmp|rtsp|rtsps|rtspu|s3|secondlife|service|session|sftp|sgn|shttp|sieve|simpleledger|sip|sips|skype|smb|sms|smtp|snews|snmp|soap\.beep|soap\.beeps|soldat|spiffe|spotify|ssh|steam|stun|stuns|submit|svn|tag|teamspeak|tel|teliaeid|telnet|tftp|tg|things|thismessage|tip|tn3270|tool|ts3server|turn|turns|tv|udp|unreal|urn|ut2004|v-event|vemmi|ventrilo|videotex|vnc|view-source|wais|webcal|wpid|ws|wss|wtai|wyciwyg|xcon|xcon-userid|xfire|xmlrpc\.beep|xmlrpc\.beeps|xmpp|xri|ymsgr|z39\.50|z39\.50r|z39\.50s'
: implode('|', $protocols);
return preg_match('/^(' . $protocolList . ')$/i', $scheme) > 0;
} |
|
I would use |
|
Alright @shaedrich, I thin we ran this as far as we can using We could go through the trouble of removing the tests that should fail in theory, however this constitutes a breaking change and would have down stream implications for anyone using the existing code. I think we have to just go the regex route for this PR and solve the greater issue in the #58132 PR. I am going to revert the changes for now and we can focus on the regex implementation unless you have any other further ideas. |
This reverts commit 94c1e9d.
|
Yeah, not changing this entirely should be better on a patch release for now |
Fixes #58538
The regex in
Str::isUrl()required a TLD-like suffix after the hostname, causing single-character domain names (e.g.http://l:8000) to fail validation. This cuased errors when using single letter/etc/hostsaliases for local development.This updates the domain regex to match Symfony's
UrlValidator(symfony/symfony#43876), this splits domain matching into three types: punycode domains, multi-level domains, and single-level domains.