Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds possibility to configure Purifier-Filter dynamically #2

Closed
wants to merge 2 commits into from

Conversation

fg-ok
Copy link

@fg-ok fg-ok commented Jun 18, 2014

This change adds the ability to set a main configuration for Purifier as well as using the Filter with dynamically set configuration options.

@juriansluiman
Copy link

@fg-ok thanks for the contribution! I never seen the purify($html, $config = null); but it seems a very nice addition.

The only thing I wory about your implementation is the state you now hold for the config. Say you turn your examples around:

//Set config dynamically
$filter->setConfig(array('HTML.Allowed' => 'em'));
echo $filter->filter('<h1 onclick="doSomeEvilStuff();"><em>Hello World!</em></h1>'); //Outputs  <em>Hello World!</em>

// Some time later...
$filter = $this->getServiceLocator()->get('FilterManager')->get('htmlpurifier');
echo $filter->filter('<h1 onclick="doSomeEvilStuff();"><em>Hello World!</em></h1>'); //Outputs  <em>Hello World!</em>

As you notice, in the second example you would expect the <h1> come through, but it doesn't since you have set the config earlier. With the modular architecture of ZF2, there might be some module early on setting the config and this results in unexpected outcomes.

My suggestion is to keep it just very simple:

public function filter($value, $config = null)
{
    return $this->getPurifier()->purify($value, $config);
}

Thoughts @fg-ok ?

@juriansluiman
Copy link

Oops, it wasn't meant to close this PR :)

@fg-ok
Copy link
Author

fg-ok commented Jun 18, 2014

Yes, I am aware of the problem that config once set is still valid for the next call.
My pull-request was just a hasty and easy change, that will things make lightly better/more dynamic.

I got an idea how this change could be configurable consistent or not. Will add this ASAP.
Thanks for the input! :)

@fg-ok
Copy link
Author

fg-ok commented Jun 23, 2014

Added a second parameter to setConfig() and a method getConfig() as well.

Now the call could look like:

$filter = $this->getServiceLocator()->get('FilterManager')->get('htmlpurifier');

//Set config dynamically and temporarily 
$filter->setConfig(array('HTML.Allowed' => 'em'));
echo $filter->filter('<h1 onclick="doSomeEvilStuff();"><em>Hello World!</em></h1>'); //Outputs  <em>Hello World!</em>
echo $filter->filter('<h1 onclick="doSomeEvilStuff();"><em>Hello World!</em></h1>'); //2nd call, outputs  <h1><em>Hello World!</em></h1>

or...

$filter = $this->getServiceLocator()->get('FilterManager')->get('htmlpurifier');

//Set config dynamically and persistent 
$filter->setConfig(array('HTML.Allowed' => 'em'), true);
echo $filter->filter('<h1 onclick="doSomeEvilStuff();"><em>Hello World!</em></h1>'); //Outputs  <em>Hello World!</em>
echo $filter->filter('<h1 onclick="doSomeEvilStuff();"><em>Hello World!</em></h1>'); //2nd call, still outputs  <em>Hello World!</em>

@juriansluiman juriansluiman mentioned this pull request Jul 16, 2015
@fg-ok fg-ok closed this May 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants