[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

@@ -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:

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(
+                        $this->info[$coll_i][$attr_i] = array_merge(
-                            $info[$coll_i][$attr_i], $attr);
+                            $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])) {
+            if ($t = $attr_types->get($def)) {
-                $attr[$def_i] = $attr_types->info[$def];
+                $attr[$def_i] = $t;
             } else {
-                trigger_error('Attempted to reference undefined attribute type', E_USER_ERROR);
                 unset($attr[$def_i]);
             }
         }

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];
+    }
 }
 
 ?>

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;

tests/HTMLPurifier/AttrCollectionsTest.php

@@ -0,0 +1,131 @@
+<?php
+
+require_once 'HTMLPurifier/AttrCollections.php';
+
+Mock::generatePartial(
+    'HTMLPurifier_AttrCollections',
+    'HTMLPurifier_AttrCollections_TestForConstruct',
+    array('performInclusions', 'expandIdentifiers')
+);
+
+class HTMLPurifier_AttrCollectionsTest extends UnitTestCase
+{
+    
+    function testConstruction() {
+        
+        generate_mock_once('HTMLPurifier_AttrTypes');
+        
+        $collections = new HTMLPurifier_AttrCollections_TestForConstruct($this);
+        
+        $types = new HTMLPurifier_AttrTypesMock($this);
+        
+        $modules = array();
+        
+        $modules['Module1'] = new HTMLPurifier_HTMLModule();
+        $modules['Module1']->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'
+            )
+        );
+        
+    }
+    
+}
+
+?>

tests/HTMLPurifier/AttrTypesTest.php

@@ -0,0 +1,23 @@
+<?php
+
+require_once 'HTMLPurifier/AttrTypes.php';
+
+class HTMLPurifier_AttrTypesTest extends UnitTestCase
+{
+    
+    function test_get() {
+        $types = new HTMLPurifier_AttrTypes();
+        
+        $this->assertIdentical(
+            $types->get('CDATA'),
+            $types->info['CDATA']
+        );
+        
+        $this->expectError('Cannot retrieve undefined attribute type foobar');
+        $types->get('foobar');
+        
+    }
+    
+}
+
+?>

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';