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.
Comment | File | Size | Author |
---|---|---|---|
#7 | 2089351-2.patch | 20.61 KB | meba |
#1 | 2089351-1.patch | 20.59 KB | thedavidmeister |
Comments
Comment #1
thedavidmeister CreditAttribution: thedavidmeister commentedpatch.
Comment #3
thedavidmeister CreditAttribution: thedavidmeister commented#1: 2089351-1.patch queued for re-testing.
Comment #4
thedavidmeister CreditAttribution: thedavidmeister commentedComment #5
jibranI have searched
core/includes
haven't found any other occurrence so RTBC.Comment #6
meba CreditAttribution: meba commentedAdding a Security tag
Comment #7
meba CreditAttribution: meba commentedLooks OK to me, rerolled to apply cleanly
Comment #8
webchickCommitted and pushed to 8.x. Thanks!
Comment #9
thedavidmeister CreditAttribution: thedavidmeister commentedComment #11
thedavidmeister CreditAttribution: thedavidmeister commentedI don't understand why this was tagged with "needs security review".
There are still references to check_plain() in:
- core/includes/menu.inc
Comment #12
kirby14 CreditAttribution: kirby14 commentedOther than comments, this is the only spot, and it isn't calling check_plain();
Comment #13
thedavidmeister CreditAttribution: thedavidmeister commentedHmmm, I see. My bad. That's interesting, could $title_callback be expecting to use String::checkPlain() somehow?
Comment #14
longwaveThis check was first introduced in #212409: Content Type title displays incorrectly, not sure if it is needed any more - maybe it can be safely removed?
Comment #15
thedavidmeister CreditAttribution: thedavidmeister commentedYeah, I think this needs further investigation. I'll take the novice tag off.
Comment #16
JiriK CreditAttribution: JiriK commentedThe piece of code mentioned in comment #12 was removed by #2107533: Remove {menu_router}. So I think this issue is irrelevant now.