[1.7.0] Add DefinitionID for HTML, to prevent caching conflicts with custom-edited definition objects. Also, more user friendly error messages from Config.

git-svn-id: http://htmlpurifier.org/svnroot/htmlpurifier/trunk@1146 48356398-32a2-884e-a903-53898d9a118a

NEWS

@@ -22,7 +22,8 @@ NEWS ( CHANGELOG and HISTORY )                                     HTMLPurifier
   new objects via make()
 # Definitions (esp. HTMLDefinition) are now cached for a significant
   performance boost. You can disable caching by setting %Core.DefinitionCache
-  to null.
+  to null. You CANNOT edit raw definitions without setting the corresponding
+  DefinitionID directive (%HTML.DefinitionID for HTMLDefinition).
 # Contents between <script> tags are now completely removed if <script>
   is not allowed
 ! HTML Purifier now works in PHP 4.3.2.
@@ -34,6 +35,7 @@ NEWS ( CHANGELOG and HISTORY )                                     HTMLPurifier
 ! All deprecated elements now natively supported
 ! Implement TinyMCE styled whitelist specification format in
   %HTML.Allowed
+! Config object gives more friendly error messages when things go wrong
 - Deprecated and removed EnableRedundantUTF8Cleaning. It didn't even work!
 . Unit test for ElementDef created, ElementDef behavior modified to
   be more flexible

TODO

@@ -7,7 +7,7 @@ TODO List
     ? Maybe I'll Do It
 ==========================
 
-1.7 release [Advanced API]
+1.7 release (possibly 2.0) [Advanced API]
  # Complete advanced API, and fully document it
     - Add framework for unsafe attributes
     - Set up anonymous module management by HTMLDefinition (Advanced API)

library/HTMLPurifier/Config.php

