From 0ee090bc7b88a29a4597bfadd0de49f191dd6a9a Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Fri, 4 Apr 2008 21:33:37 +0000 Subject: [PATCH] [3.1.0] Continue building up validation functions - Remove incorrect parsing of value aliases - Implement most allowed and value alias checks - Add assertIsBool, assertIsArray and assertIsLookup to ValidatorAtom - Publish string types in VarParser git-svn-id: http://htmlpurifier.org/svnroot/htmlpurifier/trunk@1647 48356398-32a2-884e-a903-53898d9a118a --- .../ConfigSchema/InterchangeBuilder.php | 16 ++---- .../HTMLPurifier/ConfigSchema/Validator.php | 57 ++++++++++++++++++- .../ConfigSchema/ValidatorAtom.php | 18 ++++++ library/HTMLPurifier/VarParser.php | 11 ++++ .../Validator/directive/allowedIsString.vtest | 10 ++++ .../Validator/directive/allowedNotEmpty.vtest | 10 ++++ .../typeWithAllowedIsStringType.vtest | 10 ++++ .../typeWithValueAliasesIsStringType.vtest | 10 ++++ .../directive/valueAliasesAliasIsString.vtest | 10 ++++ .../valueAliasesAliasNotAllowed.vtest | 11 ++++ .../directive/valueAliasesNotAliasSelf.vtest | 10 ++++ .../directive/valueAliasesRealAllowed.vtest | 11 ++++ .../directive/valueAliasesRealIsString.vtest | 10 ++++ .../ConfigSchema/ValidatorAtomTest.php | 32 +++++++++++ .../ConfigSchema/ValidatorTest.php | 36 ++++++++++++ 15 files changed, 247 insertions(+), 15 deletions(-) create mode 100644 tests/HTMLPurifier/ConfigSchema/Validator/directive/allowedIsString.vtest create mode 100644 tests/HTMLPurifier/ConfigSchema/Validator/directive/allowedNotEmpty.vtest create mode 100644 tests/HTMLPurifier/ConfigSchema/Validator/directive/typeWithAllowedIsStringType.vtest create mode 100644 tests/HTMLPurifier/ConfigSchema/Validator/directive/typeWithValueAliasesIsStringType.vtest create mode 100644 tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesAliasIsString.vtest create mode 100644 tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesAliasNotAllowed.vtest create mode 100644 tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesNotAliasSelf.vtest create mode 100644 tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesRealAllowed.vtest create mode 100644 tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesRealIsString.vtest diff --git a/library/HTMLPurifier/ConfigSchema/InterchangeBuilder.php b/library/HTMLPurifier/ConfigSchema/InterchangeBuilder.php index 2b2c9ab4..74a53976 100644 --- a/library/HTMLPurifier/ConfigSchema/InterchangeBuilder.php +++ b/library/HTMLPurifier/ConfigSchema/InterchangeBuilder.php @@ -3,6 +3,9 @@ class HTMLPurifier_ConfigSchema_InterchangeBuilder { + /** + * Used for processing DEFAULT, nothing else. + */ protected $varParser; public function __construct($varParser = null) { @@ -70,18 +73,7 @@ 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); - } - } catch (HTMLPurifier_VarParserException $e) { - throw new HTMLPurifier_ConfigSchema_Exception($e->getMessage() . " in $alias => $real in VALUE-ALIASES in directive hash '$id'"); - } + $directive->valueAliases = $this->evalArray($hash->offsetGet('VALUE-ALIASES')); } if (isset($hash['ALIASES'])) { diff --git a/library/HTMLPurifier/ConfigSchema/Validator.php b/library/HTMLPurifier/ConfigSchema/Validator.php index 5130ad52..fdb7b69d 100644 --- a/library/HTMLPurifier/ConfigSchema/Validator.php +++ b/library/HTMLPurifier/ConfigSchema/Validator.php @@ -79,16 +79,67 @@ class HTMLPurifier_ConfigSchema_Validator $this->validateId($d->id); $this->with($d, 'description') ->assertNotEmpty(); + + // BEGIN - handled by InterchangeBuilder $this->with($d, 'type') - ->assertNotEmpty(); // handled by InterchangeBuilder - // Much stricter default check, since we're using the base implementation. - // handled by InterchangeBuilder + ->assertNotEmpty(); + $this->with($d, 'typeAllowsNull') + ->assertIsBool(); try { + // This also tests validity of $d->type $this->parser->parse($d->default, $d->type, $d->typeAllowsNull); } catch (HTMLPurifier_VarParserException $e) { $this->error('default', 'had error: ' . $e->getMessage()); } + // END - handled by InterchangeBuilder + if (!is_null($d->allowed) || !empty($d->valueAliases)) { + // allowed and valueAliases require that we be dealing with + // strings, so check for that early. + if (!isset(HTMLPurifier_VarParser::$stringTypes[$d->type])) { + $this->error('type', 'must be a string type when used with allowed or value aliases'); + } + } + + $this->validateDirectiveAllowed($d); + $this->validateDirectiveValueAliases($d); + + array_pop($this->context); + } + + public function validateDirectiveAllowed($d) { + if (is_null($d->allowed)) return; + $this->with($d, 'allowed') + ->assertNotEmpty() + ->assertIsLookup(); // handled by InterchangeBuilder + $this->context[] = 'allowed'; + foreach ($d->allowed as $val => $x) { + if (!is_string($val)) $this->error("value $val", 'must be a string'); + } + array_pop($this->context); + } + + public function validateDirectiveValueAliases($d) { + if (is_null($d->valueAliases)) return; + $this->with($d, 'valueAliases') + ->assertIsArray(); // handled by InterchangeBuilder + $this->context[] = 'valueAliases'; + foreach ($d->valueAliases as $alias => $real) { + if (!is_string($alias)) $this->error("alias $alias", 'must be a string'); + if (!is_string($real)) $this->error("alias target $real from alias '$alias'", 'must be a string'); + if ($alias === $real) { + $this->error("alias '$alias'", "must not be an alias to itself"); + } + } + if (!is_null($d->allowed)) { + foreach ($d->valueAliases as $alias => $real) { + if (isset($d->allowed[$alias])) { + $this->error("alias '$alias'", 'must not be an allowed value'); + } elseif (!isset($d->allowed[$real])) { + $this->error("alias '$alias'", 'must be an alias to an allowed value'); + } + } + } array_pop($this->context); } diff --git a/library/HTMLPurifier/ConfigSchema/ValidatorAtom.php b/library/HTMLPurifier/ConfigSchema/ValidatorAtom.php index ac8d3e5d..2bb99676 100644 --- a/library/HTMLPurifier/ConfigSchema/ValidatorAtom.php +++ b/library/HTMLPurifier/ConfigSchema/ValidatorAtom.php @@ -23,6 +23,16 @@ class HTMLPurifier_ConfigSchema_ValidatorAtom return $this; } + public function assertIsBool() { + if (!is_bool($this->contents)) $this->error('must be a boolean'); + return $this; + } + + public function assertIsArray() { + if (!is_array($this->contents)) $this->error('must be an array'); + return $this; + } + public function assertNotNull() { if ($this->contents === null) $this->error('must not be null'); return $this; @@ -39,6 +49,14 @@ class HTMLPurifier_ConfigSchema_ValidatorAtom return $this; } + public function assertIsLookup() { + $this->assertIsArray(); + foreach ($this->contents as $v) { + if ($v !== true) $this->error('must be a lookup array'); + } + return $this; + } + protected function error($msg) { throw new HTMLPurifier_ConfigSchema_Exception(ucfirst($this->member) . ' in ' . $this->context . ' ' . $msg); } diff --git a/library/HTMLPurifier/VarParser.php b/library/HTMLPurifier/VarParser.php index 479d18ad..418622be 100644 --- a/library/HTMLPurifier/VarParser.php +++ b/library/HTMLPurifier/VarParser.php @@ -24,6 +24,17 @@ class HTMLPurifier_VarParser 'mixed' => true ); + /** + * Lookup table of types that are string, and can have aliases or + * allowed value lists. + */ + static public $stringTypes = array( + 'string' => true, + 'istring' => true, + 'text' => true, + 'itext' => true, + ); + /** * Validate a variable according to type. Throws * HTMLPurifier_VarParserException if invalid. diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/allowedIsString.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/allowedIsString.vtest new file mode 100644 index 00000000..09e175cf --- /dev/null +++ b/tests/HTMLPurifier/ConfigSchema/Validator/directive/allowedIsString.vtest @@ -0,0 +1,10 @@ +ERROR: Value 3 in allowed in directive 'Ns.Dir' must be a string +---- +Ns +DESCRIPTION: Generic namespace. +---- +ID: Ns.Dir +TYPE: string +DESCRIPTION: Description +DEFAULT: 'asdf' +ALLOWED: 'asdf', 3 diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/allowedNotEmpty.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/allowedNotEmpty.vtest new file mode 100644 index 00000000..bd27e9f1 --- /dev/null +++ b/tests/HTMLPurifier/ConfigSchema/Validator/directive/allowedNotEmpty.vtest @@ -0,0 +1,10 @@ +ERROR: Allowed in directive 'Ns.Dir' must not be empty +---- +Ns +DESCRIPTION: Generic namespace. +---- +ID: Ns.Dir +TYPE: string +DESCRIPTION: Description +DEFAULT: 'asdf' +ALLOWED: diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/typeWithAllowedIsStringType.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/typeWithAllowedIsStringType.vtest new file mode 100644 index 00000000..6ec1ecd8 --- /dev/null +++ b/tests/HTMLPurifier/ConfigSchema/Validator/directive/typeWithAllowedIsStringType.vtest @@ -0,0 +1,10 @@ +ERROR: Type in directive 'Ns.Dir' must be a string type when used with allowed or value aliases +---- +Ns +DESCRIPTION: Namespace +---- +Ns.Dir +DESCRIPTION: Directive +TYPE: int +DEFAULT: 3 +ALLOWED: 1, 2, 3 diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/typeWithValueAliasesIsStringType.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/typeWithValueAliasesIsStringType.vtest new file mode 100644 index 00000000..5569c217 --- /dev/null +++ b/tests/HTMLPurifier/ConfigSchema/Validator/directive/typeWithValueAliasesIsStringType.vtest @@ -0,0 +1,10 @@ +ERROR: Type in directive 'Ns.Dir' must be a string type when used with allowed or value aliases +---- +Ns +DESCRIPTION: Namespace +---- +Ns.Dir +DESCRIPTION: Directive +TYPE: int +DEFAULT: 3 +VALUE-ALIASES: 2 => 3 diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesAliasIsString.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesAliasIsString.vtest new file mode 100644 index 00000000..06eb6ebe --- /dev/null +++ b/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesAliasIsString.vtest @@ -0,0 +1,10 @@ +ERROR: Alias 3 in valueAliases in directive 'Ns.Dir' must be a string +---- +Ns +DESCRIPTION: Namespace +---- +Ns.Dir +DESCRIPTION: Directive +TYPE: string +DEFAULT: 'a' +VALUE-ALIASES: 3 => 'a' diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesAliasNotAllowed.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesAliasNotAllowed.vtest new file mode 100644 index 00000000..afb06bd3 --- /dev/null +++ b/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesAliasNotAllowed.vtest @@ -0,0 +1,11 @@ +ERROR: Alias 'b' in valueAliases in directive 'Ns.Dir' must not be an allowed value +---- +Ns +DESCRIPTION: Namespace +---- +Ns.Dir +DESCRIPTION: Directive +TYPE: string +DEFAULT: 'a' +ALLOWED: 'a', 'b', 'c' +VALUE-ALIASES: 'b' => 'c' diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesNotAliasSelf.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesNotAliasSelf.vtest new file mode 100644 index 00000000..354e3317 --- /dev/null +++ b/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesNotAliasSelf.vtest @@ -0,0 +1,10 @@ +ERROR: Alias 'bar' in valueAliases in directive 'Ns.Dir' must not be an alias to itself +---- +Ns +DESCRIPTION: Namespace +---- +Ns.Dir +DESCRIPTION: Directive +TYPE: string +DEFAULT: 'foo' +VALUE-ALIASES: 'bar' => 'bar' diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesRealAllowed.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesRealAllowed.vtest new file mode 100644 index 00000000..26c1b1de --- /dev/null +++ b/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesRealAllowed.vtest @@ -0,0 +1,11 @@ +ERROR: Alias 'c' in valueAliases in directive 'Ns.Dir' must be an alias to an allowed value +---- +Ns +DESCRIPTION: Namespace +---- +Ns.Dir +DESCRIPTION: Directive +TYPE: string +DEFAULT: 'a' +ALLOWED: 'a', 'b' +VALUE-ALIASES: 'c' => 'd' diff --git a/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesRealIsString.vtest b/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesRealIsString.vtest new file mode 100644 index 00000000..e06fb294 --- /dev/null +++ b/tests/HTMLPurifier/ConfigSchema/Validator/directive/valueAliasesRealIsString.vtest @@ -0,0 +1,10 @@ +ERROR: Alias target 3 from alias 'b' in valueAliases in directive 'Ns.Dir' must be a string +---- +Ns +DESCRIPTION: Namespace +---- +Ns.Dir +DESCRIPTION: Directive +TYPE: string +DEFAULT: 'a' +VALUE-ALIASES: 'b' => 3 diff --git a/tests/HTMLPurifier/ConfigSchema/ValidatorAtomTest.php b/tests/HTMLPurifier/ConfigSchema/ValidatorAtomTest.php index fe6cc262..018d3ba0 100644 --- a/tests/HTMLPurifier/ConfigSchema/ValidatorAtomTest.php +++ b/tests/HTMLPurifier/ConfigSchema/ValidatorAtomTest.php @@ -55,4 +55,36 @@ class HTMLPurifier_ConfigSchema_ValidatorAtomTest extends UnitTestCase $this->makeAtom('')->assertNotEmpty(); } + public function testAssertIsBool() { + $this->makeAtom(false)->assertIsBool(); + } + + public function testAssertIsBoolFail() { + $this->expectValidationException("Property in context must be a boolean"); + $this->makeAtom('0')->assertIsBool(); + } + + public function testAssertIsArray() { + $this->makeAtom(array())->assertIsArray(); + } + + public function testAssertIsArrayFail() { + $this->expectValidationException("Property in context must be an array"); + $this->makeAtom('asdf')->assertIsArray(); + } + + + public function testAssertIsLookup() { + $this->makeAtom(array('foo' => true))->assertIsLookup(); + } + + public function testAssertIsLookupFail() { + $this->expectValidationException("Property in context must be a lookup array"); + $this->makeAtom(array('foo' => 4))->assertIsLookup(); + } + + public function testAssertIsLookupFailIsArray() { + $this->expectValidationException("Property in context must be an array"); + $this->makeAtom('asdf')->assertIsLookup(); + } } diff --git a/tests/HTMLPurifier/ConfigSchema/ValidatorTest.php b/tests/HTMLPurifier/ConfigSchema/ValidatorTest.php index f5fa49e0..663102b9 100644 --- a/tests/HTMLPurifier/ConfigSchema/ValidatorTest.php +++ b/tests/HTMLPurifier/ConfigSchema/ValidatorTest.php @@ -72,6 +72,42 @@ class HTMLPurifier_ConfigSchema_ValidatorTest extends UnitTestCase $this->validator->validate($this->interchange); } + public function testDirectiveTypeAllowsNullIsBool() { + $this->makeNamespace('Ns'); + $d = $this->makeDirective('Ns', 'Dir'); + $d->default = 0; + $d->type = 'int'; + $d->description = 'Description'; + $d->typeAllowsNull = 'yes'; + + $this->expectValidationException("TypeAllowsNull in directive 'Ns.Dir' must be a boolean"); + $this->validator->validate($this->interchange); + } + + public function testDirectiveValueAliasesIsArray() { + $this->makeNamespace('Ns'); + $d = $this->makeDirective('Ns', 'Dir'); + $d->default = 'a'; + $d->type = 'string'; + $d->description = 'Description'; + $d->valueAliases = 2; + + $this->expectValidationException("ValueAliases in directive 'Ns.Dir' must be an array"); + $this->validator->validate($this->interchange); + } + + public function testDirectiveAllowedIsLookup() { + $this->makeNamespace('Ns'); + $d = $this->makeDirective('Ns', 'Dir'); + $d->default = 'foo'; + $d->type = 'string'; + $d->description = 'Description'; + $d->allowed = array('foo' => 1); + + $this->expectValidationException("Allowed in directive 'Ns.Dir' must be a lookup array"); + $this->validator->validate($this->interchange); + } + // helper functions protected function makeNamespace($n) {