Problem/Motivation
While working in some issues I realized the need to improve UrlHelper::isValid()( in the file ./core/lib/Drupal/Component/Utility/UrlHelper.php
The issues I've found:
- Punycode (International Domain Names) URLs cannot be validated
- It does not accept protocol-relative URLs
- It does not take in account the allowed protocols because it does not use
UrlHelper::getAllowedProtocols() - It uses custom regex, while we could use Symfony
UrlValidatorwhich has a similar regex
Currently the Symfony UrlValidator also has some issues:
- It depends of context, which is not available at
UrlHelper::isValid() - Its code has some bugs displaying the exception's messages
Proposed resolution
- Fix Punycode validation - We could use a library for it or just PHP intl' extension with the function idn_to_ascii()
- Accept protocol-relative URLs - currently being fixed in another issue, but fully rewriting this
UrlHelper::isValid()(maybe would be a better solution: #1783278: Scheme-relative URL rejected by validation - Validate with the allowed protocols by using
UrlHelper::getAllowedProtocols() - Check the possibility of fixing/using Symfony
UrlValidator: http://symfony.com/doc/current/reference/constraints/Url.html
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | improve_external_url-2691099-5.patch | 3.79 KB | casaran |
| #4 | improve_external_url-2691099-4.patch | 3.75 KB | pmchristensen |
Comments
Comment #4
pmchristensen commentedThanks for your awesome work Mac_Weber
As part of reviewing your patch for #2652236: Insufficient link validation for external URLs I realized that my problem was related to the formElement url and not the renderElement link.
Taken your description into consideration and your own patch for Issue 2652236, I made a patch which address most of the description - haven't checked Symfony
UrlValidator.I'll put this issue into Needs work as I think we should look into using a library as you suggest. But now we have something to start from and the possibility to use UrlHelper and get punycode validation for the host/domain.
Comment #6
dawehnerLet's see first whether we have some test failures.
Comment #8
wolffereast commentedOne note on the protocol handling in the initial patch. If we override the protocols by calling
UrlHelper::setAllowedProtocols(['ftp', 'http', 'https', 'feed']);immediately before doing the validation it trumps any changes made to the protocol list by external calls to setAllowedProtocols. Is allowing external calls to setAllowedProtocols a security risk? If so then we should protect the method, and if not then I propose rerolling the patch with the call to setAllowedProtocols removed.Comment #14
colanComment #17
wombatbuddy commentedAlso, I found out that you can enter url several times, for instance like this:
https://www.facebook.com/https://www.facebook.com/
Comment #18
quietone commentedRemoved related issue that is listed twice
Comment #24
casaran commentedPatch for 10.2.5 if anyone is interested.