Add support for additional protocols in Link field definition

Postponing feature requests as of the allowed beta changes https://www.drupal.org/contribute/core/beta-changes

Drupal 8-beta 1 has been released.

Drupal 8-beta 1 has been released.

Actually we are on Beta 15 now. Please read the policy at https://www.drupal.org/contribute/core/beta-changes

Yup.. I was just going on why this was postponed initially. Totally good to move this to 8.1 and postpone it as something to tackle there.

I'm currently working on a solution to allow for a custom protocol whitelist, specified in the field settings. There won't be any custom validation just yet. I'd suggest handling that in a follow-up issue.

I updated the title as per my previous comment.

Even better would be a option in the field configuration to set which scheme names are valid for this field.

This patch adds a new configuration option that takes a comma-separated list of additional protocols, which are added to the default whitelist.

Looks like I forgot to add the new configurable to the schema.

Updated IS (while waiting for test results).

Ensured that the whitelist is actually an array if unset.

Hopefully this does the trick. I updated some expected test results, and also moved some new code to its own method to make things cleaner.

This adds the extra blank line I missed.

I can't fix the build failure, but assuming that clears up on its own, we should now be left with the Drupal\Tests\link\Functional\LinkFieldUITest::testFieldUI error:

The string "Saved link 1 16 0 1 configuration." was not found anywhere in the HTML response of the current page.

Anyone have any ideas?

Through my own testing, the above patches won't work because the storage is assuming a string, when it should actually be an array.

1. whitelist -> allowlist
2. May need a reroll for D9 if the retest fails.

Re-rolling for 9.1.x and handling #32.1

Triggering the tests

Missed a few instances

Here is the patch from #35 rebased onto 8.9.x and 9.0.x (no interdiff because no changes).

I have an issue with #35, it does not apply to Drupal 9.1.5. Added fixes to the #35 patch for support Drupal 9.1.5.

A re-rolled patch against 9.2.x-dev.

The patch in #39 works as expected and allows me to add additional protocols as part of the field configuration.

Marking as RTBC, as this successfully applies and works as requested in the original ticket description.

Sorry, this is still work to do here.

The Issue Summary and the tags state that this needs tests and I do not see a test of the new schema property, 'protocol_allowlist'. The patch in #39 is for 9.2.x instead of 9.3.x and it is not passing the commit code check.

The new property name 'protocol_allowlist' is a unusual, all the other instances in core of something that is allowed begins with the word allow. Therefor I think the name should be 'allowed_protocols'. And this change will require an Issue Summary update.

