Problem/Motivation
Currently, In default link field we have URL and Link text.
URL field accepts multiple URLs, which leads to creating broken URL in the node view page.
e.g. I can add 2 urls same time https://www.google.com, https://www.google.in
and in node view, it comes as https://www.google.com%2C%20https//www.google.in
See attached images:
Steps to reproduce
- Add a Link Field to a content type allow one value
- Add a node of that content type and in the Link field add two Urls separated by a comma, for example, https://www.google.com, https://www.google.in.
- Save - notice no errors
Proposed resolution
Validate the field using UrlElement::validateUrl()
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | interdiff_20-21.txt | 939 bytes | ridhimaabrol24 |
| #21 | 2935307-21.patch | 3.41 KB | ridhimaabrol24 |
| #20 | 2935307-20.patch | 3.33 KB | ridhimaabrol24 |
| #19 | link_field_doesn_t_s-2935307-19.patch | 3.7 KB | logickal |
| #17 | link_field_validation-2935307-17.patch | 3.2 KB | logickal |
Issue fork drupal-2935307
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:
Comments
Comment #2
n.kishorekumar commented@chintan4u
i think the way you added multiple url is not right
For adding multiple URLs,
while creating field in the content type we should select unlimited values in the field settings
Comment #3
chintan4u commentedI know that :)
My point is we need to validate the URL field for Comma or Space so only one URL value should be accepted and when a user just copy-pastes multiple URLs in the same text-box it should throw an error.
So a user will understand for multiple URLs he needs to add multiple URL fields.
Currently, It accepts the multiple URLs without any warning or error, Which leads to broken URL.
Comment #4
cilefen commentedComment #5
chintan4u commentedComment #6
chintan4u commentedValidation patch added.
Comment #8
chintan4u commentedComment #9
sanduhrsThe LinkWidget apparently overrides '#element_validate' of the url form element defined in \Drupal\Core\Render\Element\Url.
The attached patch readds it, please review.
Comment #12
esmoves commentedHi, I'm at the contributionday and looking to do Major issue triage on this issue.
Comment #13
mradcliffe@esmoves, @ChrisDarke and I are learning #2474049: [meta] Major issue triage together.
Comment #14
esmoves commentedConfirmed Priority Major for this issue.
No duplicate issues found.
Comment #16
gmercer commentedWe found a small issue related to patch #9 regarding fields that support internal links. They were incorrectly being marked as invalid during validation. This patch expands on what was done in patch #9, but accounts for internal links when setting '#element_validate'.
Comment #17
logickal commentedWe ran into this recently with a client using fields configured to accept both internal and external links. A user entered a malformed URL and Drupal accepted it with no issues, breaking pages on the site. Found this issue and tried these patches but while they seem to be headed in the right direction, didn't actually approach the issue we found.
Here is a first attempt to get validation working for internal/external links in the same field, but could obviously be improved.
Comment #19
logickal commentedFound that the previous approach was failing to save internal links entered with a leading slash, which should also work in this use case. Leaving this as Needs Work since the same tests will likely fail. I will try to revisit this soon with test work.
Comment #20
ridhimaabrol24 commentedPatch #19 failed to apply. Re rolling the patch for 9.1.x
Comment #21
ridhimaabrol24 commentedFixing PHP lint error
Comment #22
quietone commentedCame here to see if this is the same as #2652236: Insufficient link validation for external URLs in link widget and appears it is not. The other issue is about validating the URL but this is that the URL itself get munged. Reading the issue (the IS and #14) suggests that this only happens for multi value fields. Is that true?
It would help if the IS was updated to clarify when this problem happens and to also include steps to reproduce. And, of course, this will need tests so adding tag for that.
To assist reviewer remember when rerolling patches to add an interdiff, or a diff if the interdiff fails. Thanks!
Comment #24
logickal commentedI agree that this needs some further issue replication testing. I'm trying to take a couple of steps back and evaluated the various URL validation issues and try to get some clarity around this. What it DOES look like is that it is specifically impacting fields configured to support both Internal and External URLs.
I would also add a caveat to my submitted patches - not only has upstream changed ahead of where I was working (thanks @ridhimaabrol24 for the re-roll) but we found that my patches then broke submission of internal links prefixed with a single slash such as /node/add, so this obviously needs a different approach.
Comment #26
quietone commentedAdding this to a meta about link and url validation. #3188110: [Meta] Improve URL validation. Not making it a child at this time.
Comment #27
quietone commentedComment #31
xjmComment #35
dcam commentedI did additional testing.
If the URL input accepts a second URL, then I think the natural next step is to wonder if it accepts any input after the URL. It does! Try entering any garbage text after a URL. It can be saved with no problem. So this issue isn't specifically about a "second URL."
Then it follows that one should wonder "How much invalid input does this actually accept?" It turns out that you can enter anything after the URL scheme and it will pass validation.
All this means that @quietone's instincts were initially correct and this issue is a duplicate of #2652236: Insufficient link validation for external URLs in link widget.
Comment #37
dcam commentedGranting credit to the major issue triage contributors from Amsterdam 2019.