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 UrlValidator which 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
  • https://github.com/symfony/symfony/issues/65374 - fails for encoded domains
  • https://github.com/symfony/symfony/issues/65375 - fails if URL contains 2 hashes, but to work around it, we can safely strip out hashes before passing it to the validator since the current regex doesn't try to match fragments anyway (this has been merged, requires a release that core can target)

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#24 improve_external_url-2691099-5.patch3.79 KBcasaran
#4 improve_external_url-2691099-4.patch3.75 KBpmchristensen

Issue fork drupal-2691099

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Mac_Weber created an issue. See original summary.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

pmchristensen’s picture

pmchristensen commented
Version: 8.2.x-dev » 8.3.x-dev
Status: Active » Needs work
StatusFileSize
new improve_external_url-2691099-4.patch3.75 KB

Thanks 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.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review

Let's see first whether we have some test failures.

Status: Needs review » Needs work

The last submitted patch, 4: improve_external_url-2691099-4.patch, failed testing.

wolffereast’s picture

wolffereast commented

One 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.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
commented
Related issues: +#2031149: Add support for additional protocols in Link field definition

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

wombatbuddy’s picture

wombatbuddy commented

Also, I found out that you can enter url several times, for instance like this:
https://www.facebook.com/https://www.facebook.com/

quietone’s picture

quietone commented
Related issues:

Removed related issue that is listed twice

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

casaran’s picture

casaran commented
StatusFileSize
new improve_external_url-2691099-5.patch3.79 KB

Patch for 10.2.5 if anyone is interested.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

codebymikey’s picture

codebymikey commented
Issue summary: View changes

From some local testing, using the UrlValidator approach works with handling most cases by switching the logic to:

$url_constraint = new Url(
  protocols: ['ftp', 'http', 'https', 'feed'],
);
// Ignore fragments as they are not relevant to the test and not fully supported at the moment.
$url = explode('#', $url)[0];
/** @var \Symfony\Component\Validator\ConstraintViolationList $violations */
$violations = Validation::createValidator()->validate($url, [$url_constraint]);
return $violations->count() === 0;

But currently fails if the domain portion of the domain is encoded.

Relevant upstream issues:

  1. https://github.com/symfony/symfony/issues/65374 - fails for encoded domains
  2. https://github.com/symfony/symfony/issues/65375 - fails if URL contains 2 hashes, but to work around it, we can safely strip out hashes before passing it to the validator since the current regex doesn't try to match fragments anyway

So it might be easier to update the regex rather than relying on an external library, or alternatively use parse_url() to validate that the URL has a valid scheme etc.

codebymikey opened merge request !16752

codebymikey’s picture

codebymikey commented
Issue summary: View changes
Status: Needs work » Needs review

Switched to using Symfony's URL validator for this, so it better supports international domains better.

Tests seem to pass for me locally, but not sure what's going on with CI. Is anyone familiar if Drupal Components need special handling or something as I can't access the composer job artifacts to inspect?