L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-41017 improve purify caching

Browse Source 
The improvements include:

* HTMLPurifier cache is stored in localcachedir
* allowobjectembed changes are not ignored any more
* the cache keys include revision and all options which makes
  this suitable for local caches on cluster nodes
* unchanged test is replaced by "true" value which should
  significantly improve performance
* removal of purge_all_caches() hack for directory recreation
* comments and coding style cleanup
This commit is contained in:
Petr Škoda
2013-08-03 11:31:18 +02:00
parent a31e811563
commit 528a7b447a
5 changed files with 76 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lib/db/caches.php
+1
View File
@@ -81,6 +81,7 @@ $definitions = array(
// This caches the html purifier cleaned text. This is done because the text is usually cleaned once for every user
// and context combo. Text caching handles caching for the combination, this cache is responsible for caching the
// cleaned text which is shareable.
// NOTE: this data may be safely stored in local caches on cluster nodes.
'htmlpurifier' => array(
'mode' => cache_store::MODE_APPLICATION,
),
lib/moodlelib.php
-4
View File
@@ -1596,10 +1596,6 @@ function purge_all_caches() {
// Make sure cache dir is writable, throws exception if not.
make_cache_directory('');
// Hack: this script may get called after the purifier was initialised,
// but we do not want to verify repeatedly this exists in each call.
make_cache_directory('htmlpurifier');
// This is the only place where we purge local caches, we are only adding files there.
// The $CFG->localcachedirpurged flag forces local directories to be purged on cluster nodes.
remove_dir($CFG->localcachedir, true);
lib/testing/classes/util.php
-1
View File
@@ -598,7 +598,6 @@ abstract class testing_util {
closedir($handle);
make_temp_directory('');
make_cache_directory('');
make_cache_directory('htmlpurifier');
make_localcache_directory('');
// Reset the cache API so that it recreates it's required directories as well.
cache_factory::reset();
lib/tests/htmlpurifier_test.php
+31
View File
@@ -124,6 +124,37 @@ class core_htmlpurifier_testcase extends basic_testcase {
$this->assertSame('<div id="example">Frog</div>', $result);
}
public function test_allowobjectembed() {
global $CFG;
$this->assertSame('0', $CFG->allowobjectembed);
$text = '<object width="425" height="350">
<param name="movie" value="http://www.youtube.com/v/AyPzM5WK8ys" />
<param name="wmode" value="transparent" />
<embed src="http://www.youtube.com/v/AyPzM5WK8ys" type="application/x-shockwave-flash" wmode="transparent" width="425" height="350" />
</object>hmmm';
$result = purify_html($text, array());
$this->assertSame('hmmm', trim($result));
$CFG->allowobjectembed = '1';
$expected = '<object width="425" height="350" data="http://www.youtube.com/v/AyPzM5WK8ys" type="application/x-shockwave-flash">
<param name="allowScriptAccess" value="never" />
<param name="allowNetworking" value="internal" />
<param name="movie" value="http://www.youtube.com/v/AyPzM5WK8ys" />
<param name="wmode" value="transparent" />
<embed src="http://www.youtube.com/v/AyPzM5WK8ys" type="application/x-shockwave-flash" wmode="transparent" width="425" height="350" allowscriptaccess="never" allownetworking="internal" />
</object>hmmm';
$result = purify_html($text, array());
$this->assertSame(str_replace("\n", '', $expected), str_replace("\n", '', $result));
$CFG->allowobjectembed = '0';
$result = purify_html($text, array());
$this->assertSame('hmmm', trim($result));
}
/**
* Test if linebreaks kept unchanged.
*/
lib/weblib.php
+44 -17
View File
@@ -1615,27 +1615,47 @@ function is_purify_html_necessary($text) {
function purify_html($text, $options = array()) {
global $CFG;
$text = (string)$text;
static $purifiers = array();
static $caches = array();
$type = !empty($options['allowid']) ? 'allowid' : 'normal';
// Purifier code can change only during major version upgrade.
$version = empty($CFG->version) ? 0 : $CFG->version;
$cachedir = "$CFG->localcachedir/htmlpurifier/$version";
if (!file_exists($cachedir)) {
// Purging of caches may remove the cache dir at any time,
// luckily file_exists() results should be cached for all existing directories.
$purifiers = array();
$caches = array();
gc_collect_cycles();
make_localcache_directory('htmlpurifier', false);
check_dir_exists($cachedir);
}
$allowid = empty($options['allowid']) ? 0 : 1;
$allowobjectembed = empty($CFG->allowobjectembed) ? 0 : 1;
$type = 'type_'.$allowid.'_'.$allowobjectembed;
if (!array_key_exists($type, $caches)) {
$caches[$type] = cache::make('core', 'htmlpurifier', array('type' => $type));
}
$cache = $caches[$type];
$filteredtext = $cache->get($text);
if ($filteredtext !== false) {
// Add revision number and all options to the text key so that it is compatible with local cluster node caches.
$key = "|$version|$allowobjectembed|$allowid|$text";
$filteredtext = $cache->get($key);
if ($filteredtext === true) {
// The filtering did not change the text last time, no need to filter anything again.
return $text;
} else if ($filteredtext !== false) {
return $filteredtext;
}
if (empty($purifiers[$type])) {
// make sure the serializer dir exists, it should be fine if it disappears later during cache reset
$cachedir = $CFG->cachedir.'/htmlpurifier';
check_dir_exists($cachedir);
require_once $CFG->libdir.'/htmlpurifier/HTMLPurifier.safe-includes.php';
require_once $CFG->libdir.'/htmlpurifier/locallib.php';
$config = HTMLPurifier_Config::createDefault();
@@ -1651,22 +1671,22 @@ function purify_html($text, $options = array()) {
$config->set('URI.AllowedSchemes', array('http'=>true, 'https'=>true, 'ftp'=>true, 'irc'=>true, 'nntp'=>true, 'news'=>true, 'rtsp'=>true, 'teamspeak'=>true, 'gopher'=>true, 'mms'=>true, 'mailto'=>true));
$config->set('Attr.AllowedFrameTargets', array('_blank'));
if (!empty($CFG->allowobjectembed)) {
if ($allowobjectembed) {
$config->set('HTML.SafeObject', true);
$config->set('Output.FlashCompat', true);
$config->set('HTML.SafeEmbed', true);
}
if ($type === 'allowid') {
if ($allowid) {
$config->set('Attr.EnableID', true);
}
if ($def = $config->maybeGetRawHTMLDefinition()) {
$def->addElement('nolink', 'Block', 'Flow', array()); // skip our filters inside
$def->addElement('tex', 'Inline', 'Inline', array()); // tex syntax, equivalent to $$xx$$
$def->addElement('algebra', 'Inline', 'Inline', array()); // algebra syntax, equivalent to @@xx@@
$def->addElement('lang', 'Block', 'Flow', array(), array('lang'=>'CDATA')); // old and future style multilang - only our hacked lang attribute
$def->addAttribute('span', 'xxxlang', 'CDATA'); // current problematic multilang
$def->addElement('nolink', 'Block', 'Flow', array()); // Skip our filters inside.
$def->addElement('tex', 'Inline', 'Inline', array()); // Tex syntax, equivalent to $$xx$$.
$def->addElement('algebra', 'Inline', 'Inline', array()); // Algebra syntax, equivalent to @@xx@@.
$def->addElement('lang', 'Block', 'Flow', array(), array('lang'=>'CDATA')); // Original multilang style - only our hacked lang attribute.
$def->addAttribute('span', 'xxxlang', 'CDATA'); // Current very problematic multilang.
}
$purifier = new HTMLPurifier($config);
@@ -1681,11 +1701,18 @@ function purify_html($text, $options = array()) {
if ($multilang) {
$filteredtext = preg_replace('/<span(\s+lang="([a-zA-Z0-9_-]+)"|\s+class="multilang"){2}\s*>/', '<span xxxlang="${2}">', $filteredtext);
}
$filteredtext = $purifier->purify($filteredtext);
$filteredtext = (string)$purifier->purify($filteredtext);
if ($multilang) {
$filteredtext = preg_replace('/<span xxxlang="([a-zA-Z0-9_-]+)">/', '<span lang="${1}" class="multilang">', $filteredtext);
}
$cache->set($text, $filteredtext);
if ($text === $filteredtext) {
// No need to store the filtered text, next time we will just return unfiltered text
// because it was not changed by purifying.
$cache->set($key, true);
} else {
$cache->set($key, $filteredtext);
}
return $filteredtext;
}