Custom service provider gives Purify errors in PHPUnit test

Level 73

I think that the Service provider is being booted but the thing that does not work is that you don't have the target attribute in your allowed HTML attributes list. So open the purify.php config file, and change this:

'HTML.Allowed' => 'h1,h2,h3,h4,h5,h6,b,strong,i,em,a[href|title],ul,ol,li,p[style],br,span,img[width|height|alt|src]',

with this:

'HTML.Allowed' => 'h1,h2,h3,h4,h5,h6,b,strong,i,em,a[href|title|target],ul,ol,li,p[style],br,span,img[width|height|alt|src]',

notice the target within the a tag.

Level 17

Hi @Nakov, thanks for your reply.

As said, accessing the application via the browser yields the correct results. The config is fine:

        'HTML.Allowed' => 'a[href|target|title|rel],b,big,blockquote,br,code,del,div,em,h1,h2,h3,h4,h5,h6,hr,i,img[style|width|height|alt|src],li,ol,p[style],pre,small,span[style],strong,sub,sup,table[style|summary],td[abbr],th[abbr],tr,ul,*[style]',

This is the PurifySetupProvider:

<?php

namespace App\Providers;

use HTMLPurifier_HTMLDefinition;
use Stevebauman\Purify\Facades\Purify;
use Illuminate\Support\ServiceProvider;

class PurifySetupProvider extends ServiceProvider
{
    const DEFINITION_ID = 'tinymce-editor';
    const DEFINITION_REV = 1;

    /**
     * Register services.
     *
     * @return void
     */
    public function register()
    {
        /** @var \HTMLPurifier $purifier */
        $purifier = Purify::getPurifier();

        /** @var \HTMLPurifier_Config $config */
        $config = $purifier->config;

        $config->set('HTML.DefinitionID', static::DEFINITION_ID);
        $config->set('HTML.DefinitionRev', static::DEFINITION_REV);

        if ($def = $config->maybeGetRawHTMLDefinition()) {
            $this->setupDefinitions($def);
        }

        $purifier->config = $config;
    }

    protected function setupDefinitions(HTMLPurifier_HTMLDefinition $def)
    {
        $def->addAttribute('a', 'target', 'Text');
    }
}

Which is then loaded in config/app.php

/*
         * Application Service Providers...
         */
        App\Providers\AppServiceProvider::class,
        App\Providers\AuthServiceProvider::class,
        // App\Providers\BroadcastServiceProvider::class,
        App\Providers\EventServiceProvider::class,
        App\Providers\RouteServiceProvider::class,
        App\Providers\PurifySetupProvider::class,
    ],

As said, it works fine using the browser, just not in the tests. Is there any caching going on maybe? For testing? Is there a PHPUnit cache I need to clear?

Level 73

@Thyrosis based on your provider, I think that you should add the logic within the boot function instead of register.

Level 17

Thanks @nakov . I tried this as well. Same result: accessing the application via browser works fine and allows the target attribute in anchors.

Test results still fail.

Level 73

I tried to google your problem, and the only thing that I've found that makes sense and I don't know if you have tried it, is to add the allowed frame target. Change _blank with whatever target you use.

$config->set('Attr.AllowedFrameTargets', array('_blank'));

Other than that I don't think that the problem is specific to the testing env only. You can try removing the custom code in the provider and you will see that the error will be different. Which means that the provider will be booted, but some of the configs are wrong, I am saying all of this based on your error message.

Level 17

TLDR; Am I right in assuming code in an if-statement will never be executed if the statement itself defaults to null?

Well, the extra $config->set doesn't work either @nakov , but you did give me an idea.

I've been backtracking the error, adding a dd($def) to the setupDefinitions method from the PurifySetupProvider. It didn't get called in the tests. But, it didn't get called in the browser either. That leads me to believe that my statement "it works in browser" is actually incorrect. It doesn't work, it just fails silently and doesn't purify anything any more.

That led me to debug through Steve Baumans code and the Ezyang HTMLPurifier. Running through the methods, I think there might be a bug somewhere.

    if ($def = $config->maybeGetRawHTMLDefinition()) {
        $this->setupDefinitions($def);
    }

    public function maybeGetRawHTMLDefinition()
    {
        return $this->getDefinition('HTML', true, true);
    }

    public function getDefinition($type, $raw = false, $optimized = false)
    {
[...]
            if ($optimized) {
                // This code path only gets run once; once we put
                // something in $definitions (which is guaranteed by the
                // trailing code), we always short-circuit above.
                $def = $cache->get($this);
                if ($def) {
                    // save the full definition for later, but don't
                    // return it yet
                    $this->definitions[$type] = $def;
                    return null;
                }
            }

So, $optimized is true. $def is retrieved from the cache and set to $this->definitions[$type]. Return value: null.

Returning back to the start:

    if (null) {
        $this->setupDefinitions($def);
    }

Am I right in assuming $this->setupDefinitions($def) will never be executed because $def always defaults to null?

Level 73

This is true if you have cached the wrong configuration, because once cached it defaults to that version, and it will never call the setupDefinitions. So whenever you change something I think that you should clear the cache, here is a note from the library itself:

Note: Remember that after this definition is created, and you have ran Purify::clean(), the definition will be cached, and you will have to clear it from your storage/app/purify folder if you want to make changes to the definition.

Otherwise, you will have to change the definition version number or ID for it to be re-cached.

Level 17

Hi @nakov, that's exactly what my code does already.

I have an attribute on Post:

    public function getBodyAttribute($body)
    {
        return \Purify::clean($body);
    }

The clean-method can't be called without an argument. Upping the definition_rev number declared at the top of the PurifySetupProvider doesn't work either.

Thank you so much for your help so far! If you have any great ideas left, I'm more than happy to hear them.

Level 17

Okay, I've finally given up. Even with @nakov s help, I've come to the conclusion that sadly, there is no fixing this for me.

So, instead of having the target stored in the database inside the anchors, I've extracted it to some JavaScript code.

    <script type="text/javascript">
            var anchors = document.getElementsByTagName('a');
            for (var i in anchors) {
                if (anchors[i].href && anchors[i].href.indexOf('{{ config('app.url') }}') == -1) {
                    anchors[i].onclick = function () { return !window.open(this); };
                }
            }
        </script>

This little piece of code just loops through all anchors in the page and then checks the href. If the current app.url is not part of the href, it's an external link and it opens in a new window. If the current app.url is in fact in the href, it's a local link and so it just opens in the same window.

No, it's not foolproof and it takes away freedom from the user, but it 'works'. Still, if someone stumbles across this thread and knows the answer, shout away.

Level 73

@thyrosis as a last piece of advice, have you tried opening an issue on the github page for the library and see if some of the maintainers of the code would give you an answer?