Not all URIs are URLs, but UriWidget won't accept non-URL URIs

Created on 5 December 2020, over 3 years ago
Updated 29 February 2024, 4 months ago

Problem/Motivation

Meta issue: Drupal's nomenclature and code blurs the line between URIs and URLs in such a way that pretty much always assumes all URIs are URLs.

Example: The UriWidget provides a field widget for the uri entity field type. While the docblock on the field type references RFC 2616, this is actually the HTTP/1.1 spec; the applicable spec for URIs (of which URLs are a specific type) is RFC 3986.

Or, consider the case of simple_oauth (I'm the junior maintainer now) which adds a uri field on to the consumer entity for purposes of registering a redirect_uri value for OAuth 2.0. This value is often - but not always - a URL, but in the case of apps might look like my.app:/oauthredirect (a valid URI resolved by the phone's OS/middleware.)

In core, the core Link module has a uri property but stores only URLs and this makes sense in context.

(There are some internal URIs (e.g., internal://) that can be converted to URLs, and in this case the nomenclature is correct. E.g., Url::fromUri())

Proposed resolution

Clearly the 99% case here is that in practice URL == URI for most all users. However, this amounts to a bit of DX false advertising when a developer might reasonably assume that UriWidget accepts URIs. Consider however the url form element calls UrlHelper::isValid() for validation, which checks only a subset of RFC 3986. (It actually also broadens the validation to restirct to only http/https/feed.)

(It's worth noting this is some of the oldest code in Drupal; I traced the genesis of the inclusion of feed as a scheme to Dries in 2010!)

URLs and URL generation are important to web sites and this is definitely one area in which we have to consider BC. That said, it would also be good/appropriate for Drupal to treat URLs and URIs as per the spec and not create unnecessary developer WTF conditions when you really, actually want a URI.

There's no particular reason why developers who actually want to store a URI in a uri field can't change the element validation callbacks for the form element in an alter. I propose adding a validation function (most of the examples around the web speak to parsing, which is not necessary here) and also cleaning up a few of the more confusing docblocks that intermix URL and URI terminology.

Remaining tasks

Decide if this is a bigger issue we want to tackle (probably not/maybe only for Drupal 10?) and hone in on the DX components that are actionable and don't break BC.

User interface changes

None

API changes

None unless we want to tackle this e.g. by deprecating/renaming to be more consistent in Drupal 10.

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Field 

Last updated about 17 hours ago

Created by

🇺🇸United States bradjones1 Digital Nomad Life

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.

  • Merge request !101Update plugin docblocks → (Open) created by bradjones1
  • 🇺🇸United States smustgrave

    Can the MR be updated for 10.1 please

    As far as your question in #11 maybe the #contribute channel could give some feedback? Or #bugsmash?

  • First commit to issue fork.
  • 🇺🇸United States bradjones1 Digital Nomad Life

    @sourabh_jain_419 can you explain what you're doing?

  • 🇮🇳India sourabhjain

    @bradjones I have rebase the branch.

  • 🇺🇸United States smustgrave

    FYI rebasing the branch but not adding anything does not get credit.

  • 🇺🇸United States bradjones1 Digital Nomad Life

    Also you rebased on 9.2 which is not helpful here.

  • 🇺🇸United States bradjones1 Digital Nomad Life

    This is also pertinent to 🐛 `LinkCollection::validKey()` does not properly validate RFC 8288 link relation types Active which also needs "true" URI validation.

  • 🇩🇪Germany tstoeckler Essen, Germany

    Thanks for the patch! Didn't review it, but can verify it works. Hit a bug with Diff module with this, though, that it fails to diff non-URL URIs that are created with this patch.

    Created a little diff plugin for that, attaching that in case this helps anyone. Once this lands, will open an issue for that over in Diff module. To be placed in src/Plugin/diff/Field for you custom module (removing the _.txt extension) and then just replace the module name in the namespace.

  • Pipeline finished with Success
    3 months ago
    Total: 186s
    #138542
  • Pipeline finished with Success
    3 months ago
    Total: 216s
    #138554
  • Pipeline finished with Failed
    2 months ago
    #142716
  • Pipeline finished with Canceled
    2 months ago
    Total: 61s
    #142723
  • Pipeline finished with Failed
    2 months ago
    Total: 416s
    #142729
  • Pipeline finished with Failed
    2 months ago
    Total: 412s
    #142749
  • Pipeline finished with Canceled
    2 months ago
    Total: 32s
    #142753
  • Pipeline finished with Failed
    2 months ago
    Total: 225s
    #142755
  • Pipeline finished with Success
    2 months ago
    Total: 417s
    #142758
  • Pipeline finished with Failed
    about 1 month ago
    Total: 213s
    #171880
Production build 0.69.0 2024