Cache directory should default to PHP temporary directory

[fluxsauce]

A site using htmlpurifier as part of a deployable artifact doesn't have access to write to the codebase, but htmlpurifier defaults to writing within the codebase, specifically:

Directory .../sites/all/libraries/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer not writable, please chmod to 777
File .../sites/all/libraries/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer.php, line 278

The library should instead default to PHP's built-in temporary directory support.

Also, 777 is an extremely unsafe permission and should not be recommended, especially on a webserver.

[fluxsauce]

Configuration workaround, but again, this should be the default:

  $html_purifier_cache_dir = sys_get_temp_dir() . '/HTMLPurifier/DefinitionCache';
  if (!is_dir($html_purifier_cache_dir)) {
    mkdir($html_purifier_cache_dir, 0770, TRUE);
  }
  $config->set('Cache.SerializerPath', $html_purifier_cache_dir);

[ezyang]

This is a bit tricky, what are you supposed to do if sys_get_temp_dir() . '/HTMLPurifier/DefinitionCache' already exists but is not owned by you? I suppose we should namespace by running user name. We can't just create a new temporary directory because it needs to persist across calls. In fact /tmp is not a great place to put these files because you don't really ever want to flush them.

[mahony0]

there are pros and cons for changing the cache dir. But surely it'll be logical to not giving writable permission to folders which is in vendors folder (for composer). Also it is not sensible to change the folder permissions after every composer update..

[itbdw]

I think we can throw an error if the cache dir was not set...

[ezyang]

I'd prefer not to break people's code upon an upgrade, if it was working before. But I think a mode to let the user completely manually manage the cache directory would be quite reasonable.

[ezyang]

I pushed a possible patch to fix this, but I am not confident it works. Could y'all apply the patch, set Cache.SerializerPermissions to NULL and see if that works?

[ezyang]

Well, we'll do a release with this, and bugfix if it doesn't work.

[alexander-schranz]

I still get this error: .../Serializer not writable, please chmod to 777. So it is still not the php sys tmp dir? Whats the recommended way to change the cache dir? I use version 4.8.0.

as @fluxsauce said chmod 777 is nothing you should recommend todo would change the error message that the cache dir should be change to a writeable folder.

[ezyang]

@alexander-schranz Try this commit 1ef4375 does it solve your problem?

[alexander-schranz]

@ezyang still same message Directory not writable, please chmod to 777. For me it was no problem to set the Cache.SerializerPath to a path where it works but you should not recommend to set

The message come from: https://github.com/ezyang/htmlpurifier/blob/master/library/HTMLPurifier/DefinitionCache/Serializer.php#L289.

If I look at this class I would recommend you not to set file permissions on runtime. This could change permissions which a user dont want and can open a security whole. I would just output a message when it is not writeable and not try to fix the permissions and it should not recommend to set it to 777. If it is just a read and write cache 660 should be enough for the correct webserver user. If you look at symfony they use a regex to get the correct webserver user: http://symfony.com/doc/current/setup/file_permissions.html#using-acl-on-a-system-that-supports-chmod-a-macos.

[scottpidzarko]

Still getting this issue when using composer in a Laravel 5.1 project after install. Running on Linux Mint.

What's the status on the recommended fix?

[alexander-schranz]

@scottpidzarko I'm not sure how you define this in laravel but maybe this symfony service definition helps you:

        <service id="app.purifier.config" class="HTMLPurifier_Config">
            <factory class="HTMLPurifier_Config" method="createDefault"/>

            <call method="set">
                <argument type="string">Cache.SerializerPath</argument>
                <argument type="string">%app.purifier.serializer-path%</argument><!-- writeable folder path -->
            </call>
            <!-- other configurations -->
            <call method="set">
                <argument type="string">HTML.Allowed</argument>
                <argument type="string">a[href],p,b,i,em,strong,ul,li,ol,s</argument>
            </call>
        </service>

        <service id="app.purifier" class="HTMLPurifier">
            <argument type="service" id="app.purifier.config"/>
        </service>

[Finesse]

How to mate it with Laravel

Create the storage/htmlPurifierCache directory and add a .gitignore file there (like in the storage/logs).

And add this code to the register method of the AppServiceProvider class:

$this->app->bind(\HTMLPurifier_Config::class, function () {
    $config = \HTMLPurifier_Config::createDefault();
    $config->set('Cache.SerializerPath', storage_path('htmlPurifierCache'));
    return $config;
});
$this->app->singleton(\HTMLPurifier::class, function (\Illuminate\Foundation\Application $app) {
    return new \HTMLPurifier($app->make(\HTMLPurifier_Config::class));
});

