From dcec92e7b383f1cd34f6e39d8846befe2992f682 Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Fri, 25 Aug 2006 02:48:49 +0000 Subject: [PATCH] Fix bug: number spans should not allow zero as a value. This required augmenting HTMLPurifier/AttrDef/Integer.php to have a richer negative/zero/positive specification interface that can be extrapolated to Number and friends. git-svn-id: http://htmlpurifier.org/svnroot/htmlpurifier/trunk@318 48356398-32a2-884e-a903-53898d9a118a --- library/HTMLPurifier/AttrDef/Integer.php | 44 +++++++++++++++---- library/HTMLPurifier/HTMLDefinition.php | 2 +- tests/HTMLPurifier/AttrDef/IntegerTest.php | 37 +++++++++++++--- .../Strategy/ValidateAttributesTest.php | 4 ++ 4 files changed, 72 insertions(+), 15 deletions(-) diff --git a/library/HTMLPurifier/AttrDef/Integer.php b/library/HTMLPurifier/AttrDef/Integer.php index 71ac98c3..d6953d61 100644 --- a/library/HTMLPurifier/AttrDef/Integer.php +++ b/library/HTMLPurifier/AttrDef/Integer.php @@ -13,15 +13,31 @@ class HTMLPurifier_AttrDef_Integer extends HTMLPurifier_AttrDef { /** - * Bool indicating whether or not integers can only be positive. + * Bool indicating whether or not negative values are allowed */ - var $non_negative = false; + var $negative = true; /** - * @param $non_negative bool indicating whether or not only positive + * Bool indicating whether or not zero is allowed */ - function HTMLPurifier_AttrDef_Integer($non_negative = false) { - $this->non_negative = $non_negative; + var $zero = true; + + /** + * Bool indicating whether or not positive values are allowed + */ + var $positive = true; + + /** + * @param $negative Bool indicating whether or not negative values are allowed + * @param $zero Bool indicating whether or not zero is allowed + * @param $positive Bool indicating whether or not positive values are allowed + */ + function HTMLPurifier_AttrDef_Integer( + $negative = true, $zero = true, $positive = true + ) { + $this->negative = $negative; + $this->zero = $zero; + $this->positive = $positive; } function validate($integer, $config, &$context) { @@ -29,15 +45,27 @@ class HTMLPurifier_AttrDef_Integer extends HTMLPurifier_AttrDef $integer = $this->parseCDATA($integer); if ($integer === '') return false; - if ( !$this->non_negative && $integer[0] === '-' ) { + // we could possibly simply typecast it to integer, but there are + // certain fringe cases that must not return an integer. + + // clip leading sign + if ( $this->negative && $integer[0] === '-' ) { $digits = substr($integer, 1); - } elseif( $integer[0] === '+' ) { - $digits = $integer = substr($integer, 1); + if ($digits === '0') $integer = '0'; // rm minus sign for zero + } elseif( $this->positive && $integer[0] === '+' ) { + $digits = $integer = substr($integer, 1); // rm unnecessary plus } else { $digits = $integer; } + // test if it's numeric if (!ctype_digit($digits)) return false; + + // perform scope tests + if (!$this->zero && $integer == 0) return false; + if (!$this->positive && $integer > 0) return false; + if (!$this->negative && $integer < 0) return false; + return $integer; } diff --git a/library/HTMLPurifier/HTMLDefinition.php b/library/HTMLPurifier/HTMLDefinition.php index 374eea07..709663e4 100644 --- a/library/HTMLPurifier/HTMLDefinition.php +++ b/library/HTMLPurifier/HTMLDefinition.php @@ -331,7 +331,7 @@ class HTMLPurifier_HTMLDefinition $this->info['col']->attr['width'] = $this->info['colgroup']->attr['width'] = $e_MultiLength; - $e__NumberSpan = new HTMLPurifier_AttrDef_Integer(true); + $e__NumberSpan = new HTMLPurifier_AttrDef_Integer(false, false, true); $this->info['colgroup']->attr['span'] = $this->info['col']->attr['span'] = $this->info['td']->attr['rowspan'] = diff --git a/tests/HTMLPurifier/AttrDef/IntegerTest.php b/tests/HTMLPurifier/AttrDef/IntegerTest.php index 00d5831c..20d71689 100644 --- a/tests/HTMLPurifier/AttrDef/IntegerTest.php +++ b/tests/HTMLPurifier/AttrDef/IntegerTest.php @@ -16,20 +16,45 @@ class HTMLPurifier_AttrDef_IntegerTest extends HTMLPurifier_AttrDefHarness $this->assertDef('14'); $this->assertDef('+24', '24'); $this->assertDef(' 14 ', '14'); + $this->assertDef('-0', '0'); $this->assertDef('-1.4', false); $this->assertDef('3.4', false); - $this->assertDef('asdf', false); + $this->assertDef('asdf', false); // must not return zero + $this->assertDef('2in', false); // must not return zero } - function testNonNegative() { + function assertRange($negative, $zero, $positive) { + $this->assertDef('-100', $negative); + $this->assertDef('-1', $negative); + $this->assertDef('0', $zero); + $this->assertDef('1', $positive); + $this->assertDef('42', $positive); + } + + function testRange() { - $this->def = new HTMLPurifier_AttrDef_Integer(true); + $this->def = new HTMLPurifier_AttrDef_Integer(false); + $this->assertRange(false, true, true); // non-negative - $this->assertDef('0'); - $this->assertDef('1'); - $this->assertDef('-1', false); + $this->def = new HTMLPurifier_AttrDef_Integer(false, false); + $this->assertRange(false, false, true); // positive + + + // fringe cases + + $this->def = new HTMLPurifier_AttrDef_Integer(false, false, false); + $this->assertRange(false, false, false); // allow none + + $this->def = new HTMLPurifier_AttrDef_Integer(true, false, false); + $this->assertRange(true, false, false); // negative + + $this->def = new HTMLPurifier_AttrDef_Integer(false, true, false); + $this->assertRange(false, true, false); // zero + + $this->def = new HTMLPurifier_AttrDef_Integer(true, true, false); + $this->assertRange(true, true, false); // non-positive } diff --git a/tests/HTMLPurifier/Strategy/ValidateAttributesTest.php b/tests/HTMLPurifier/Strategy/ValidateAttributesTest.php index 6e610fb7..82f87607 100644 --- a/tests/HTMLPurifier/Strategy/ValidateAttributesTest.php +++ b/tests/HTMLPurifier/Strategy/ValidateAttributesTest.php @@ -120,6 +120,10 @@ class HTMLPurifier_Strategy_ValidateAttributesTest extends $inputs[21] = 'Invalid value!'; $expect[21] = 'Invalid value!'; + // test col.span is non-zero + $inputs[22] = ''; + $expect[22] = ''; + $this->assertStrategyWorks($strategy, $inputs, $expect, $config); }