Wrong implementation of method setHtmlBody()

[allush]

public function setHtmlBody($html)
    {
        if (is_string($html)) {
            $this->_html = HtmlPurifier::process($html);
        }

        return $this;
    }

HtmlPurifier::process($html) It cuts a lot of styles from the property "style". for example 'border-radius' excluded from result html.

i think, that using HtmlPurifier::process($html) is the wrong approach. better to use a simple assignment.

[nickcv-ln]

I'm ok about making the change, but maybe it should still be an option.

I could add a new configuration option for the module to choose whether or not you want to purify the html input. This would be useful in case the html text is coming from a user input.

[allush]

@nickcv-ln good, option is ok.

[andrey-bahrachev]

@nickcv-ln Using HtmlPurifier within setHtmlBody() method absolutely doesn't make sense, because it totally strips out applied html layout thus making htmlLayout property useless which is wrong.

[nickcv-ln]

@andrey-bahrachev if the html body is coming from a for the purifier prevents the user from injecting potential malicious code, so I still think it could be an option, I can default it to false

[nickcv-ln]

the more I think about it the more I think you guys are right.
It's up to the developer really at this point.

I will just scrap the purifier from setHtmlBody

[andrey-bahrachev]

Yeah, it makes sense to make it optional, but don't use the purifier inside the setHtmlBody() method, put it into the compose() method of the Mailer class before the layout view is rendered.

[nickcv-ln]

@andrey-bahrachev I see what you mean.

Doing it now btw

[andrey-bahrachev]

Cool, thanks a lot!

[nickcv-ln]

@andrey-bahrachev looking at it right now the only way to do it on just the user input before I render the view is if I loop through every single parameter sent through the view and use the purifier on each one that is a string, basically ignoring every user input if they are part of an object sent as a parameter.

If I do apply the purifier to the rendered view there is the issue that It might strip some code that the developer actually wanted to be there.
Probably it would be best to leave the developer in charge of sanitising the user input.

[andrey-bahrachev]

Ah, that's right.

[nickcv-ln]

pushed the new release! thanks @andrey-bahrachev and @allush

[andrey-bahrachev]

Awesome! Thanks :)