diff --git a/includes/path.inc b/includes/path.inc index 630b34c..7cac7bb 100644 --- a/includes/path.inc +++ b/includes/path.inc @@ -55,21 +55,13 @@ function drupal_lookup_path($action, $path = '', $path_language = NULL) { $cache = array( 'map' => array(), 'no_source' => array(), - 'whitelist' => NULL, + 'whitelist' => new PathAliasWhitelist('path_alias_whitelist', 'cache'), 'system_paths' => array(), 'no_aliases' => array(), 'first_call' => TRUE, ); } - // Retrieve the path alias whitelist. - if (!isset($cache['whitelist'])) { - $cache['whitelist'] = variable_get('path_alias_whitelist', NULL); - if (!isset($cache['whitelist'])) { - $cache['whitelist'] = drupal_path_alias_whitelist_rebuild(); - } - } - // If no language is explicitly specified we default to the current URL // language. If we used a language different from the one conveyed by the // requested URL, we might end up being unable to check if there is a path @@ -356,33 +348,121 @@ function current_path() { } /** + * Extends DrupalCacheArray to build the path alias whitelist over time. + */ +class PathAliasWhitelist extends DrupalCacheArray { + + /** + * The substring length for whitelisted paths. + * + * The path alias whitelist is an array of path substrings, and whether + * that substring matches any aliases. The size of this array must be + * restricted, but the longer the substring, the more opportunity there is + * to avoid looking up paths that don't match. Therefore the length of + * the substring is maintained dynamically and reduced when there are more + * than 100 paths in the array. Since the cache has to be rebuilt each time + * the variable is shortened, it defaults to 5 at first. + */ + protected $length; + + function __construct($cid, $bin) { + $this->length = variable_get('path_alias_whitelist_length', 5); + $this->cid = $cid; + $this->bin = $bin; + parent::__construct($this->cid, $this->bin); + } + + /** + * Substring a path to a standard length. + */ + protected function substr($offset) { + return drupal_substr($offset, 0, $this->length); + } + + /** + * Check the size of the whitelist cache and modify it if necessary. + * + * @return + * True if the size of the storage exceeded the limit. + */ + function checkStorage() { + // If the whitelist is larger than 100 items, reduce the allowed key length + // by one to reduce the number of possible unique keys. If the allowed + // length is already one, leave it as it is since it is not possible to + // subdivide any more. + if ($this->length > 1 && count($this->storage) >= 100) { + // Reduce the length by 1, + $this->length = max(1, $this->length - 1); + $this->keysToPersist = array(); + $this->storage = array(); + // Acquire a lock to prevent other processes writing to the cache with + // whitelist items using the old variable value. Since checkStorage() is + // called by resolveCacheMiss(), a new cache item will be entered when + // the class goes out of scope in DrupalCacheArray::set() before the lock + // is released. + lock_acquire($this->bin . ':' . $this->cid); + variable_set('path_alias_whitelist_length', $this->length); + cache($this->bin)->delete($this->cid); + return TRUE; + } + } + + + function offsetExists($offset) { + return parent::offsetExists($this->substr($offset)); + } + + function offsetGet($offset) { + return parent::offsetGet($this->substr($offset)); + } + + function offsetSet($offset, $value) { + parent::offsetSet($this->substr($offset), $value); + } + + function offsetUnset($offset) { + parent::offsetUnset($this->substr($offset)); + } + + function resolveCacheMiss($path_root) { + // Check the size of the storage. If it's too big, reduce the substring + // length by one. + if ($this->checkStorage()) { + $path_root = $this->substr($path_root); + } + $query = db_select('url_alias', 'u'); + $query->addExpression(1); + $query->condition('u.source', db_like($path_root) . '%', 'LIKE') ->range(0, 1); + $exists = $query->execute()->fetchField(); + // If a path prefix does not exist, store it as NULL rather than FALSE + // since DrupalCacheArray uses isset() in offsetExists(). + $value = $exists ? TRUE : NULL; + $this->storage[$path_root] = $value; + $this->persist($path_root); + return $value; + } +} + +/** * Rebuild the path alias white list. * * @param $source * An optional system path for which an alias is being inserted. * * @return - * An array containing a white list of path aliases. + * An instance of the PathAliasWhitelist class. */ function drupal_path_alias_whitelist_rebuild($source = NULL) { // When paths are inserted, only rebuild the whitelist if the system path // has a top level component which is not already in the whitelist. if (!empty($source)) { - $whitelist = variable_get('path_alias_whitelist', NULL); - if (isset($whitelist[strtok($source, '/')])) { + $whitelist = new PathAliasWhitelist('path_alias_whitelist', 'cache'); + if (isset($whitelist[$soure])) { return $whitelist; } } - // For each alias in the database, get the top level component of the system - // path it corresponds to. This is the portion of the path before the first - // '/', if present, otherwise the whole path itself. - $whitelist = array(); - $result = db_query("SELECT DISTINCT SUBSTRING_INDEX(source, '/', 1) AS path FROM {url_alias}"); - foreach ($result as $row) { - $whitelist[$row->path] = TRUE; - } - variable_set('path_alias_whitelist', $whitelist); - return $whitelist; + cache_clear_all('path_alias_whitelist', 'cache'); + return new PathAliasWhitelist('path_alias_whitelist', 'cache'); } /** diff --git a/modules/simpletest/tests/path.test b/modules/simpletest/tests/path.test index 82598b5..f5784ec 100644 --- a/modules/simpletest/tests/path.test +++ b/modules/simpletest/tests/path.test @@ -237,6 +237,47 @@ class UrlAlterFunctionalTest extends DrupalWebTestCase { } /** + * Tests for path whitelist lookup. + */ +class PathWhitelistTestCase extends DrupalWebTestCase { + public static function getInfo() { + return array( + 'name' => t('Path whitelist'), + 'description' => t('Tests the path alias whitelist'), + 'group'=> t('Path API'), + ); + } + + /** + * Test size limiting of the path whitelist. + */ + function testPathWhitelist() { + + // Whitelist key length should begin at five. + $length = 5; + while ($length >= 1) { + $whitelist = new PathAliasWhitelist('path_alias_whitelist', 'cache'); + $this->assertEqual(variable_get('path_alias_whitelist_length', 5), $length); + // For each iteration, check the whitelist for more than 100 unique URLs. + // This should trigger a change in the path_alias_whitelist_length + // variable each time. + $n = 0; + while ($n <= 101) { + $n++; + $foo = isset($whitelist[$this->randomString(10)]); + } + $length--; + } + // Unset the whitelist to trigger a cache set. + unset($whitelist); + // After the last iteration, there should be less than 100 items in the + // cache, but more than one. + $cached = cache()->get('path_alias_whitelist'); + $this->assertTrue(count($cached->data) > 1); + } +} + +/** * Unit test for drupal_lookup_path(). */ class PathLookupTest extends DrupalWebTestCase { diff --git a/modules/simpletest/tests/theme.test b/modules/simpletest/tests/theme.test index d064359..7c68989 100644 --- a/modules/simpletest/tests/theme.test +++ b/modules/simpletest/tests/theme.test @@ -265,7 +265,7 @@ class ThemeFunctionsTestCase extends DrupalWebTestCase { /** * Unit tests for theme_links(). */ -class ThemeLinksUnitTest extends DrupalUnitTestCase { +class ThemeLinksTest extends DrupalWebTestCase { public static function getInfo() { return array( 'name' => 'Links',