1. +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
   @@ -206,11 +206,13 @@ public static function encodePath($path) {
   +   *   A list of extra protocols (strings) that have been explicitly allowed.
   

   s/array/string[]/

2. +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
   @@ -206,11 +206,13 @@ public static function encodePath($path) {
   +   * @param array $protocol_allowlist
   

   s/array $protocol_allowlist /string[] $allowed_protocols/

   There are many, many places in the patch where $protocol_allowlist should be changed.

3. +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
   @@ -206,11 +206,13 @@ public static function encodePath($path) {
   +   *   A list of extra protocols (strings) that have been explicitly allowed.
   

   I think custom is better than extra and there is no need to add strings, the hint is above.

   'The list of custom allowed protocols'.

4. +++ b/core/lib/Drupal/Component/Utility/UrlHelper.php
   @@ -344,13 +346,15 @@ public static function setAllowedProtocols(array $protocols = []) {
       * @see \Drupal\Component\Utility\Html::escape()
   

   As above

5. +++ b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php
   @@ -112,6 +122,25 @@ public function fieldSettingsForm(array $form, FormStateInterface $form_state) {
   +   * Element validate; ensure that the Protocol Allowlist setting is valid.
   

   This feels clumsy. How about, "Validate the format of the allowed_protocol list.'

   It would probably be a good idea to add a sentence that this is not validating the protocols. The user can enter meaningless strings here.

6. +++ b/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php
   @@ -112,6 +122,25 @@ public function fieldSettingsForm(array $form, FormStateInterface $form_state) {
   +  public static function validateSettingsFormElementProtocolAllowlist($element, FormStateInterface $form_state, $form) {
   

   Not convinced the comment is needed here and the code can be simpler. One option is

       $allowed_protocols = array_map('trim', explode(',', $element['#value']));
       $allowed_protocols = $allowed_protocols == [''] ? [] : $allowed_protocols;
       $form_state->setValueForElement($element, $allowed_protocols);

This also changes the user interface for the link field settings so a screenshot before and after is needed. The screenshots needs to be added to the Issue Summary to help reviewers and committers, thanks.

Updated the patch in #39 to address all the changes provided in #43. Also fixed the custom command failure errors shown in #39. Kindly review the changes made in this patch.

Thank you!

Fixed custom command failed for unknown word. patch added and interdiff attached. Please review

The CSpell check failed in core/modules/link/src/Plugin/Validation/Constraint/LinkExternalProtocolsConstraintValidator.php file, so updated the patch accordingly.

Thank you!

Patch from #47 doesn't apply to D9.4 (due to the smallest change in Umani demo config files).

This patch works for 9.4. Haven't tested with 9.5 but suspect it works there as well.

Seems to have a test failure

Left some comments on MR.

Previously tagged for issue summary and screenshots.

Leaving tests tag as they will be needed for post_update hook.

Rolled a patch for 10.2.5 from merge request 6038

clean for 10.2.5

• Updated Issue Summary.
• Rebased the MR over the most recent 11.x branch.
• Resolved all GitLab threads.
• Added a post_update hook and a test for it.

The MR Gitlab pipeline is failing and I believe it has to do with some unrelated problem in the 11.x branch. Not setting this issue to "Needs Review" until this is solved.

Just figured out that the failing of:

• Drupal\Tests\package_manager\Build\PackageInstallTest
• Drupal\Tests\package_manager\Build\PackageInstallSubmoduleTest
• Drupal\Tests\package_manager\Build\PackageUpdateTest

with the error message
Some modules have database schema updates to install. You should run the database update script immediately
was caused by the incorrect placement of the post_update hook function in the link.module file instead of the required link.post_update.php file.

Left some comments on MR.

Hiding patches

Addressed :)

This patch applies to Drupal 10.4.2 ok, but I want to use a link field to store links to files such as public:///interactive/zoomify/mytiles and I get "The URI 'public:///interactive/zoomify/mytiles' is invalid. You must use a valid URI scheme. Use base: for a path, e.g., to a Drupal file that needs the base path. Do not use this for internal paths controlled by Drupal."

Should I be able to set Allowed protocols to public, private? If so, there may be more work required. If not, this should be made clear in the issue and UI?

@jonathan_hunt, if you add the "public" protocol in a link field configuration (after applying the changes in this issue fork: https://git.drupalcode.org/issue/drupal-2031149) you will be able to add links like public://whatever/you/want/file.pdf, but those URIs will not be converted to an URL. You will have to process them in a custom hook.

Maybe the module Linkit (https://www.drupal.org/project/linkit) would address what you want? Although a relative path would be stored in the field, instead of a public:// style URI, the public directory is not likely to change, so it could be enough for you, depending on the case.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Rebased the MR :)

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Rebased again

patch file for D10.4.6 , upgrading a D10.3.x site for a client

Hello , I have tested the MR on Drupal 11.0.x and it is working as expected.
Steps:
1. Added a link field to article content type.
2. Added Allowed protocols field value as "mailto, ftp"
3. Created a article and added value to the link URL field mailto:example@example.com
4. Added a field label value 'Mail this to'.

Added screenshots for reference

Left a comment in the MR.

Rebased against 11.x and converted to a functional test that calls ->runUpdates().