Closed (outdated)
Project:
Drupal core
Version:
7.x-dev
Component:
field system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
21 Mar 2009 at 16:07 UTC
Updated:
9 Aug 2015 at 18:09 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
karens commentedPretty much a no-brainer, needed in D6 and needed here. Are we deliberately being obtuse in the description? Should we mention that it is necessary for security reasons?
Comment #2
yched commentedThis specific change is actually not security related, Selects display their options as is, without interpreting HTML or encoded entities, so this patch just makes sure all incoming HTML is removed. Just cosmetic, if you wish.
The actual sec fixes went in user_ref and node_ref HEAD (which are currently still in CCK contrib).
Comment #3
bjaspan commentedI improved the comment to include the information I needed to convince myself the patch is correct. I did not change the code, so RTBC.
Comment #4
bjaspan commentedComment #5
cburschkaI understand the rationale for decoding these entities, but what kind of HTML tags are being stripped? Where do they originate and why don't we want them?
Patch looks ready, though.
Comment #6
yched commented- the custom list of allowed values for list fields can include HTML, which will be displayed normally (after filter_xss sanitization) by the radios / checkboxes widgets. The select widget, however, needs to filter this HTML out.
- some field types (nodereference, userreference) use this widget with Views-generated data. which typically includes divs / spans wrapping Views fields.
Comment #7
bjaspan commentedI think yched's explanation in #6 is even more clarifying that the comment in the patch before, so I integrated. Still no code changes, so still RTBC.
Comment #8
webchickTalked this over with Barry on IRC, and we agreed a test for this would be good. It looks like our test coverage for fields outside of text.module is a bit spotty.
Comment #9
webchick.
Comment #10
Island Usurper commentedI have a hard time seeing how this can be tested in core. As far as I can see, and I'm not that familiar with fields to see far, it looks like some of CCK's functionality is needed to create a list field and instance programmatically. In particular, there isn't a way to store the allowed values except in CCK's database table. Without knowing what values are listed in the widget, you can't figure out what the default is or even post one of them as data.
My intuition tells me it should be possible to just say
$instance['default_values'], but I don't see any evidence that it's actually used.Comment #11
cosmicdreams commented#7: field-select-widget-409440-7.patch queued for re-testing.
Comment #14
damienwhaley commentedI'd like to take another pass at this as I recently stumbled across a similar problem.
The problem I had was that I had created a taxonomy term with an angle bracket in the name (
"Before<After"), and what I found was that the name was being truncated to"Before". I set up a test content type and added three term reference fields each with different widgets. The autocomplete worked correctly and returned"Before<After", but the radio buttons and the select list returned the truncated version of the label"Before".I think that the behaviour should be to escape the tags using
check_plain()rather thanstrip_tags().The patch I've uploaded changes the behaviour escape the tags rather than strip them. It's still safe as it's escaped, and what the user sees is what has been entered in a safe form. The images uploaded show the before behaviour and after behaviour.
As a result I have also had to change the tests to account for the new behaviour.
If we think this solves the problem, then I am happy to port this to D8.
Comment #15
a.a.smirnov commentedGreetings to all.
It was the same problem as the patch #14 from damienwhaley help to solve it. Thank damienwhaley. :)
Drupal 7.38, php 5.5.26.