To get a HTMLPurifier instance, call:

$purifier = app()->make(\HTMLPurifier::class);

Or, if you make a controller method, add the class to the method arguments:

public function action(\HTMLPurifier $purifier, Request $request)
{
    // ...
}

[okainov]

Why is this issue closed? Problem still exists...

[chuck-wood]

FYI for anyone following @Finesse 's suggestion, that Application reference is specifically Illuminate\Foundation\Application. Cheers.

[roben]

Why is this issue closed? Problem still exists...

I second that. And anyways, why does the definition need to be generated on the fly? It could just be packaged together with the rest of the library and all those problems here would be gone.

[ignacio-dev]

Using $config->set('Cache.SerializerPath', storage_path('htmlPurifierCache')); did not solve the issue for me.. A HTML folder is indeed created inside of storage/htmlPurifierCache, but after, the script is still throwing the same error:

vendor/ezyang/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer not writable, please chmod to 777

[asecondwill]

This is still broken for me. HTMLPurifier and capistrano are in a permisiions / chown death grapple. No other composer dependency does this. Please can it just write a cache to somewhere and then leave it alone! stop chowning and setting the permissions. seems like massive over reach.

[asecondwill]

i'm trying to stop it by adding this
$config->set('Cache.SerializerPermissions', 0775);

doesn't make any difference.

[ignacio-dev]

Would it be okay to re-open this issue? Not sure why it's marked as closed.

[GregJohnStewart]

Bump to this, still appears to be an issue in 4.17.0:

Warning
: Directory /var/www/html/vendor/ezyang/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer not writable. in
/var/www/html/vendor/ezyang/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer.php

Using HTMLPurifier_Config::createDefault() as my config.

The $config->set('Cache.SerializerPath', $html_purifier_cache_dir); example code above seemed to fix it for me though.

[vischw]

This issue is valid; the library should not attempt to write to vendor directory for its caching.

The following issues are duplicates:

• Cache directory #105
• Cache.DefinitionImpl / Cache.SerializerPath ignored #370
• Absolute worst security advice #371

[parrycarry]

Recently Updated via Composer and am getting this issue now when I didn't before.

It still tells us to cmod to 777... and I did that, despite now finding issues saying not to do that, and it didn't even fix it anyway, so I reinstalled. I've tried downgrading to V4.16 and it lists it is 4.15 in the version, but it still has the same issue.

$config = HTMLPurifier_Config::createDefault();
$config->set('Core.LexerImpl', 'DirectLex');
$config->set('HTML.Doctype', 'HTML 4.01 Transitional');
$config->set('AutoFormat.RemoveEmpty.RemoveNbsp', TRUE);
$config->set('AutoFormat.RemoveEmpty', TRUE);
$config->set('Attr.AllowedFrameTargets', array('_blank'));
$config->set('HTML.Nofollow', TRUE);
$purifier = new HTMLPurifier($config);

[ignacio-dev]

Is this package just no longer maintained? This has been an ongoing issue for a few years now!

[fluxsauce]

Is this package just no longer maintained? This has been an ongoing issue for a few years now!

Edit: #373 (comment) The 777 issue is resolved, but the broader use of a vendor directory is still applicable.

I'm no longer active in PHP, but they're still commits and PRs are accepted, so while I can't personally guarantee it'll be accepted, it'd be reasonable to expect that a PR would be considered.

[ezyang]

Basically, this one is kind of complicated and I haven't had time to figure out what we should do / how to fix it. If someone wants to come up with a proposal and explain why it is the right thing to do, and then send a PR, I can help shepherd it. But whatever the fix is also needs to not gratuitously break old installs and needs to handle edge cases involving tmp directories.

[fluxsauce]

Basically, this one is kind of complicated and I haven't had time to figure out what we should do / how to fix it. If someone wants to come up with a proposal and explain why it is the right thing to do, and then send a PR, I can help shepherd it. But whatever the fix is also needs to not gratuitously break old installs and needs to handle edge cases involving tmp directories.

That sounds very reasonable. Thank you for your continued support and the project!

[parrycarry]

I mean, is this affecting anything? It feels like everything is working just fine, except I have this error screaming at me for no good reason clogging up my error logs.

[kevincerro]

For anyone, you can disable the cache with

$config->set('Cache.DefinitionImpl', null);