Check_url marks URIs as safe although in the current code base this is never actually necessary as the output is being using directly in a template or in SafeMarkup::format or t(). Therefore we can just use UrlHelper::stripDangerousProtocols() and let the auto escape magic work.
Also this helps with remove one of the use cases for !raw support in SafeMarkup::format().
This is a bug because strings ending up in the SafeMarkup safe list unnecessarily is wrong.
| Issue category | Bug, because we are doing a SafeMarkup::checkPlain(), which ideally should not be there. This is not output level code. |
|---|---|
| Issue priority | Normal, because this just removes one ::checkPlain() call |
Disruption | No disruption. The URLs are still escaped, so no security problem. For code which passes check_url() results onto template it will be double escaped, but we have a CR for that |
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2545906.2.patch | 5.3 KB | alexpott |
Comments
Comment #2
alexpottThe patch seems quite simple.
Comment #3
dawehnerLooks perfect for me.
Added a beta evaluation and a change record.
Comment #4
alexpottPostponing on #2545972: Remove all code usages SafeMarkup::checkPlain() and rely more on Twig autoescaping as that is adding Html::encodeEntities() that would be used here.
Comment #5
alexpottComment #6
mgiffordComment #9
joelpittetThis got superseded by the colon url placeholder in part and the other the Html::escape() changes.