[REF] Remove xssString as it is providing a false sense of security

[seamuslee001]

Overview

This removes the xssString function which only partially captures possible xss and is only really providing a false sense of security

Before

Function Present

After

Function Gone

ping @colemanw @eileenmcnaughton @totten @pfigel

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

@seamuslee001 I agree with the concept. Test failures seem to indicate the rule is still in use by many forms though.

[seamuslee001]

@colemanw I think that is coming from needing civicrm/civicrm-packages#312 to be merged first then

[totten]

It's a little surprising, but - fortunately - xssString doesn't appear to be used in universe. So I guess we don't need to think to hard about special/extension-specific impacts. 🎉

At a macro level, this also seems a bit redundant with PHP IDS (if you assume that PHP IDS is wired-up correctly to the inputs). Relevant factoids:

• xssString first appears in v1.7: https://github.com/civicrm/civicrm-svn/blob/v1.7/CRM/Utils/Rule.php#L498
• PHP IDS first appears in v2.1: https://github.com/civicrm/civicrm-svn/blob/v2.1/CRM/Core/Invoke.php#L72

For me, I guess the main question would be -- if you took the list of xssString use-cases/fields and overlayed with the list of PHP IDS use-cases/fields, would the disjoint section be problematic?

[seamuslee001]

@totten I don't think so, I think having this function may lead to people to potentially try and use it when it doesn't work properly so I think we should get it out given its not really providing a decent defence

[seamuslee001]

Jenkins re test this please,

I'm going to add merge on pass based on Coleman and Tim's comments and based on Coleman putting MOP on the packages PR