[3.1.0] Implemented redundant validators and tests

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

library/HTMLPurifier/ConfigSchema/Interchange.php

@@ -32,7 +32,7 @@ class HTMLPurifier_ConfigSchema_Interchange
      * Adds a directive array to $directives
      */
     public function addDirective($directive) {
-        if (isset($this->directives[$i = $directive->id->__toString()])) {
+        if (isset($this->directives[$i = "{$directive->id}"])) {
             throw new HTMLPurifier_ConfigSchema_Exception("Cannot redefine directive '$i'");
         }
         $this->directives[$i] = $directive;

library/HTMLPurifier/ConfigSchema/InterchangeBuilder.php

@@ -56,7 +56,7 @@ class HTMLPurifier_ConfigSchema_InterchangeBuilder
             try {
                 $directive->default = $this->varParser->parse($hash->offsetGet('DEFAULT'), $directive->type, $directive->typeAllowsNull);
             } catch (HTMLPurifier_VarParserException $e) {
-                throw new HTMLPurifier_ConfigSchema_Exception($e->getMessage() . " in DEFAULT in directive hash '{$directive->id}'");
+                throw new HTMLPurifier_ConfigSchema_Exception($e->getMessage() . " in TYPE/DEFAULT in directive hash '{$directive->id}'");
             }
         }
         
@@ -70,8 +70,11 @@ class HTMLPurifier_ConfigSchema_InterchangeBuilder
         
         if (isset($hash['VALUE-ALIASES'])) {
             $value_aliases = $this->evalArray($hash->offsetGet('VALUE-ALIASES'));
+            // :TODO: Build corresponding test in Validator.php
             try {
                 foreach ($value_aliases as $alias => $real) {
+                    // might want to allow users to use a different var parser
+                    // in this case
                     $directive->valueAliases[$this->varParser->parse($alias, $directive->type, $directive->typeAllowsNull)] =
                         $this->varParser->parse($real, $directive->type, $directive->typeAllowsNull);
                 }

library/HTMLPurifier/ConfigSchema/Validator.php

@@ -2,6 +2,11 @@
 
 /**
  * Performs validations on HTMLPurifier_ConfigSchema_Interchange
+ *
+ * @note If you see '// handled by InterchangeBuilder', that means a
+ *       design decision in that class would prevent this validation from
+ *       ever being necessary. We have them anyway, however, for
+ *       redundancy.
  */
 class HTMLPurifier_ConfigSchema_Validator
 {
@@ -28,10 +33,14 @@ class HTMLPurifier_ConfigSchema_Validator
      */
     public function validate($interchange) {
         $this->interchange = $interchange;
-        foreach ($interchange->namespaces as $namespace) {
+        // PHP is a bit lax with integer <=> string conversions in
+        // arrays, so we don't use the identical !== comparison
+        foreach ($interchange->namespaces as $i => $namespace) {
+            if ($i != $namespace->namespace) $this->error(false, "Integrity violation: key '$i' does not match internal id '{$namespace->namespace}'");
             $this->validateNamespace($namespace);
         }
-        foreach ($interchange->directives as $directive) {
+        foreach ($interchange->directives as $i => $directive) {
+            if ($i != "{$directive->id}") $this->error(false, "Integrity violation: key '$i' does not match internal id '{$directive->id}'");
             $this->validateDirective($directive);
         }
     }
@@ -40,24 +49,25 @@ class HTMLPurifier_ConfigSchema_Validator
         $this->context[] = "namespace '{$n->namespace}'";
         $this->with($n, 'namespace')
             ->assertNotEmpty()
-            ->assertAlnum();
+            ->assertAlnum(); // implicit assertIsString handled by InterchangeBuilder
         $this->with($n, 'description')
             ->assertNotEmpty()
-            ->assertIsString(); // technically redundant
+            ->assertIsString(); // handled by InterchangeBuilder
         array_pop($this->context);
     }
     
     public function validateId($id) {
         $this->context[] = "id '$id'";
-        if (!isset($this->interchange->namespaces[$id->namespace])) {
+        if (!$id instanceof HTMLPurifier_ConfigSchema_Interchange_Id) {
-            $this->error('namespace', 'does not exist');
+            // handled by InterchangeBuilder
+            $this->error(false, 'is not an instance of HTMLPurifier_ConfigSchema_Interchange_Id');
+        }
+        if (!isset($this->interchange->namespaces[$id->namespace])) {
+            $this->error('namespace', 'does not exist'); // assumes that the namespace was validated already
         }
-        $this->with($id, 'namespace')
-            ->assertNotEmpty()
-            ->assertAlnum();
         $this->with($id, 'directive')
             ->assertNotEmpty()
-            ->assertAlnum();
+            ->assertAlnum(); // implicit assertIsString handled by InterchangeBuilder
         array_pop($this->context);
     }
     
@@ -67,11 +77,14 @@ class HTMLPurifier_ConfigSchema_Validator
         $this->with($d, 'description')
             ->assertNotEmpty();
         $this->with($d, 'type')
-            ->assertNotEmpty();
+            ->assertNotEmpty(); // handled by InterchangeBuilder
-        if (!isset(HTMLPurifier_VarParser::$types[$d->type])) {
+        // Much stricter default check, since we're using the base implementation.
-            $this->error('type', 'is invalid');
+        // handled by InterchangeBuilder
-        }
+        try {
             $this->parser->parse($d->default, $d->type, $d->typeAllowsNull);
+        } catch (HTMLPurifier_VarParserException $e) {
+            $this->error('default', 'had error: ' . $e->getMessage());
+        }
         
         array_pop($this->context);
     }
@@ -83,7 +96,9 @@ class HTMLPurifier_ConfigSchema_Validator
     }
     
     protected function error($target, $msg) {
-        throw new HTMLPurifier_ConfigSchema_Exception(ucfirst($target) . ' in ' . $this->getFormattedContext() . ' ' . $msg);
+        if ($target !== false) $prefix = ucfirst($target) . ' in ' .  $this->getFormattedContext();
+        else $prefix = ucfirst($this->getFormattedContext());
+        throw new HTMLPurifier_ConfigSchema_Exception(trim($prefix . ' ' . $msg));
     }
     
     protected function getFormattedContext() {

tests/HTMLPurifier/ConfigSchema/Validator/directive/defaultType.vtest

@@ -1,4 +1,4 @@
-ERROR: Expected type string, got integer in DEFAULT in directive hash 'Ns.Dir'
+ERROR: Expected type string, got integer in TYPE/DEFAULT in directive hash 'Ns.Dir'
 ----
 Ns
 DESCRIPTION: Namespace

tests/HTMLPurifier/ConfigSchema/Validator/directive/typeExists.vtest

@@ -1,4 +1,4 @@
-ERROR: Invalid type 'foobar' in DEFAULT in directive hash 'Ns.Dir'
+ERROR: Invalid type 'foobar' in TYPE/DEFAULT in directive hash 'Ns.Dir'
 ----
 Ns
 DESCRIPTION: Namespace

tests/HTMLPurifier/ConfigSchema/ValidatorTest.php

@@ -0,0 +1,110 @@
+<?php
+
+/**
+ * Special test-case for cases that can't be tested using
+ * HTMLPurifier_ConfigSchema_ValidatorTestCase.
+ */
+class HTMLPurifier_ConfigSchema_ValidatorTest extends UnitTestCase
+{
+    public $validator, $interchange;
+    
+    public function setup() {
+        $this->validator = new HTMLPurifier_ConfigSchema_Validator();
+        $this->interchange = new HTMLPurifier_ConfigSchema_Interchange();
+    }
+    
+    public function testNamespaceIntegrityViolation() {
+        $ns = $this->makeNamespace('Ns');
+        $ns->namespace = 'AltNs';
+        $this->expectValidationException("Integrity violation: key 'Ns' does not match internal id 'AltNs'");
+        $this->validator->validate($this->interchange);
+    }
+    
+    public function testNamespaceNamespaceIsString() {
+        $this->makeNamespace(3);
+        $this->expectValidationException("Namespace in namespace '3' must be a string");
+        $this->validator->validate($this->interchange);
+    }
+    
+    public function testNamespaceDescriptionIsString() {
+        $ns = $this->makeNamespace('Ns');
+        $ns->description = 3;
+        $this->expectValidationException("Description in namespace 'Ns' must be a string");
+        $this->validator->validate($this->interchange);
+    }
+    
+    public function testDirectiveIntegrityViolation() {
+        $d = $this->makeDirective('Ns', 'Dir');
+        $d->id = new HTMLPurifier_ConfigSchema_Interchange_Id('Ns', 'Dir2');
+        $this->expectValidationException("Integrity violation: key 'Ns.Dir' does not match internal id 'Ns.Dir2'");
+        $this->validator->validate($this->interchange);
+    }
+    
+    public function testDirectiveIdInstanceof() {
+        // This example is somewhat contrived
+        $this->makeNamespace('Ns');
+        $d = new HTMLPurifier_ConfigSchema_Interchange_Directive();
+        $d->id = 3;
+        $d->default = 0;
+        $d->type = 'int';
+        $d->description = 'Description';
+        $this->interchange->addDirective($d);
+        
+        $this->expectValidationException("Id '3' in directive '3' is not an instance of HTMLPurifier_ConfigSchema_Interchange_Id");
+        $this->validator->validate($this->interchange);
+    }
+    
+    public function testDirectiveTypeNotEmpty() {
+        $this->makeNamespace('Ns');
+        $d = $this->makeDirective('Ns', 'Dir');
+        $d->default = 0;
+        $d->description = 'Description';
+        
+        $this->expectValidationException("Type in directive 'Ns.Dir' must not be empty");
+        $this->validator->validate($this->interchange);
+    }
+    
+    public function testDirectiveDefaultInvalid() {
+        $this->makeNamespace('Ns');
+        $d = $this->makeDirective('Ns', 'Dir');
+        $d->default = 'asdf';
+        $d->type = 'int';
+        $d->description = 'Description';
+        
+        $this->expectValidationException("Default in directive 'Ns.Dir' had error: Expected type int, got string");
+        $this->validator->validate($this->interchange);
+    }
+    
+    public function testDirectiveIdDirectiveIsString() {
+        $this->makeNamespace('Ns');
+        $d = $this->makeDirective('Ns', 3);
+        $d->default = 0;
+        $d->type = 'int';
+        $d->description = 'Description';
+        
+        $this->expectValidationException("Directive in id 'Ns.3' in directive 'Ns.3' must be a string");
+        $this->validator->validate($this->interchange);
+    }
+    
+    // helper functions
+    
+    protected function makeNamespace($n) {
+        $namespace = new HTMLPurifier_ConfigSchema_Interchange_Namespace();
+        $namespace->namespace = $n;
+        $namespace->description = 'Description'; // non-essential, but we won't set it most of the time
+        $this->interchange->addNamespace($namespace);
+        return $namespace;
+    }
+    
+    protected function makeDirective($n, $d) {
+        $directive = new HTMLPurifier_ConfigSchema_Interchange_Directive();
+        $directive->id = new HTMLPurifier_ConfigSchema_Interchange_Id($n, $d);
+        $this->interchange->addDirective($directive);
+        return $directive;
+    }
+    
+    protected function expectValidationException($msg) {
+        $this->expectException(new HTMLPurifier_ConfigSchema_Exception($msg));
+    }
+    
+}