Insufficient link validation for external URLs in link widget

Created on 18 January 2016, over 9 years ago
Updated 29 July 2024, about 1 year ago

Problem/Motivation

The link widget seems to rely purely on native browser side validation for checking the validity of external URLs. When an invalid URL such as "http:" (on Firefox) or "irc:" (on Chromium and Firefox) is used then these malformed URLs are accepted.

Steps to replicate:

  1. Add a link field on the "Article" node type with the option "Allowed link type" set to "External links only".
  2. Create an article, enter "http:" or "irc:" for the URL, and submit the form.
  3. Result: the invalid URL is accepted.

This was originally reported by idimopoulos .

Proposed resolution

There are two proposals
1) Add validation for punycode and magnet links in /core/modules/link/src/Plugin/Validation/Constraint/LinkExternalProtocolsConstraintValidator
or
2) Use the Symfony Url Validator, #34 🐛 Insufficient link validation for external URLs in link widget Needs work . This was proposed 6 years ago in #295021: filter_var() with FILTER_VALIDATE_URL accepts malformed URLs and rejects not all valid URLs and 4 years ago in #2691099: Improve external URL validation in many ways

Remaining tasks

Choose a proposed resolution and if the 1) then decide if these changes should be in UrlHelper See #21 🐛 Insufficient link validation for external URLs in link widget Needs work

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Link 

Last updated 8 days ago

Created by

🇧🇬Bulgaria pfrenssen Sofia

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇮🇳India uddeshy2

    adding patch working patch for drupal 10.3.

  • Status changed to Needs review about 1 year ago
  • Status changed to Needs work about 1 year ago
  • The Needs Review Queue Bot tested this issue.

    While you are making the above changes, we recommend that you convert this patch to a merge request . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

  • 🇺🇸United States dcam

    I closed 🐛 Link field doesn't sufficiently validate input Needs work as a duplicate. I updated the issue summary with info uncovered by that issue.

  • First commit to issue fork.
  • Pipeline finished with Failed
    26 days ago
    #576369
  • Pipeline finished with Failed
    26 days ago
    #576383
  • Pipeline finished with Running
    26 days ago
    #576434
  • In the previous patch the URL constraints failed because the value was passed directly to the parent validator.

    Updates in this patch:

    1. Created a Symfony Url constraint object and passed it to the parent validate().
    2. Added allowed protocols and used the child constraint message.
    3. Applied the constraint only for external URLs.
    4. Added test coverage for invalid characters, multiple comma-separated URLs, and other edge cases.

    This ensures external links are properly validated and the behavior is covered by tests.

  • 🇺🇸United States dcam

    @riyas_nr I haven't done an in-depth review of the MR yet, but I notice we're adding a condition for mailto: URLs. There's no test coverage for it though. Is it possible to add a couple of test cases to LinkItemUrlValidationTest::getTestLinks() for them?

  • Added test coverage for mailto scheme. Moving to NR.

  • Pipeline finished with Failed
    25 days ago
    Total: 1495s
    #576633
Production build 0.71.5 2024