salesagility / SuiteCRM

SuiteCRM - Open source CRM for the world
https://www.suitecrm.com
GNU Affero General Public License v3.0
4.42k stars 2.07k forks source link

html smarty tag overwriting other filters in Smarty template by default #659

Closed samus-aran closed 5 years ago

samus-aran commented 8 years ago

strip_tags does not enact when the Smarty template includes the 'html' escape parameters or when 'html_entity_decode' is not included.

Need to include function to exclude html (when not needed) as this is set to default for all textarea fields...

How safe is this with HTML fields i.e. Cases' description?

Referenced with issue #655

adriangibanelbtactic commented 8 years ago

Cases module description field it's setup as "Text area" field in Studio by default.

The tip about editing the detailviewdefs.php file and adding:

'customCode' => '{$fields.description.value|escape:\'htmlentitydecode\' |escape:\'html\'|strip_tags|url2html|nl2br}',

no longer works after having applied: #655 (which it's a must because else all the other text area fields are not seen ok.)

I guess the reason is what you describe about strip_tags not enacting?


Workarounds till a proper fix are welcomed.

samus-aran commented 8 years ago

So this was a general bug/discussion that by default there isn't strip_tags included when there should for particular types of textareas i.e. like you discovered that the HTML editor is a 'textarea' the same as non HTML editors.

With regards to your tip that is (what I felt) is in an incorrect order.

{$fields.description.value|escape:'html'|escape:'html_entity_decode'|strip_tags|url2html|nl2br}

The above is what I felt was correct for the bug fix #655 as the default output of textarea detailviews template. With this particular 'bug' (which I've renamed), we'll require to think of how to remove the escape 'html' tag whenever required which should essentially be when the textarea is an HTML editor. If you run the below with the fix in #655 you will be able to display strip_tags etc.

{$fields.description.value|escape:'html_entity_decode'|strip_tags|url2html|nl2br}

This is just a matter of how to go forward with what tags are interchangeable without having to overwrite with CustomCode.

adriangibanelbtactic commented 8 years ago

I finally applied a patch of mine that involved doing a custom controller and custom view.detail.php so that it showed ok.

I made many changes similar to yours (when trying random things) but I'm not sure it was the same order as you. I will need to test your suggestion in another installation.

My main concern about this stuff is that the Cases's description field in the edit view has been changed to be a html editor (I guess to match the support portal being html). However in detailview it's not seen ok.

Do you know if there is an specific bug for this ... or should I open a new one?

Thank you for your suggestion.

adriangibanelbtactic commented 8 years ago

I confirm that using:

'{$fields.description.value|escape:\'htmlentitydecode\'|strip_tags|url2html|nl2br}' 

or

'{$fields.description.value|escape:\'htmlentitydecode\'|escape:\'html\'|url2html|nl2br}'

in Suitecrm 7.4.3 does not fix the problem.

adriangibanelbtactic commented 8 years ago

Here there is my workaround for Suitecrm 7.4.1 (and probably newer versions) just in case it helps someone: 1) Create:

custom/modules/Cases/controller.php

file which its contents are:

<?php
if(!defined('sugarEntry') || !sugarEntry) die('Not A Valid Entry Point');

require_once('include/MVC/Controller/SugarController.php');
require_once('custom/modules/Cases/PREFIXCustomaCase.php');

class CustomCasesController extends SugarController {
        function action_listview() {
                $this->bean = new PREFIXCustomaCase();
                parent::action_listview();
        }
}

2) Create:

custom/modules/Cases/PREFIXCustomaCase.php

file which its contents are:

<?php
if(!defined('sugarEntry') || !sugarEntry) die('Not A Valid Entry Point');

require_once('modules/Cases/Case.php');

class PREFIXCustomaCase extends aCase {

        function get_list_view_data(){
                $temp_array = parent::get_list_view_data();

                $currentCase = new aCase;
                $currentCase->retrieve($this->id);

                $temp_array['DESCRIPTIONHTML'] = from_html($currentCase->description);

                return $temp_array;
        }
}

3) Create:

custom/modules/Cases/views/view.detail.php

file which its contents are:

<?php
if(!defined('sugarEntry') || !sugarEntry) die('Not A Valid Entry Point');

require_once('include/MVC/View/views/view.detail.php');

class CasesViewDetail extends ViewDetail {

        function CasesViewDetail(){
                parent::ViewDetail();
        }

        function display(){
                $this->dv->process();
                /*$this->ss->assign("sDescription", $this->bean->description);*/
                $this->ss->assign("sDescription", from_html($this->bean->description));
                echo $this->dv->display();

        }
}

?>

4) Edit:

custom/modules/Cases/metadata/detailviewdefs.php

so that you use:

          0 =>
          array (
            'name' => 'description',
            'customCode' => '{$sDescription}',
          ),

5) Edit:

custom/modules/Cases/metadata/listviewdefs.php

so that you use:

  'DESCRIPTIONHTML' =>
  array (
    'type' => 'text',
    'label' => 'LBL_DESCRIPTION',
    'sortable' => false,
    'width' => '10%',
    'default' => true,
  ),

Then, afterwards, you just need to do a quick repair and rebuild so that changes are taken into account.

As always feedback is welcome.

Hopefully this will help Suitecrm developers in order to find a proper fix to this problem.

horus68 commented 6 years ago

Please consider the htmlentitydecode / html_entity_decode usage form https://github.com/salesagility/SuiteCRM/pull/5476

cameronblaikie commented 5 years ago

We have found that this is no longer an issue as a PR has been merged here (https://github.com/salesagility/SuiteCRM/pull/5476). This will now be closed if this is still an issue please feel free to reopen it and we will review your issue again.