On node_edit screen suffix shows as it defined (ex. °C) but when node is viewed it looks like after check_plain()

Is it normal or should be fixed?

CommentFileSizeAuthor
#10 number-prefix-356908-10.patch2.08 KBbjaspan
#6 number_prefix-356908-6.patch2.05 KByched
#4 number_d7.patch1.38 KBandypost
#1 number_c_p_d6.patch1.06 KBandypost

Comments

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new number_c_p_d6.patch1.06 KB

Suppose there's no need in check_plain because only developers allowed to edit prefix and suffix
Widget for editing is not escaped by visible by users

/**
 * Proxy theme function for number field formatters.
 */
function theme_number_formatter_generic($element) {
//skip
-  $prefixes = isset($field['prefix']) ? explode('|', check_plain($field['prefix'])) : array(0 => '');
-  $suffixes = isset($field['suffix']) ? explode('|', check_plain($field['suffix'])) : array(0 => '');
+  $prefixes = isset($field['prefix']) ? explode('|', $field['prefix']) : array(0 => '');
+  $suffixes = isset($field['suffix']) ? explode('|', $field['suffix']) : array(0 => '');
andypost’s picture

andypost
he/him
Primary language Russian
commented

can someone review this patch?

andypost’s picture

andypost
he/him
Primary language Russian
commented
Project: Content Construction Kit (CCK) » Drupal core
Version: 6.x-2.x-dev » 7.x-dev
Component: General » field system
Category: support » bug
Status: Needs review » Patch (to be ported)

First update 7-version then backport

but this issue better move to drupal project

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new number_d7.patch1.38 KB

Status: Needs review » Needs work

The last submitted patch failed testing.

yched’s picture

yched commented
Status: Needs work » Needs review
StatusFileSize
new number_prefix-356908-6.patch2.05 KB

Fixed in CCK.

The answer is of course not to display admin-entered text unfiltered, but to apply consistent filtering - we can be more permissive than a strict check_plain, though.
Attached patch runs prefix and suffix through field_filter_xss()

andypost’s picture

andypost
he/him
Primary language Russian
commented

Thanx for patching cck!

Is there a reason to check prefix and suffix if edit them only available for admin (developer)?

yched’s picture

yched commented

Yes. User with 'administer content types' perm != developer. You don't know what role assignments people use out there. *No* user input goes out unfiltered, that's e general rule.
Note that http://drupal.org/node/372836 specifically advises site admins to grant 'administer content types' perm with care.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Patch applies by hand - but works fine on d7
thanks

bjaspan’s picture

bjaspan commented
StatusFileSize
new number-prefix-356908-10.patch2.08 KB

I don't understand how the patch works for andypost in #9 because it does not actually remove the check_plain call which causes the problem. New patch attached that does. I'll call this "RTBC except I want yched to verify my change."

andypost’s picture

andypost
he/him
Primary language Russian
commented

@bjaspan you right about HEAD - patch #6 with typo, yched just forget to clean check_plain

but as http://drupal.org/cvs?commit=189120 code is same as you provide

Thanks for review

yched’s picture

yched commented
Status: Needs review » Reviewed & tested by the community

Right, I made a mistake while transposing the D6 fix, sorry. #10 is RTBC.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed, thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.