html characters show up in export and print

[adrian-stein]

When you export or print a form submission with a form that has a text area field (maybe others as well) the html is shown in the export. If the user hits enter or adds an apostrophe the html characters are shown which is not very user friendly. See example of a forms export below.

testing<br /> <br /> more text goes here<br /> <br /> <br /> what do I do&#039;s text text

[robbieaverill]

Hey @adrian-stein - I have a feeling that the double encoding has been fixed, but I could easily be wrong. As for displaying the HTML, it's potentially an XSS issue if we were to render arbitrary user HTML in the CMS. I suppose we could sanitise it a little - what do you think?

[adrian-stein]

Yeah I would think sanitising it would be the best option so it only displays the raw text. What would be the benefit of showing <br/> in the results over stripping it out?

[dhensby]

Who gets to add the HTML? That's the crux of this. Can it be added by end users or is it automatically added to convert new lines to <br>?

If it's not coming from the form submission directly, then we shouldn't encode these chars - if it is, then it should be escaped as it's not safe to trust.

[adrian-stein]

As far as I can tell with not knowing the form code that well its being made by the code. When you hit 'enter' when filling in a textarea field it converts the enter into a <br>. It is not the user entering the < b r / > characters into the box. For a user who does not know html seeing random characters in the export and results is not a good thing.

[dhensby]

OK - so it sounds like we shouldn't be rendering that HTML literally because it's not what was submitted by the user, we're adding it so it renders nicely in HTML emails and the CMS.

[adrian-stein]

Yeah. It makes sense to show the return character as a br in html as it reflects the 'enter' that the user performed but in exports and prints it doesnt make sense.

[tardinha]

This is a huge, ongoing issue with all our clients.

Any exports they do, they have to then do find/replace on the CSV file to clean out all the HTML.

Changing ATT() to RAW() here solves the issue for both the GridField and the CSV Export. As the it's already been sanitised, adding the RAW() does nothing bad :) (though, I've been wrong before).

As an aside, getExportValue() seems to do nothing.

[robbieaverill]

Ive bumped the impact rating. PRs welcome, in case the core team doesn’t get to it fast enough :-)

[robbieaverill]

I've just re-tested this, here are my findings:

SS 3.7.1, userforms 4.5.x-dev

• HTML is double escaped when viewing it in the CMS (should be single escaped e.g. show <p>Hello world</p> rather than <p>Hello world</p>
• HTML is not escaped in CSV exports: <p>Hello world</p>
• Print view uses the CMS, so double escaped

SS 4.2.0, userforms 5.2.x-dev

• Identical results

Actions

• Remove double escaping of submitted field data in the CMS view

[tardinha]

Ideally, there'd be no HTML at all in CSV exports!

[robbieaverill]

You can extend userforms and remove HTML from submitted form field data if you'd like to, but it seems like an opinionated choice to make from the module's point of view =)

[tardinha]

We get constant complaints that all form exports have "weird characters" all over the place.

Seems to me, putting HTML into a CSV is the opinionated part :P

[robbieaverill]

Well, it'd be your website users that are putting the HTML there wouldn't it? 😄

[tardinha]

Nope.

People fill in a textarea field on a form, adding new lines and so on.

Then the the CSV contains lots of <br>s, etc

[robbieaverill]

Ah, I see what you mean! The export value shouldn't contain <br>, it should contain \n instead

[tardinha]

Yeah. As CSVs are usually exported to be used in some other apps, the content should be lovely and clean and deHTMLed :)

[robbieaverill]

Yeah. As CSVs are usually exported to be used in some other apps, the content should be lovely and clean and deHTMLed :)

To clarify, I agree that the <br> tags which userforms adds should not be displayed literally at any point.

I'm not agreeing that we should be stripping HTML from submitted form fields as part of this module - user code can do that if they want to

[tardinha]

No, the form fields should be as is.

But he CSV export should be HTML free.

[robbieaverill]

If a user submits HTML in a user defined form field I would expect it to be in the CSV export as well

[tardinha]

Yeah...

So what happens if someone puts in Mark's dumb "example", does that end up in the CSV as Mark's dumb "example" ?

[robbieaverill]

No, the CSV exports are currently not escaping HTML at the moment (see findings here).

[tardinha]

Well, they should be :)

At the moment, CSV exports are pretty much useless for end users.

[robbieaverill]

@tardinha I've put a WIP with some detail in #799, we also need https://github.com/silverstripe/silverstripe-framework/pull/8326 as well for print view.

If you want to strip HTML from user generated form submissions you can do it in user code:

SilverStripe\Core\Injector\Injector:
  SilverStripe\UserForms\Model\Submission\SubmittedFormField:
    class: StripIt

<?php

use SilverStripe\UserForms\Model\Submission\SubmittedFormField;

class StripIt extends SubmittedFormField
{
    public function getExportValue()
    {
        return strip_tags(parent::getExportValue());
    }
}

At the moment, CSV exports are pretty much useless for end users.

Yeah, I believe this is because of the HTML escaping issue and having <br> tags in the CSV file. Both of these issues are resolved with #799.

[tardinha]

@robbieaverill awesome, thanks chief :)

[maxime-rainville]

Still a problem in SS4.