From 3f06d8316cc162f49df3833773c1e43cf9e937fb Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Sun, 20 May 2007 19:29:05 +0000 Subject: [PATCH] [1.7.0] Add unit test for AttrCollections - Fixed bug where recursive attribute collections would result in infinite loop - Fixed bug with deep inclusions in attribute collections - Reset doctype object if HTML or Attr is changed - Add accessor functions to AttrTypes, unit tested class git-svn-id: http://htmlpurifier.org/svnroot/htmlpurifier/trunk@1077 48356398-32a2-884e-a903-53898d9a118a --- NEWS | 2 + library/HTMLPurifier/AttrCollections.php | 32 ++--- library/HTMLPurifier/AttrTypes.php | 12 ++ library/HTMLPurifier/Config.php | 6 +- tests/HTMLPurifier/AttrCollectionsTest.php | 131 +++++++++++++++++++++ tests/HTMLPurifier/AttrTypesTest.php | 23 ++++ tests/test_files.php | 2 + 7 files changed, 192 insertions(+), 16 deletions(-) create mode 100644 tests/HTMLPurifier/AttrCollectionsTest.php create mode 100644 tests/HTMLPurifier/AttrTypesTest.php diff --git a/NEWS b/NEWS index 34b892b3..4ba36902 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,8 @@ NEWS ( CHANGELOG and HISTORY ) HTMLPurifier . Unit test for ElementDef created, ElementDef behavior modified to be more flexible . Added convenience functions for HTMLModule constructors +. AttrTypes now has accessor functions that should be used instead + of directly manipulating info 1.6.1, released 2007-05-05 ! Support for more deprecated attributes via transformations: diff --git a/library/HTMLPurifier/AttrCollections.php b/library/HTMLPurifier/AttrCollections.php index 8318abb1..018efb19 100644 --- a/library/HTMLPurifier/AttrCollections.php +++ b/library/HTMLPurifier/AttrCollections.php @@ -12,8 +12,6 @@ class HTMLPurifier_AttrCollections /** * Associative array of attribute collections, indexed by name - * @note Technically, the composition of these is more complicated, - * but we bypass it using our own excludes property */ var $info = array(); @@ -25,27 +23,29 @@ class HTMLPurifier_AttrCollections * @param $modules Hash array of HTMLPurifier_HTMLModule members */ function HTMLPurifier_AttrCollections($attr_types, $modules) { - $info =& $this->info; // load extensions from the modules foreach ($modules as $module) { foreach ($module->attr_collections as $coll_i => $coll) { + if (!isset($this->info[$coll_i])) { + $this->info[$coll_i] = array(); + } foreach ($coll as $attr_i => $attr) { - if ($attr_i === 0 && isset($info[$coll_i][$attr_i])) { + if ($attr_i === 0 && isset($this->info[$coll_i][$attr_i])) { // merge in includes - $info[$coll_i][$attr_i] = array_merge( - $info[$coll_i][$attr_i], $attr); + $this->info[$coll_i][$attr_i] = array_merge( + $this->info[$coll_i][$attr_i], $attr); continue; } - $info[$coll_i][$attr_i] = $attr; + $this->info[$coll_i][$attr_i] = $attr; } } } // perform internal expansions and inclusions - foreach ($info as $name => $attr) { + foreach ($this->info as $name => $attr) { // merge attribute collections that include others - $this->performInclusions($info[$name]); + $this->performInclusions($this->info[$name]); // replace string identifiers with actual attribute objects - $this->expandIdentifiers($info[$name], $attr_types); + $this->expandIdentifiers($this->info[$name], $attr_types); } } @@ -57,16 +57,19 @@ class HTMLPurifier_AttrCollections function performInclusions(&$attr) { if (!isset($attr[0])) return; $merge = $attr[0]; + $seen = array(); // recursion guard // loop through all the inclusions for ($i = 0; isset($merge[$i]); $i++) { + if (isset($seen[$merge[$i]])) continue; + $seen[$merge[$i]] = true; // foreach attribute of the inclusion, copy it over foreach ($this->info[$merge[$i]] as $key => $value) { if (isset($attr[$key])) continue; // also catches more inclusions $attr[$key] = $value; } - if (isset($info[$merge[$i]][0])) { + if (isset($this->info[$merge[$i]][0])) { // recursion - $merge = array_merge($merge, isset($info[$merge[$i]][0])); + $merge = array_merge($merge, $this->info[$merge[$i]][0]); } } unset($attr[0]); @@ -86,10 +89,9 @@ class HTMLPurifier_AttrCollections unset($attr[$def_i]); continue; } - if (isset($attr_types->info[$def])) { - $attr[$def_i] = $attr_types->info[$def]; + if ($t = $attr_types->get($def)) { + $attr[$def_i] = $t; } else { - trigger_error('Attempted to reference undefined attribute type', E_USER_ERROR); unset($attr[$def_i]); } } diff --git a/library/HTMLPurifier/AttrTypes.php b/library/HTMLPurifier/AttrTypes.php index ea6c1912..f186d260 100644 --- a/library/HTMLPurifier/AttrTypes.php +++ b/library/HTMLPurifier/AttrTypes.php @@ -37,6 +37,18 @@ class HTMLPurifier_AttrTypes // number is really a positive integer (one or more digits) $this->info['Number'] = new HTMLPurifier_AttrDef_Integer(false, false, true); } + + /** + * Retrieves a type + */ + function get($type) { + // maybe some extra initialization could be done + if (!isset($this->info[$type])) { + trigger_error('Cannot retrieve undefined attribute type ' . $type, E_USER_ERROR); + return; + } + return $this->info[$type]; + } } ?> diff --git a/library/HTMLPurifier/Config.php b/library/HTMLPurifier/Config.php index 8878684e..2bc2f460 100644 --- a/library/HTMLPurifier/Config.php +++ b/library/HTMLPurifier/Config.php @@ -168,9 +168,13 @@ class HTMLPurifier_Config return; } $this->conf[$namespace][$key] = $value; + + // reset definitions if the directives they depend on changed + // this is a very costly process, so it's discouraged + // with finalization if ($namespace == 'HTML' || $namespace == 'Attr') { - // reset HTML definition if relevant attributes changed $this->html_definition = null; + $this->doctype = null; } if ($namespace == 'CSS') { $this->css_definition = null; diff --git a/tests/HTMLPurifier/AttrCollectionsTest.php b/tests/HTMLPurifier/AttrCollectionsTest.php new file mode 100644 index 00000000..f43ee90e --- /dev/null +++ b/tests/HTMLPurifier/AttrCollectionsTest.php @@ -0,0 +1,131 @@ +attr_collections = array( + 'Core' => array( + 0 => array('Soup', 'Undefined'), + 'attribute' => 'Type', + 'attribute-2' => 'Type2', + ), + 'Soup' => array( + 'attribute-3' => 'Type3-old' // overwritten + ) + ); + + $modules['Module2'] = new HTMLPurifier_HTMLModule(); + $modules['Module2']->attr_collections = array( + 'Core' => array( + 0 => array('Brocolli') + ), + 'Soup' => array( + 'attribute-3' => 'Type3' + ), + 'Brocolli' => array() + ); + + $collections->HTMLPurifier_AttrCollections($types, $modules); + // this is without identifier expansion or inclusions + $this->assertIdentical( + $collections->info, + array( + 'Core' => array( + 0 => array('Soup', 'Undefined', 'Brocolli'), + 'attribute' => 'Type', + 'attribute-2' => 'Type2' + ), + 'Soup' => array( + 'attribute-3' => 'Type3' + ), + 'Brocolli' => array() + ) + ); + + } + + function test_performInclusions() { + + generate_mock_once('HTMLPurifier_AttrTypes'); + + $types = new HTMLPurifier_AttrTypesMock($this); + $collections = new HTMLPurifier_AttrCollections($types, array()); + $collections->info = array( + 'Core' => array(0 => array('Inclusion'), 'attr-original' => 'Type'), + 'Inclusion' => array(0 => array('SubInclusion'), 'attr' => 'Type'), + 'SubInclusion' => array('attr2' => 'Type') + ); + + $collections->performInclusions($collections->info['Core']); + $this->assertIdentical( + $collections->info['Core'], + array( + 'attr-original' => 'Type', + 'attr' => 'Type', + 'attr2' => 'Type' + ) + ); + + // test recursive + $collections->info = array( + 'One' => array(0 => array('Two'), 'one' => 'Type'), + 'Two' => array(0 => array('One'), 'two' => 'Type') + ); + $collections->performInclusions($collections->info['One']); + $this->assertIdentical( + $collections->info['One'], + array( + 'one' => 'Type', + 'two' => 'Type' + ) + ); + + } + + function test_expandIdentifiers() { + + generate_mock_once('HTMLPurifier_AttrTypes'); + + $types = new HTMLPurifier_AttrTypesMock($this); + $collections = new HTMLPurifier_AttrCollections($types, array()); + + $attr = array( + 'attr1' => 'Color', + 'attr2' => 'URI' + ); + $types->setReturnValue('get', 'ColorObject', array('Color')); + $types->setReturnValue('get', 'URIObject', array('URI')); + $collections->expandIdentifiers($attr, $types); + + $this->assertIdentical( + $attr, + array( + 'attr1' => 'ColorObject', + 'attr2' => 'URIObject' + ) + ); + + } + +} + +?> \ No newline at end of file diff --git a/tests/HTMLPurifier/AttrTypesTest.php b/tests/HTMLPurifier/AttrTypesTest.php new file mode 100644 index 00000000..8501d9e2 --- /dev/null +++ b/tests/HTMLPurifier/AttrTypesTest.php @@ -0,0 +1,23 @@ +assertIdentical( + $types->get('CDATA'), + $types->info['CDATA'] + ); + + $this->expectError('Cannot retrieve undefined attribute type foobar'); + $types->get('foobar'); + + } + +} + +?> \ No newline at end of file diff --git a/tests/test_files.php b/tests/test_files.php index c9cab78a..1112f231 100644 --- a/tests/test_files.php +++ b/tests/test_files.php @@ -3,6 +3,7 @@ if (!defined('HTMLPurifierTest')) exit; // define callable test files (sorted alphabetically) +$test_files[] = 'AttrCollectionsTest.php'; $test_files[] = 'AttrDef/CSS/BackgroundPositionTest.php'; $test_files[] = 'AttrDef/CSS/BackgroundTest.php'; $test_files[] = 'AttrDef/CSS/BorderTest.php'; @@ -46,6 +47,7 @@ $test_files[] = 'AttrTransform/ImgSpaceTest.php'; $test_files[] = 'AttrTransform/LangTest.php'; $test_files[] = 'AttrTransform/LengthTest.php'; $test_files[] = 'AttrTransform/NameTest.php'; +$test_files[] = 'AttrTypesTest.php'; $test_files[] = 'ChildDef/ChameleonTest.php'; $test_files[] = 'ChildDef/CustomTest.php'; $test_files[] = 'ChildDef/OptionalTest.php';