Problem/Motivation

Protocol-relative URLs are saved by LinkWidget as internal:. LinkWidget::getUserEnteredStringAsUri() checks for URL strings that have no scheme, which is true for protocol-relative URLs, and assumes they are internal.

Steps to reproduce:

  1. Add a Link field to any content type.
  2. Create a new instance of that content type. In the link field, add a protocol-relative URL like //www.google.com.
  3. Check the database table for the Link field that you created. See that the value in the uri column is internal://www.google.com.

Initially this report stated that when viewing the node the link was formatted as being a link back to the current page. But now in D11 the link isn't rendered at all.

Proposed resolution

Make LinkWidget detect and handle protocol-relative URLs.

Remaining tasks

  1. Write an MR.
  2. Review.
  3. Commit.

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#3 evidence.png63.85 KByago elias
#2 vokoscreen-2017-06-26_09-41-01_0-426.gif9.11 MByago elias

Issue fork drupal-2889658

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

Yago Elias created an issue. See original summary.

yago elias’s picture

yago elias commented
StatusFileSize
new vokoscreen-2017-06-26_09-41-01_0-426.gif9.11 MB

evidence

yago elias’s picture

yago elias commented
StatusFileSize
new evidence.png63.85 KB

Log is clean.
evidence2

Version: 8.3.4 » 8.3.x-dev

Core issues are now filed against the dev versions where changes will be made. Document the specific release you are using in your issue comment. More information about choosing a version.

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

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Branches prior to 8.8.x are not supported, and Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should 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: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
quietone’s picture

quietone commented
Issue tags: +Bug Smash Initiative

I tested on Drupal 9.4.x, standard install, and can confirm the problem still exists.

Tagging and adding to #3188110: [Meta] Improve URL validation

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

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should 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.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should 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: 9.5.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. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

dcam’s picture

dcam commented
Title: When using protocol-less url no errors are given and link loops into the node » Handle protocol-relative URLs in LinkWidget
Issue summary: View changes
Issue tags: -ciandt-contrib

The issue still exists in D11, though now the link isn't rendered at all. This is caused by LinkWidget assuming that any URL with no scheme is internal. There is no check for protocol-relative URLs.

dcam’s picture

dcam commented
Issue summary: View changes

riyas_nr made their first commit to this issue’s fork.

riyas_nr opened merge request !12957

riyas_nr’s picture

riyas_nr commented
Status: Active » Needs review

Allowed protocol-relative URLs in Link fields and updated constraints to exempt them from validation errors.
Added test coverage for protocol-relative URLs.
Moving to NR.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
dcam’s picture

dcam commented

The one thing that concerned me was the "typo" cases where someone accidentally starts an internal path with a double slash. I tested these manually:

  • //
    • Before the fix an InvalidArgumentException is caught by LinkTypeConstraintValidator and the validation fails.
    • After the fix the same thing occurs. So there's no change in behavior.
  • //example
    • Before the fix the problem as described in the issue summary occurs.
    • After the fix the URL is treated as external, just like complete URLs.

I've wondered whether there's any benefit to adding assertions for these cases, just so we know if there's any change in behavior. I don't know that there is, so I won't insist on it. But it's something to consider.

I think this is almost ready to go. We just need that MR comment @smustgrave made to be addressed.

dcam’s picture

dcam commented
Issue summary: View changes
riyas_nr’s picture

riyas_nr commented
Status: Needs work » Needs review

Combined conditions and moving to NR.

dcam’s picture

dcam commented

There's a duplicate feature request issue, #2744729: Link field should accept protocol-relative URLs.

dcam’s picture

dcam commented
Related issues: +#2744729: Link field should accept protocol-relative URLs
smustgrave’s picture

smustgrave commented

@dcam should that one be closed for this or vice versa?

dcam’s picture

dcam commented

@dcam should that one be closed for this or vice versa?

I think I was still trying to figure that out. Finding that other issue led to a bit of a rabbit hole of issues. There's a larger problem with external URL validation in general.

dcam’s picture

dcam commented

The other feature request was closed as a duplicate.

dcam’s picture

dcam commented
Status: Needs review » Needs work

I left feedback on the MR.

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.