Follow-up to #2297711: Fix HTML escaping due to Twig autoescape
Follow-up to #1825952: Turn on twig autoescape by default
Non-technical explanation
After #2264041: Add a test to ensure title callbacks are not vulnerable to XSS have proven that even battle hardened core developers can't write XSS free code we have introduced #1825952: Turn on twig autoescape by default to fix a torrent of security holes already present in core known and unknown and to avoid the most frequent kind of sechole(Security Hole) in the history of Drupal contrib. However, this has broken some places that were already securely written, resulting in broken layout and HTML tags shown to users. We need to find those places and update them to be compatible with the new method.
Problem/Motivation
Can be tricky to discover the double escaping.
Proposed resolution
@dawehner's and @joelpittet's idea about testing existing checked routes that are already tested for double escaping.
Duckpatch drupalGet/drupalPost methods in simpletest to check for double escaping on pages.
Potentially a more permanent fixture could be possible.
Remaining tasks
Comment | File | Size | Author |
---|---|---|---|
#23 | 2571065.11-reroll.patch | 12.72 KB | joelpittet |
#11 | 2571065.11.patch | 12.77 KB | alexpott |
#2 | find_escaping_due_to-2571065-2.patch | 2.41 KB | joelpittet |
Comments
Comment #2
joelpittetComment #5
dawehnerI know that I had an issue about that too, well, no idea where it is now :)
There have been just a few actual valid failures, most the other ones seems to be real bugs ... wow
Comment #7
joelpittet160 was the count, retesting to see where we are at so far.
Comment #10
joelpittetWould be nice to have no unintentional escaping with D8 release.
Comment #11
alexpottCombines #2597608: User role action labels get double-escaped and out of sync with the corresponding user role label to see what is left...
Comment #13
joelpittet32 to 6 is quite nice!
Comment #14
alexpott@joelpittet it is even better than that! I'd argue we only really have to investigate 2 of the fails...
Comment #15
alexpottDrupal\search\Tests\SearchNodePunctuationTest is fine - it is searching for
&
so it is an expected fail.Comment #16
alexpottCreated #2598502: Double escaping in views attachment titles to fix
Drupal\system\Tests\Theme\EntityFilteringThemeTest
Comment #19
xjmComment #23
joelpittetReroll