diff --git a/library/HTMLPurifier/ConfigSchema/Interchange.php b/library/HTMLPurifier/ConfigSchema/Interchange.php index 18866dbf..1ae4330b 100644 --- a/library/HTMLPurifier/ConfigSchema/Interchange.php +++ b/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; diff --git a/library/HTMLPurifier/ConfigSchema/InterchangeBuilder.php b/library/HTMLPurifier/ConfigSchema/InterchangeBuilder.php index 5b4ca1dc..4723b1d5 100644 --- a/library/HTMLPurifier/ConfigSchema/InterchangeBuilder.php +++ b/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); } diff --git a/library/HTMLPurifier/ConfigSchema/Validator.php b/library/HTMLPurifier/ConfigSchema/Validator.php index d107623c..c7b29f37 100644 --- a/library/HTMLPurifier/ConfigSchema/Validator.php +++ b/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])) { - $this->error('namespace', 'does not exist'); + if (!$id instanceof HTMLPurifier_ConfigSchema_Interchange_Id) { + // 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(); - if (!isset(HTMLPurifier_VarParser::$types[$d->type])) { - $this->error('type', 'is invalid'); + ->assertNotEmpty(); // handled by InterchangeBuilder + // Much stricter default check, since we're using the base implementation. + // handled by InterchangeBuilder + try { + $this->parser->parse($d->default, $d->type, $d->typeAllowsNull); + } catch (HTMLPurifier_VarParserException $e) { + $this->error('default', 'had error: ' . $e->getMessage()); } - $this->parser->parse($d->default, $d->type, $d->typeAllowsNull); 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() { diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/defaultType.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/defaultType.vtest index 24ccbc61..d051faaf 100644 --- a/tests/HTMLPurifier/ConfigSchema/Validator/directive/defaultType.vtest +++ b/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 diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/typeExists.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/typeExists.vtest index 1b5d4197..d1fd791e 100644 --- a/tests/HTMLPurifier/ConfigSchema/Validator/directive/typeExists.vtest +++ b/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 diff --git a/tests/HTMLPurifier/ConfigSchema/ValidatorTest.php b/tests/HTMLPurifier/ConfigSchema/ValidatorTest.php new file mode 100644 index 00000000..78456bc4 --- /dev/null +++ b/tests/HTMLPurifier/ConfigSchema/ValidatorTest.php @@ -0,0 +1,110 @@ +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)); + } + +}