@@ -129,12 +129,14 @@ class HTMLPurifier_Config
     function get($namespace, $key, $from_alias = false) {
         if (!$this->finalized && $this->autoFinalize) $this->finalize();
         if (!isset($this->def->info[$namespace][$key])) {
-            trigger_error('Cannot retrieve value of undefined directive',
+            // can't add % due to SimpleTest bug
+            trigger_error('Cannot retrieve value of undefined directive ' . htmlspecialchars("$namespace.$key"),
                 E_USER_WARNING);
             return;
         }
         if ($this->def->info[$namespace][$key]->class == 'alias') {
-            trigger_error('Cannot get value from aliased directive, use real name',
+            $d = $this->def->info[$namespace][$key];
+            trigger_error('Cannot get value from aliased directive, use real name ' . $d->namespace . '.' . $d->name,
                 E_USER_ERROR);
             return;
         }
@@ -148,7 +150,7 @@ class HTMLPurifier_Config
     function getBatch($namespace) {
         if (!$this->finalized && $this->autoFinalize) $this->finalize();
         if (!isset($this->def->info[$namespace])) {
-            trigger_error('Cannot retrieve undefined namespace',
+            trigger_error('Cannot retrieve undefined namespace ' . htmlspecialchars($namespace),
                 E_USER_WARNING);
             return;
         }
@@ -184,14 +186,14 @@ class HTMLPurifier_Config
     function set($namespace, $key, $value, $from_alias = false) {
         if ($this->isFinalized('Cannot set directive after finalization')) return;
         if (!isset($this->def->info[$namespace][$key])) {
-            trigger_error('Cannot set undefined directive to value',
+            trigger_error('Cannot set undefined directive ' . htmlspecialchars("$namespace.$key") . ' to value',
                 E_USER_WARNING);
             return;
         }
         if ($this->def->info[$namespace][$key]->class == 'alias') {
             if ($from_alias) {
                 trigger_error('Double-aliases not allowed, please fix '.
-                    'ConfigSchema bug');
+                    'ConfigSchema bug with' . "$namespace.$key");
             }
             $this->set($this->def->info[$namespace][$key]->namespace,
                        $this->def->info[$namespace][$key]->name,
@@ -200,7 +202,7 @@ class HTMLPurifier_Config
         }
         $value = $this->def->validate(
                     $value,
-                    $this->def->info[$namespace][$key]->type,
+                    $type = $this->def->info[$namespace][$key]->type,
                     $this->def->info[$namespace][$key]->allow_null
                  );
         if (is_string($value)) {
@@ -211,13 +213,14 @@ class HTMLPurifier_Config
             if ($this->def->info[$namespace][$key]->allowed !== true) {
                 // check to see if the value is allowed
                 if (!isset($this->def->info[$namespace][$key]->allowed[$value])) {
-                    trigger_error('Value not supported', E_USER_WARNING);
+                    trigger_error('Value not supported, valid values are: ' .
+                        $this->_listify($this->def->info[$namespace][$key]->allowed), E_USER_WARNING);
                     return;
                 }
             }
         }
         if ($this->def->isError($value)) {
-            trigger_error('Value is of invalid type', E_USER_WARNING);
+            trigger_error('Value for ' . "$namespace.$key" . ' is of invalid type, should be ' . $type, E_USER_WARNING);
             return;
         }
         $this->conf[$namespace][$key] = $value;
@@ -232,6 +235,16 @@ class HTMLPurifier_Config
         $this->serials[$namespace] = false;
     }
     
+    /**
+     * Convenience function for error reporting
+     * @private
+     */
+    function _listify($lookup) {
+        $list = array();
+        foreach ($lookup as $name => $b) $list[] = $name;
+        return implode(', ', $list);
+    }
+    
     /**
      * Retrieves reference to the HTML definition.
      * @param $raw Return a copy that has not been setup yet. Must be
@@ -285,10 +298,19 @@ class HTMLPurifier_Config
             $this->definitions[$type] = new HTMLPurifier_CSSDefinition();
         } else {
             trigger_error("Definition of $type type not supported");
-            return false;
+            $false = false;
+            return $false;
         }
         // quick abort if raw
-        if ($raw) return $this->definitions[$type];
+        if ($raw) {
+            if (is_null($this->get($type, 'DefinitionID'))) {
+                // fatally error out if definition ID not set
+                trigger_error("Cannot retrieve raw version without specifying %$type.DefinitionID", E_USER_ERROR);
+                $false = false;
+                return $false;
+            }
+            return $this->definitions[$type];
+        }
         // set it up
         $this->definitions[$type]->setup($this);
         // save in cache

library/HTMLPurifier/HTMLDefinition.php

@@ -6,6 +6,37 @@ require_once 'HTMLPurifier/HTMLModuleManager.php';
 // this definition and its modules MUST NOT define configuration directives
 // outside of the HTML or Attr namespaces
 
+HTMLPurifier_ConfigSchema::define(
+    'HTML', 'DefinitionID', null, 'string/null', '
+<p>
+    Unique identifier for a custom-built HTML definition. If you edit
+    the raw version of the HTMLDefinition, introducing changes that the
+    configuration object does not reflect, you must specify this variable.
+    If you change your custom edits, you should change this directive, or
+    clear your cache. Example:
+</p>
+<pre>
+$config = HTMLPurifier_Config::createDefault();
+$config->set(\'HTML\', \'DefinitionID\', \'1\');
+$def = $config->getHTMLDefinition();
+$def->addAttribute(\'a\', \'tabindex\', \'Number\');
+</pre>
+<p>
+    In the above example, the configuration is still at the defaults, but
+    using the advanced API, an extra attribute has been added. The
+    configuration object normally has no way of knowing that this change
+    has taken place, so it needs an extra directive: %HTML.DefinitionID.
+    If someone else attempts to use the default configuration, these two
+    pieces of code will not clobber each other in the cache, since one has
+    an extra directive attached to it.
+</p>
+<p>
+    This directive has been available since 1.7.0, and in that version or
+    later you <em>must</em> specify a value to this directive to use the
+    advanced API features.
+</p>
+');
+
 HTMLPurifier_ConfigSchema::define(
     'HTML', 'BlockWrapper', 'p', 'string', '
 <p>
@@ -107,7 +138,7 @@ HTMLPurifier_ConfigSchema::define(
 class HTMLPurifier_HTMLDefinition extends HTMLPurifier_Definition
 {
     
-    /** FULLY-PUBLIC VARIABLES */
+    // FULLY-PUBLIC VARIABLES ---------------------------------------------
     
     /**
      * Associative array of element names to HTMLPurifier_ElementDef
@@ -172,7 +203,9 @@ class HTMLPurifier_HTMLDefinition extends HTMLPurifier_Definition
     var $doctype;
     
     
-    /** PUBLIC BUT INTERNAL VARIABLES */
+    
+    
+    // PUBLIC BUT INTERNAL VARIABLES --------------------------------------
     
     var $type = 'HTML';
     var $manager; /**< Instance of HTMLPurifier_HTMLModuleManager */

tests/HTMLPurifier/ConfigTest.php

@@ -66,10 +66,10 @@ class HTMLPurifier_ConfigTest extends UnitTestCase
         $config->set('Element', 'IsotopeNames', array(238 => 'Plutonium-238', 239 => 'Plutonium-239'));
         $config->set('Element', 'Object', false); // unmodeled
         
-        $this->expectError('Cannot set undefined directive to value');
+        $this->expectError('Cannot set undefined directive Element.Metal to value');
         $config->set('Element', 'Metal', true);
         
-        $this->expectError('Value is of invalid type');
+        $this->expectError('Value for Element.Radioactive is of invalid type, should be bool');
         $config->set('Element', 'Radioactive', 'very');
         
         // test value retrieval
@@ -83,7 +83,7 @@ class HTMLPurifier_ConfigTest extends UnitTestCase
         $this->assertIdentical($config->get('Element', 'IsotopeNames'), array(238 => 'Plutonium-238', 239 => 'Plutonium-239'));
         $this->assertIdentical($config->get('Element', 'Object'), false);
         
-        $this->expectError('Cannot retrieve value of undefined directive');
+        $this->expectError('Cannot retrieve value of undefined directive Element.Metal');
         $config->get('Element', 'Metal');
         
     }
@@ -117,7 +117,7 @@ class HTMLPurifier_ConfigTest extends UnitTestCase
         $config->set('Instrument', 'Manufacturer', 'Selmer');
         $this->assertIdentical($config->get('Instrument', 'Manufacturer'), 'Conn-Selmer');
         
-        $this->expectError('Value not supported');
+        $this->expectError('Value not supported, valid values are: Yamaha, Conn-Selmer, Vandoren, Laubin, Buffet, other');
         $config->set('Instrument', 'Manufacturer', 'buffet');
         
         // case insensitive
@@ -152,7 +152,7 @@ class HTMLPurifier_ConfigTest extends UnitTestCase
         $this->assertIdentical($config->get('ReportCard', 'English'), null);
         
         // error
-        $this->expectError('Value is of invalid type');
+        $this->expectError('Value for ReportCard.Absences is of invalid type, should be int');
         $config->set('ReportCard', 'Absences', null);
         
     }
@@ -168,7 +168,7 @@ class HTMLPurifier_ConfigTest extends UnitTestCase
         
         $this->assertIdentical($config->get('Home', 'Rug'), 3);
         
-        $this->expectError('Cannot get value from aliased directive, use real name');
+        $this->expectError('Cannot get value from aliased directive, use real name Home.Rug');
         $config->get('Home', 'Carpet');
         
         $config->set('Home', 'Carpet', 999);
@@ -197,7 +197,7 @@ class HTMLPurifier_ConfigTest extends UnitTestCase
         );
         
         // grab a non-existant namespace
-        $this->expectError('Cannot retrieve undefined namespace');
+        $this->expectError('Cannot retrieve undefined namespace Constants');
         $config->getBatch('Constants');
         
     }
@@ -251,6 +251,7 @@ class HTMLPurifier_ConfigTest extends UnitTestCase
         $this->assertTrue($def->setup);
         
         // test retrieval of raw definition
+        $config->set('HTML', 'DefinitionID', 'HTMLPurifier_ConfigTest->test_getHTMLDefinition()');
         $def =& $config->getHTMLDefinition(true);
         $this->assertNotEqual($def, $def2);
         $this->assertFalse($def->setup);
@@ -261,16 +262,29 @@ class HTMLPurifier_ConfigTest extends UnitTestCase
         
     }
     
+    function test_getHTMLDefinition_rawError() {
+        $this->old_copy = HTMLPurifier_ConfigSchema::instance($this->old_copy);
+        $config = HTMLPurifier_Config::createDefault();
+        $this->expectError('Cannot retrieve raw version without specifying %HTML.DefinitionID');
+        $def =& $config->getHTMLDefinition(true);
+    }
+    
     function test_getCSSDefinition() {
         $this->old_copy = HTMLPurifier_ConfigSchema::instance($this->old_copy);
-        
         $config = HTMLPurifier_Config::createDefault();
-        $config->autoFinalize = false;
-        
         $def = $config->getCSSDefinition();
         $this->assertIsA($def, 'HTMLPurifier_CSSDefinition');
     }
     
+    function test_getDefinition() {
+        CS::defineNamespace('Core', 'Core stuff');
+        CS::define('Core', 'DefinitionCache', null, 'string/null', 'Cache?');
+        CS::defineNamespace('Crust', 'Krusty Krabs');
+        $config = HTMLPurifier_Config::createDefault();
+        $this->expectError("Definition of Crust type not supported");
+        $config->getDefinition('Crust');
+    }
+    
     function test_loadArray() {
         // setup a few dummy namespaces/directives for our testing
         CS::defineNamespace('Zoo', 'Animals we have.');