Clean repeater list extension row content of XSS (fix #1973)

[swilliamset]

fix #1973

Added unit test structure for multiple known XSS patterns and ease of adding more.

Updated existing clean method to prevent double encoding.

[swilliamset]

@TheJaredWilcurt please review this as well

[swilliamset]

After review with @TheJaredWilcurt this needs further discussion.

Between customizing the content of data returned by the dataSource data object or the usage of list_columnRendered option the responsibility of ensuring the data is xss clean is up to the consuming code.

The current approach would be a breaking change. Any markup returned by the dataSource will be encoded and not render. Instead applications would need to sure only straight text is passed via the dataSource and any custom markup (ie a link) should be provided using list_customRendered.

To not be a breaking change cleanUp could include an option for a less strict regex that only matches <script> or agreed upon pattern to recognize unsafe code. This can be super complicated.

Possible options:

• reject solution entirely - all xss responsibility on consuming code
• keep solution - document only pure text (no markup) should be returned from dataSource
• modified solution - only encode unsafe markup returned by dataSource

cc @futuremint @CormacMcCarthy

[swilliamset]

Closing as wontfix.

XSS free data is the responsibility of the consuming application. It is recommended to use the dataSource object to insure data is xss free. The cleanInput utility used in this solution could be used to scrub the data.

In the spirit of separation of concerns any markup used to enhance the data (links, icons, etc) should be introduced via the list_columnRendered hook. This will insure dataSource data is markup free.

[cmcculloh-kr]

As we discussed yesterday, I don't believe that dataSource actually should be markup free. I believe that dataSource is where the pre-render actions take place. list_columnRendered is only for things that must take place after something has been rendered to the page (like checking to see if something is visible, or measuring it). Any formatting or html templating should take place prior to attaching to the DOM because of the massive performance hit that doing those things in list_columnRendered (after they have been attached to the DOM) can be. If you have 100 rows, each with 10 columns, then running list_columnRendered 1000 times and doing all of your DOM manipulation there on "data" that has already been attached can and has caused page crashes.

The misnamed, so-called, "dataSource" is really more of a "contentSource". It both gets the data from the server (or itself) and takes care of all of the rendering.