When using Diff on a meaningful protected setup with no write permission in Drupal root except the file directory, you get the following error messages:

User warning: Directory .../vendor/ezyang/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer not writable, please chmod to 777 in HTMLPurifier_DefinitionCache_Serializer->_testPermissions() (line 299 of .../vendor/ezyang/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer.php).
HTMLPurifier_DefinitionCache_Serializer->_testPermissions('.../vendor/ezyang/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer', 511) (Line: 229)
HTMLPurifier_DefinitionCache_Serializer->_prepareDir(Object) (Line: 124)
HTMLPurifier_DefinitionCache_Serializer->cleanup(Object) (Line: 108)
HTMLPurifier_DefinitionCache_Decorator->cleanup(Object) (Line: 72)
HTMLPurifier_DefinitionCache_Decorator_Cleanup->get(Object) (Line: 505)
HTMLPurifier_Config->getDefinition('HTML', , ) (Line: 415)
HTMLPurifier_Config->getHTMLDefinition() (Line: 74)
HTMLPurifier_Generator->__construct(Object, Object) (Line: 158)
HTMLPurifier->purify(' SVG Embed ') (Line: 425)
Caxy\HtmlDiff\AbstractDiff->purifyHtml(' SVG Embed ') (Line: 147)
Caxy\HtmlDiff\AbstractDiff->prepare() (Line: 94)
Caxy\HtmlDiff\HtmlDiff->build() (Line: 370)
Caxy\HtmlDiff\HtmlDiff->diffElements(' SVG Embed ', ' SVG Embed ') (Line: 414)
Caxy\HtmlDiff\HtmlDiff->diffElementsByAttribute('<a href="/en/project/svg-embed"> SVG Embed </a>', '<a href="/en/project/svg-embed"> SVG Embed </a>', 'href', 'a') (Line: 336)
Caxy\HtmlDiff\HtmlDiff->diffIsolatedPlaceholder(Object, 4, '[[REPLACE_A]]') (Line: 426)
Caxy\HtmlDiff\HtmlDiff->processEqualOperation(Object) (Line: 251)
Caxy\HtmlDiff\HtmlDiff->performOperation(Object) (Line: 116)
Caxy\HtmlDiff\HtmlDiff->build() (Line: 85)
HtmlDiffAdvanced->build() (Line: 170)
Drupal\diff\Plugin\diff\Layout\VisualInlineDiffLayout->build(Object, Object, Object) (Line: 153)
Drupal\diff\Controller\PluginRevisionController->compareEntityRevisions(Object, Object, Object, 'visual_inline') (Line: 52)
Drupal\diff\Controller\NodeRevisionController->compareNodeRevisions(Object, Object, Object, 'visual_inline')
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 574)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
call_user_func_array(Object, Array) (Line: 144)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 64)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 628)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Looking into the readme of ezyang/htmlpurifier it turns out that they do have a mechanism to configure their caching directory and I think Diff should actually do that when initializing the service.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jurgenhaas created an issue. See original summary.

miro_dietiker’s picture

Priority: Normal » Major

Yeah, that's indeed a problem then on real deployments.

johnchque’s picture

Assigned: Unassigned » johnchque

Let's see what can be done here.

johnchque’s picture

The Visual Inline Layout adds:

    $storage = PhpStorageFactory::get('html_purifier_serializer');
    if (!$storage->exists('cache.php')) {
      $storage->save('cache.php', 'dummy');
    }
    $html_diff->setPurifierSerializerCachePath(dirname($storage->getFullPath('cache.php')));

to prevent this scenarios. Not really sure what else needs to be done. :/

johnchque’s picture

Status: Active » Postponed (maintainer needs more info)

As the code shows. we do set the cache directory already. Will need more info to reproduce the issue.

jurgenhaas’s picture

Here is what I found from debugging:

Your code above is setting a meaningful cache file, it calls mkalkbrenner/php-htmldiff-advanced/src/HtmlDiffAdvanced.php like this:

HtmlDiffAdvanced::setPurifierSerializerCachePath('sites/default/files/php/html_purifier_serializer/cache/848c785f91d80ef06cc3e25fa386d3299b1e516ad45983cf856854baeb31ae34.php')

Later on, the errors are thrown by a completely different package in ezyang/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer.php:

Serializer::_testPermissions('/var/www/vendor/ezyang/htmlpurifier/library/HTMLPurifier/DefinitionCache/Serializer')

So there are two different purifier packages in action? The first one seems to know about the cache location but the latter doesn't?

miro_dietiker’s picture

Priority: Major » Normal

Works for us even with a readonly environment. Please provide more information to investigate.

jurgenhaas’s picture

Status: Postponed (maintainer needs more info) » Needs review
FileSize
737 bytes

The problem is that by calling $html_diff->setPurifierSerializerCachePath() the HtmlDiffAdvanced object is initializing a \HTMLPurifier_Config instance with the correct settings, but that instance is never used. When you later call $this->htmlDiff->build();, then HtmlDiffAdvanced is creating new instances of HtmlDiff for each field in the given HTML string and none of those HtmlDiff instances knows anything about the original \HTMLPurifier_Config and hence the given cache path is not used.

Instead, calling $html_diff->getConfig()->setPurifierCacheLocation() does the right job. The attached patch does the job.

Status: Needs review » Needs work

The last submitted patch, 8: htmlpurifier_serializer-2802261-8.patch, failed testing.

jurgenhaas’s picture

Status: Needs work » Needs review
FileSize
752 bytes

Stupid PhpStorm and their patch syntax. Now corrected.

johnchque’s picture

Status: Needs review » Reviewed & tested by the community

If this fixes the problem for you. I also tested locally as before and the result didn't change. Then RTBC. :) Thank you for the patch!

miro_dietiker’s picture

Status: Reviewed & tested by the community » Fixed

OK, thx, committed. Neeed a conflict free reroll. :-) Thx!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.