It looks like Highcharts does not like check_plain: HTML encoded characters are displayed as { instead of being displayed properly. I'm not sure removing the check_plain is the right solution for this, as it might introduce security issues, but I will provide a patch for it.

Update

The same behaviour have Google-Visualization API library. And it seems that check_plain and strip_tags are redundat functions here, becouse chart libraries should be responsible for safe string processing. And Visualization module should be responsible for checking proper type of the data that would be send to the browser.

CommentFileSizeAuthor
#5 plaintext-field-used.png31.31 KBmradcliffe
#5 formattedtext-field-used.png33.31 KBmradcliffe
#1 check_plain-1985066-1.patch1.04 KBguillaumev
#5 check_plain-1985066-5.patch1.08 KBmradcliffe
#5 interdiff.txt1.06 KBmradcliffe

Comments

guillaumev’s picture

guillaumev commented
StatusFileSize
new check_plain-1985066-1.patch1.04 KB

Here is the patch.

rooby’s picture

rooby commented
Status: Active » Needs review
troybthompson’s picture

troybthompson commented

Hrm, this doesn't seem to have helped my problem, I have a single quote in the label of a pie chart through views and it's still showing up as ' after applying the patch, clearing caches, etc.

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented
Issue summary: View changes
Status: Needs review » Active

Same issue here and patch provided don't resolve it.
A quick (and dirty) fix is to replace check_plain by str_replace(''', "'",

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new interdiff.txt1.06 KB
new check_plain-1985066-5.patch1.08 KB
new formattedtext-field-used.png33.31 KB
new plaintext-field-used.png31.31 KB

There are two issues.

1. Views should already be making the field values safe, and Visualization is double-encoding the text.
2. Highcharts is also doing its own HTML entity encoding.

When I removed the check_plain, this solved #1, but not #2. Instead I changed check_plain to html_entity_decode with ENT_QUOTES flag. I also did a manual test for security using field api text fields configured for plain text and filtered html text formats.

I have attached a test and screenshots.

Steps to test:

  1. Download and install Visualization, Libraries, Views, and Highcharts library.
  2. Create a simple content type with a text field (using text format), a text field (using plain text), and an integer field (for visualization api).
  3. Create some content where both text fields have the same text:
    • AAA & BBB
    • CCC's AAA
    • <span>blah</span>
    • <script>alert(document.cookie);</script>

  4. Create a view with two displays - one using each respective text field.

Results:

  • Niremizov committed 40f6a74 on 7.x-1.x authored by mradcliffe 
    Issue #1985066 by guillaumev, mradcliffe: Highcharts does not like...
Niremizov’s picture

Niremizov commented
Title: Highcharts does not like check_plain » Reundant check_plain() in labels processing
Issue summary: View changes

The same behaviour have Google-Visualization API library.

I have also tested, what would happen if strip_tags and check_plain would be removed and html_entity_decode() would be used instead: Everything seems to look fine, and secure. Beause actually, chart library should be responsible for the way it uses strings and numbers, and we must be sure that data that we send data of appropriate type (number or string).

Fix would arrive soon.

Niremizov’s picture

Niremizov commented
Title: Reundant check_plain() in labels processing » Redundant check_plain() in labels processing

Issue title update.

  • Niremizov committed 8ad2a82 on 7.x-1.x 
    Issue #1985066 by mradcliffe, guillaumev, Niremizov: Fixed redundant...
Niremizov’s picture

Niremizov commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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