Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Since the latest update to 8.x-5.0-beta3 if i paste an external link and i save without confirming the no result option the link wont take, but if i click the no result option it will change any capitalized letter to lowercase and breaks the link.
how to reproduce
Scenario One:
- use module as usual
- paste in an (external) URL
- directly hit enter while not selecting the upcomming autosuggest
- anchor link will not get applied in the rich text editor
Scenario Two:
- use module as usual
- paste in an (external) URL with lowercase and uppercase characters
- select the autosuggest saying ('Linkit could not find any suggestions. This URL will be used as is.')
- anchor link will be set
- look the the source code of the riche text
- all uppercase characters will be now lowercase
Problem
Using external URLs with uppercase letters (e.g. url shortener) will not work and lead to broken links.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | uppercase_and_external-2805239-13.patch | 318 bytes | abogomolov |
| #7 | uppercase_and_external-2805239-7.patch | 896 bytes | tassilogroeper |
Comments
Comment #2
anonwell, you have to select the result in the autocomplete field in order to "set it".
Comment #3
wolfhowling CreditAttribution: wolfhowling commentedyes but when i do that it changes all uppercase to lowercase and that breaks the links i have which are case sensitive.
Comment #4
yobottehg CreditAttribution: yobottehg commentedWe also have the problem that everything is lowercased. I will investigate.
Comment #5
anonDoes it make sense to select the external link in the list?
The reason for this is that that now there are more then one field that gets populated when selecting a link in the result list and the reason for that is that I don't wanted to have a "self made uri" inserted in the visible field.
If this doesn't make sense for a lot of users, we might change this behavior.
Comment #6
tassilogroeper CreditAttribution: tassilogroeper commentedComment #7
tassilogroeper CreditAttribution: tassilogroeper at WONDROUS commented@anon yes this is a big problem for us, since linkit will replace the normal anchor modal for us. It is a great tool - so thanks for that.
- In my patch I actually just removed the uppercase from the default fallback. So uppercase links will work.
- But the Submit on enter will not. (to me, this is not a big problem. Only annoying, though)
Actually, I see a bigger security problem here:
Yes, the label in the autosuggest gets escaped in `src/SuggestionManager.php:55` but the actual path not `src/SuggestionManager.php:58`.
So can you think of a way to escape this? PHP `filter_var` using the url filter will not allow telephone numbers or url without a protocol, for example. http://php.net/manual/en/filter.filters.validate.php#110411
Or do you think this is not a problem, since the rich text gets escaped in the final rendering of the page. But what about the rich text in the backend then? can this be used as an attack vector? I' unsure about this one
Comment #8
anonIf you mean the text saved in the database I don't see this is a problem. The output is where the sanitation should be.
Or do I misunderstand you?
Comment #9
tassilogroeper CreditAttribution: tassilogroeper at WONDROUS commentedComment #10
tassilogroeper CreditAttribution: tassilogroeper at WONDROUS commentedHonestly the only attack vector I can think of is:
- after hitting ok in the modal with a url-like JS injected
- the rich text will have this js injected
BUT I just played along with it and was not able to inject it. It was escaped before being injected to the rich text.
It was a gut feeling about the query being directly displayed again. so never mind.
Comment #12
anonI have committed the patch in #7.
Comment #13
abogomolov CreditAttribution: abogomolov commented@anon it's very hard to explain this behavior to the editors. It looks broken. Is it possible to catch the submit even and populate the hidden fields?
Adding the "autoFocus" option can also help.
Comment #14
anon@abogomolov: I agree with you, this might not be the best solution. I would really appreciate some feedback and how we can change this to the better. However, I see this as a new issue. Please, feel free to open a new issue about this.