Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
If a poll option has special HTML characters such as & ' " > <, after the answer is submitted the answer will not render the character. Instead it will show up in the HTML entity form. The list of characters above would appear as & ' " > <
in the answer after a user has voted
Comments
Comment #2
bbell171 CreditAttribution: bbell171 commentedby changing line 198 in PollViewForm.php to
'#prefix' => '<dt class="choice-title">' . SafeMarkup::escape($options[$pid]) . "</dt>\n" . '<dd class="choice-result">',
HTML characters will display
Comment #3
dakalaComment #4
dakalaAttached patch fixes incorrect rendering of HTML entities in poll titles and answers.
Comment #5
edurenye CreditAttribution: edurenye at MD Systems GmbH commentedMove to needs review to run the tests.
Comment #6
edurenye CreditAttribution: edurenye at MD Systems GmbH commentedThis needs tests.
Comment #9
BerdirPretty sure that this needs updates with the recent core changes. We also need tests.
Comment #10
BerdirComment #11
dakalaRe-rolled patch after rework and added unit test.
Comment #12
BerdirNo t(). That's for interface translation.
Comment #13
BerdirWe should remove all Xss::filter() and checkPlain() calls from the module.
The extend the test to also cover choices.
We need to make sure that both the question and the choices are escaped once and only once in all places (form, results, admin).
Comment #14
dakalaAfter upgrading to 8.0.2 and removing all Xss::filter() and checkPlain() calls, I noticed that all labels are being escaped properly without any explicit calls to Html::escape() or any other methods. Not sure if any changes in 8.0.2 have rectified the problem. I'll go ahead with tests to cover choices first.
Comment #15
BerdirHm.
what happens if you add something like
<script>alert('XSS')</script>
in there? Is that removed? It should be...Comment #16
dakalaI'm afraid alert('XSS'); is not removed from the title or choice fields. Interestingly, it's the same with a basic page node too. alert('XSS'); is displayed as is, in both title and body fields.
Comment #17
BerdirIt's perfectly fine if that is shown. As long as it's not executed. The important part is that the script tags around it are removed or escaped.
Comment #18
BerdirAh, I just noticed I was missing code tags in my comment. Sorry :) See the edit
Comment #19
dakalaYeah, it's still the same with the
tags added. The text is displayed with the tags but not executed - both poll and page node.Comment #20
dakala@berdir: Looks like we no longer require special handling of these characters with 8.0.2 version of core. Added more tests.
Comment #21
dakalaComment #22
Berdir$sanitize as an option no longer exists. Remove the check and the variable completely.
Not sure if this one is safe since it's part of #prefix.
This is already using a temlate, let's just move the prefix and suffix inside the template and provide it as #title => $options[$pid]
t() shouldn't be used for assertion messages, so this was actually correct. You can also just leave out the @title part, Drupal 8 now shows the original value if it can't be found.
Comment #23
tduong CreditAttribution: tduong at MD Systems GmbH commentedFixed as suggested above + some refactoring.
Comment #24
Berdirdouble space?
I know I said that above. But, I think we don't use "choice title" usually, just choice. So lets use #choice and so on.
Comment #25
BerdirComment #26
tduong CreditAttribution: tduong at MD Systems GmbH commentedFixed based on notes above.
Comment #29
BerdirThanks, committed.
Comment #30
tduong CreditAttribution: tduong at MD Systems GmbH commentedComment #32
Bojan Zivkov CreditAttribution: Bojan Zivkov commentedThis problem still remains if poll is used as entity reference field in another content with "inline entity form" set as widget in form display.
Comment #33
ahmadhalah CreditAttribution: ahmadhalah as a volunteer and at Vardot commentedThe problem is still for me I deleted Html::escape from the reprocess hook then its worked fine