[2.1.5] [MFH] Complete the imagecrash added protection fixes

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

INSTALL

@@ -23,8 +23,9 @@ fixes will be issued for the PHP 4 version until August 8, 2008.
 
 These optional extensions can enhance the capabilities of HTML Purifier:
 
-    * iconv : Converts text to and from non-UTF-8 encodings
+    * iconv  : Converts text to and from non-UTF-8 encodings
-    * tidy  : Used for pretty-printing HTML
+    * bcmath : Used for unit conversion and imagecrash protection
+    * tidy   : Used for pretty-printing HTML
 
 
 ---------------------------------------------------------------------------

library/HTMLPurifier/AttrDef/CSS/Length.php

@@ -1,7 +1,21 @@
 <?php
 
-require_once 'HTMLPurifier/AttrDef.php';
+require_once 'HTMLPurifier/Length.php';
-require_once 'HTMLPurifier/AttrDef/CSS/Number.php';
+require_once 'HTMLPurifier/UnitConverter.php';
+
+HTMLPurifier_ConfigSchema::define(
+    'CSS', 'MaxImgLength', '1200px', 'string/null', '
+<p>
+ This parameter sets the maximum allowed length on <code>img</code> tags,
+ effectively the <code>width</code> and <code>height</code> properties.
+ Only absolute units of measurement (in, pt, pc, mm, cm) and pixels (px) are allowed. This is
+ in place to prevent imagecrash attacks, disable with null at your own risk.
+ This directive is similar to %HTML.MaxImgLength, and both should be
+ concurrently edited, although there are
+ subtle differences in the input format (the CSS max is a number with
+ a unit).
+</p>
+');
 
 /**
  * Represents a Length as defined by CSS.
@@ -9,46 +23,40 @@ require_once 'HTMLPurifier/AttrDef/CSS/Number.php';
 class HTMLPurifier_AttrDef_CSS_Length extends HTMLPurifier_AttrDef
 {
     
-    /**
+    var $min, $max;
-     * Valid unit lookup table.
-     * @warning The code assumes all units are two characters long.  Be careful
-     *          if we have to change this behavior!
-     */
-    var $units = array('em' => true, 'ex' => true, 'px' => true, 'in' => true,
-         'cm' => true, 'mm' => true, 'pt' => true, 'pc' => true);
-    /**
-     * Instance of HTMLPurifier_AttrDef_Number to defer number validation to
-     */
-    var $number_def;
     
     /**
-     * @param $non_negative Bool indication whether or not negative values are
+     * @param HTMLPurifier_Length $max Minimum length, or null for no bound. String is also acceptable.
-     *                      allowed.
+     * @param HTMLPurifier_Length $max Maximum length, or null for no bound. String is also acceptable.
      */
-    function HTMLPurifier_AttrDef_CSS_Length($non_negative = false) {
+    function HTMLPurifier_AttrDef_CSS_Length($min = null, $max = null) {
-        $this->number_def = new HTMLPurifier_AttrDef_CSS_Number($non_negative);
+        $this->min = $min !== null ? HTMLPurifier_Length::make($min) : null;
+        $this->max = $max !== null ? HTMLPurifier_Length::make($max) : null;
     }
     
-    function validate($length, $config, &$context) {
+    function validate($string, $config, $context) {
+        $string = $this->parseCDATA($string);
         
-        $length = $this->parseCDATA($length);
+        // Optimizations
-        if ($length === '') return false;
+        if ($string === '') return false;
-        if ($length === '0') return '0';
+        if ($string === '0') return '0';
-        $strlen = strlen($length);
+        if (strlen($string) === 1) return false;
-        if ($strlen === 1) return false; // impossible!
         
-        // we assume all units are two characters
+        $length = HTMLPurifier_Length::make($string);
-        $unit = substr($length, $strlen - 2);
+        if (!$length->isValid()) return false;
-        if (!ctype_lower($unit)) $unit = strtolower($unit);
-        $number = substr($length, 0, $strlen - 2);
         
-        if (!isset($this->units[$unit])) return false;
+        if ($this->min) {
-        
+            $c = $length->compareTo($this->min);
-        $number = $this->number_def->validate($number, $config, $context);
+            if ($c === false) return false;
-        if ($number === false) return false;
+            if ($c < 0) return false;
-        
+        }
-        return $number . $unit;
+        if ($this->max) {
+            $c = $length->compareTo($this->max);
+            if ($c === false) return false;
+            if ($c > 0) return false;
+        }
         
+        return $length->toString();
     }
     
 }

library/HTMLPurifier/AttrDef/CSS/Number.php

@@ -18,6 +18,11 @@ class HTMLPurifier_AttrDef_CSS_Number extends HTMLPurifier_AttrDef
         $this->non_negative = $non_negative;
     }
     
+    /**
+     * @warning Some contexts do not pass $config, $context. These
+     *          variables should not be used without checking HTMLPurifier_Length.
+     *          This might not work properly in PHP4.
+     */
     function validate($number, $config, &$context) {
         
         $number = $this->parseCDATA($number);

library/HTMLPurifier/AttrDef/Switch.php

@@ -0,0 +1,32 @@
+<?php
+
+/**
+ * Decorator that, depending on a token, switches between two definitions.
+ */
+class HTMLPurifier_AttrDef_Switch
+{
+    
+    var $tag;
+    var $withTag, $withoutTag;
+    
+    /**
+     * @param string $tag Tag name to switch upon
+     * @param HTMLPurifier_AttrDef $with_tag Call if token matches tag
+     * @param HTMLPurifier_AttrDef $without_tag Call if token doesn't match, or there is no token
+     */
+    function HTMLPurifier_AttrDef_Switch($tag, $with_tag, $without_tag) {
+        $this->tag = $tag;
+        $this->withTag = $with_tag;
+        $this->withoutTag = $without_tag;
+    }
+    
+    function validate($string, $config, $context) {
+        $token = $context->get('CurrentToken', true);
+        if (!$token || $token->name !== $this->tag) {
+            return $this->withoutTag->validate($string, $config, $context);
+        } else {
+            return $this->withTag->validate($string, $config, $context);
+        }
+    }
+    
+}

library/HTMLPurifier/CSSDefinition.php

@@ -117,7 +117,7 @@ class HTMLPurifier_CSSDefinition extends HTMLPurifier_Definition
         $this->info['border-left-width'] = 
         $this->info['border-right-width'] = new HTMLPurifier_AttrDef_CSS_Composite(array(
             new HTMLPurifier_AttrDef_Enum(array('thin', 'medium', 'thick')),
-            new HTMLPurifier_AttrDef_CSS_Length(true) //disallow negative
+            new HTMLPurifier_AttrDef_CSS_Length('0') //disallow negative
         ));
         
         $this->info['border-width'] = new HTMLPurifier_AttrDef_CSS_Multiple($border_width);
@@ -143,7 +143,7 @@ class HTMLPurifier_CSSDefinition extends HTMLPurifier_Definition
         $this->info['line-height'] = new HTMLPurifier_AttrDef_CSS_Composite(array(
             new HTMLPurifier_AttrDef_Enum(array('normal')),
             new HTMLPurifier_AttrDef_CSS_Number(true), // no negatives
-            new HTMLPurifier_AttrDef_CSS_Length(true),
+            new HTMLPurifier_AttrDef_CSS_Length('0'),
             new HTMLPurifier_AttrDef_CSS_Percentage(true)
         ));
         
@@ -165,7 +165,7 @@ class HTMLPurifier_CSSDefinition extends HTMLPurifier_Definition
         $this->info['padding-bottom'] = 
         $this->info['padding-left'] = 
         $this->info['padding-right'] = new HTMLPurifier_AttrDef_CSS_Composite(array(
-            new HTMLPurifier_AttrDef_CSS_Length(true),
+            new HTMLPurifier_AttrDef_CSS_Length('0'),
             new HTMLPurifier_AttrDef_CSS_Percentage(true)
         ));
         
@@ -178,12 +178,19 @@ class HTMLPurifier_CSSDefinition extends HTMLPurifier_Definition
         
         $this->info['width'] =
         $this->info['height'] =
-        new HTMLPurifier_AttrDef_CSS_DenyElementDecorator(
+        new HTMLPurifier_AttrDef_Switch('img',
-        new HTMLPurifier_AttrDef_CSS_Composite(array(
+            // For img tags:
-            new HTMLPurifier_AttrDef_CSS_Length(true),
+            new HTMLPurifier_AttrDef_CSS_Composite(array(
-            new HTMLPurifier_AttrDef_CSS_Percentage(true),
+                new HTMLPurifier_AttrDef_CSS_Length('0', $config->get('CSS', 'MaxImgLength')),
-            new HTMLPurifier_AttrDef_Enum(array('auto'))
+                new HTMLPurifier_AttrDef_Enum(array('auto'))
-        )), 'img');
+            )),
+            // For everyone else:
+            new HTMLPurifier_AttrDef_CSS_Composite(array(
+                new HTMLPurifier_AttrDef_CSS_Length('0'),
+                new HTMLPurifier_AttrDef_CSS_Percentage(true),
+                new HTMLPurifier_AttrDef_Enum(array('auto'))
+            ))
+        );
         
         $this->info['text-decoration'] = new HTMLPurifier_AttrDef_CSS_TextDecoration();
         

library/HTMLPurifier/HTMLModule/Image.php

@@ -20,10 +20,13 @@ class HTMLPurifier_HTMLModule_Image extends HTMLPurifier_HTMLModule
             'img', true, 'Inline', 'Empty', 'Common',
             array(
                 'alt*' => 'Text',
-                'height' => 'Length',
+                // According to the spec, it's Length, but percents can
+                // be abused, so we allow only Pixels. A trusted module
+                // could overload this with the real value.
+                'height' => 'Pixels',
+                'width' => 'Pixels',
                 'longdesc' => 'URI', 
                 'src*' => new HTMLPurifier_AttrDef_URI(true), // embedded
-                'width' => 'Length'
             )
         );
         // kind of strange, but splitting things up would be inefficient

library/HTMLPurifier/Length.php

@@ -2,7 +2,7 @@
 
 /**
  * Represents a measurable length, with a string numeric magnitude
- * and a unit.
+ * and a unit. This object is immutable.
  */
 class HTMLPurifier_Length
 {
@@ -18,12 +18,17 @@ class HTMLPurifier_Length
     var $unit;
     
     /**
+     * Whether or not this length is valid. Null if not calculated yet.
+     */
+    var $isValid;
+    
+    /*
      * @param number $n Magnitude
      * @param string $u Unit
      */
     function HTMLPurifier_Length($n = '0', $u = false) {
-        $this->n = $n;
+        $this->n = (string) $n;
-        $this->unit = $u;
+        $this->unit = $u !== false ? (string) $u : false;
     }
     
     /**
@@ -31,6 +36,7 @@ class HTMLPurifier_Length
      * @warning Does not perform validation.
      */
     function make($s) {
+        if (is_a($s, 'HTMLPurifier_Length')) return $s;
         $n_length = strspn($s, '1234567890.+-');
         $n = substr($s, 0, $n_length);
         $unit = substr($s, $n_length);
@@ -40,20 +46,22 @@ class HTMLPurifier_Length
     
     /**
      * Validates the number and unit.
-     * @param bool $non_negative Whether or not to disable negative values.
-     * @note Maybe should be put in another class.
      */
-    function validate($non_negative = false, $config, $context) {
+    function validate() {
         // Special case:
+
         static $allowedUnits = array(
             'em' => true, 'ex' => true, 'px' => true, 'in' => true,
             'cm' => true, 'mm' => true, 'pt' => true, 'pc' => true
         );
+        if ($this->n === '+0' || $this->n === '-0') $this->n = '0';
         if ($this->n === '0' && $this->unit === false) return true;
         if (!ctype_lower($this->unit)) $this->unit = strtolower($this->unit);
         if (!isset($allowedUnits[$this->unit])) return false;
-        $def = new HTMLPurifier_AttrDef_CSS_Number($non_negative);
+        // Hack:
-        $result = $def->validate($this->n, $config, $context);
+        $def = new HTMLPurifier_AttrDef_CSS_Number();
+        $a = false; // hack hack
+        $result = $def->validate($this->n, $a, $a);
         if ($result === false) return false;
         $this->n = $result;
         return true;
@@ -63,7 +71,41 @@ class HTMLPurifier_Length
      * Returns string representation of number.
      */
     function toString() {
+        if (!$this->isValid()) return false;
         return $this->n . $this->unit;
     }
     
+    /**
+     * Retrieves string numeric magnitude.
+     */
+    function getN() {return $this->n;}
+    
+    /**
+     * Retrieves string unit.
+     */
+    function getUnit() {return $this->unit;}
+    
+    /**
+     * Returns true if this length unit is valid.
+     */
+    function isValid() {
+        if ($this->isValid === null) $this->isValid = $this->validate();
+        return $this->isValid;
+    }
+    
+    /**
+     * Compares two lengths, and returns 1 if greater, -1 if less and 0 if equal.
+     * @warning If both values are too large or small, this calculation will
+     *          not work properly
+     */
+    function compareTo($l) {
+        if ($l === false) return false;
+        if ($l->unit !== $this->unit) {
+            $converter = new HTMLPurifier_UnitConverter();
+            $l = $converter->convert($l, $this->unit);
+            if ($l === false) return false;
+        }
+        return $this->n - $l->n;
+    }
+    
 }

library/HTMLPurifier/UnitConverter.php

@@ -1,7 +1,5 @@
 <?php
 
-require_once 'HTMLPurifier/Length.php';
-
 /**
  * Class for converting between different unit-lengths as specified by
  * CSS.
@@ -9,10 +7,6 @@ require_once 'HTMLPurifier/Length.php';
 class HTMLPurifier_UnitConverter
 {
     
-    // For documentation purposes: these are magical numbers
-    // const ENGLISH = 1;
-    // const METRIC = 2;
-    
     /**
      * Minimum bcmath precision for output.
      */
@@ -23,13 +17,24 @@ class HTMLPurifier_UnitConverter
      */
     var $internalPrecision;
     
-    function HTMLPurifier_UnitConverter($output_precision = 4, $internal_precision = 10) {
+    /**
+     * Whether or not BCMath is available
+     */
+    var $bcmath;
+    
+    function HTMLPurifier_UnitConverter($output_precision = 4, $internal_precision = 10, $force_no_bcmath = false) {
         $this->outputPrecision = $output_precision;
         $this->internalPrecision = $internal_precision;
+        $this->bcmath = !$force_no_bcmath && function_exists('bcmul');
     }
     
     /**
      * Converts a length object of one unit into another unit.
+     * @param HTMLPurifier_Length $length
+     *      Instance of HTMLPurifier_Length to convert. You must validate()
+     *      it before passing it here!
+     * @param string $to_unit
+     *      Unit to convert to.
      * @note
      *      About precision: This conversion function pays very special
      *      attention to the incoming precision of values and attempts
@@ -40,9 +45,6 @@ class HTMLPurifier_UnitConverter
      *          - If a number contains less than four sigfigs ($outputPrecision)
      *            and this causes some decimals to be excluded, those
      *            decimals will be added on.
-     *          - Significant digits will be ignored for quantities greater
-     *            than one. This is a limitation of BCMath and I don't
-     *            feel like coding around it.
      */
     function convert($length, $to_unit) {
         
@@ -57,9 +59,10 @@ class HTMLPurifier_UnitConverter
          */
         static $units = array(
             1 => array(
-                'pt' => 1,
+                'px' => 3, // This is as per CSS 2.1 and Firefox. Your mileage may vary
-                'pc' => 12,
+                'pt' => 4,
-                'in' => 72,
+                'pc' => 48,
+                'in' => 288,
                 2 => array('pt', '0.352777778', 'mm'),
             ),
             2 => array(
@@ -69,34 +72,32 @@ class HTMLPurifier_UnitConverter
             ),
         );
         
-        if ($length->n === '0' || $length->unit === false) {
+        if (!$length->isValid()) return false;
-            return new HTMLPurifier_Length('0', $unit);
+        
+        $n    = $length->getN();
+        $unit = $length->getUnit();
+        
+        if ($n === '0' || $unit === false) {
+            return new HTMLPurifier_Length('0', false);
         }
         
-        $state = $dest = false;
+        $state = $dest_state = false;
         foreach ($units as $k => $x) {
-            if (isset($x[$length->unit])) $state = $k;
+            if (isset($x[$unit])) $state = $k;
             if (isset($x[$to_unit])) $dest_state = $k;
         }
         if (!$state || !$dest_state) return false;
         
-        $n    = $length->n;
-        $unit = $length->unit;
-        
         // Some calculations about the initial precision of the number;
         // this will be useful when we need to do final rounding.
-        $log = (int) floor(log($n, 10));
+        $sigfigs = $this->getSigFigs($n);
-        if (strpos($n, '.') === false) {
-            $sigfigs = strlen(trim($n, '0+-'));
-        } else {
-            $sigfigs = strlen(ltrim($n, '0+-')) - 1; // eliminate extra decimal character
-        }
         if ($sigfigs < $this->outputPrecision) $sigfigs = $this->outputPrecision;
         
         // BCMath's internal precision deals only with decimals. Use
         // our default if the initial number has no decimals, or increase
         // it by how ever many decimals, thus, the number of guard digits
         // will always be greater than or equal to internalPrecision.
+        $log = (int) floor(log(abs($n), 10));
         $cp = ($log < 0) ? $this->internalPrecision - $log : $this->internalPrecision; // internal precision
         
         for ($i = 0; $i < 2; $i++) {
@@ -112,12 +113,12 @@ class HTMLPurifier_UnitConverter
             
             // Do the conversion if necessary
             if ($dest_unit !== $unit) {
-                $factor = bcdiv($units[$state][$unit], $units[$state][$dest_unit], $cp);
+                $factor = $this->div($units[$state][$unit], $units[$state][$dest_unit], $cp);
-                $n = bcmul($n, $factor, $cp);
+                $n = $this->mul($n, $factor, $cp);
                 $unit = $dest_unit;
             }
             
-            // Output was zero, so bail out early
+            // Output was zero, so bail out early. Shouldn't ever happen.
             if ($n === '') {
                 $n = '0';
                 $unit = $to_unit;
@@ -138,7 +139,7 @@ class HTMLPurifier_UnitConverter
             // Pre-condition: $i == 0
             
             // Perform conversion to next system of units
-            $n = bcmul($n, $units[$state][$dest_state][1], $cp);
+            $n = $this->mul($n, $units[$state][$dest_state][1], $cp);
             $unit = $units[$state][$dest_state][2];
             $state = $dest_state;
             
@@ -149,20 +150,87 @@ class HTMLPurifier_UnitConverter
         // Post-condition: $unit == $to_unit
         if ($unit !== $to_unit) return false;
         
-        // Calculate how many decimals we need ($rp)
+        // Useful for debugging:
-        // Calculations will always be carried to the decimal; this is
+        //echo "<pre>n";
-        // a limitation with BC (we can't set the scale to be negative)
+        //echo "$n\nsigfigs = $sigfigs\nnew_log = $new_log\nlog = $log\nrp = $rp\n</pre>\n";
-        $new_log = (int) floor(log($n, 10));
         
-        $rp = $sigfigs - $new_log - $log - 1;
+        $n = $this->round($n, $sigfigs);
-        if ($rp < 0) $rp = 0;
-        
-        $n = bcadd($n, '0.' .  str_repeat('0', $rp) . '5', $rp + 1);
-        $n = bcdiv($n, '1', $rp);
         if (strpos($n, '.') !== false) $n = rtrim($n, '0');
         $n = rtrim($n, '.');
         
         return new HTMLPurifier_Length($n, $unit);
     }
     
+    /**
+     * Returns the number of significant figures in a string number.
+     * @param string $n Decimal number
+     * @return int number of sigfigs
+     */
+    function getSigFigs($n) {
+        $n = ltrim($n, '0+-');
+        $dp = strpos($n, '.'); // decimal position
+        if ($dp === false) {
+            $sigfigs = strlen(rtrim($n, '0'));
+        } else {
+            $sigfigs = strlen(ltrim($n, '0.')); // eliminate extra decimal character
+            if ($dp !== 0) $sigfigs--;
+        }
+        return $sigfigs;
+    }
+    
+    /**
+     * Adds two numbers, using arbitrary precision when available.
+     */
+    function add($s1, $s2, $scale) {
+        if ($this->bcmath) return bcadd($s1, $s2, $scale);
+        else return $this->scale($s1 + $s2, $scale);
+    }
+    
+    /**
+     * Multiples two numbers, using arbitrary precision when available.
+     */
+    function mul($s1, $s2, $scale) {
+        if ($this->bcmath) return bcmul($s1, $s2, $scale);
+        else return $this->scale($s1 * $s2, $scale);
+    }
+    
+    /**
+     * Divides two numbers, using arbitrary precision when available.
+     */
+    function div($s1, $s2, $scale) {
+        if ($this->bcmath) return bcdiv($s1, $s2, $scale);
+        else return $this->scale($s1 / $s2, $scale);
+    }
+    
+    /**
+     * Rounds a number according to the number of sigfigs it should have,
+     * using arbitrary precision when available.
+     */
+    function round($n, $sigfigs) {
+        $new_log = (int) floor(log(abs($n), 10)); // Number of digits left of decimal - 1
+        $rp = $sigfigs - $new_log - 1; // Number of decimal places needed
+        $neg = $n < 0 ? '-' : ''; // Negative sign
+        if ($this->bcmath) {
+            if ($rp >= 0) {
+                $n = bcadd($n, $neg . '0.' .  str_repeat('0', $rp) . '5', $rp + 1);
+                $n = bcdiv($n, '1', $rp);
+            } else {
+                // This algorithm partially depends on the standardized
+                // form of numbers that comes out of bcmath.
+                $n = bcadd($n, $neg . '5' . str_repeat('0', $new_log - $sigfigs), 0);
+                $n = substr($n, 0, $sigfigs + strlen($neg)) . str_repeat('0', $new_log - $sigfigs + 1);
+            }
+            return $n;
+        } else {
+            return $this->scale(round($n, $sigfigs - $new_log - 1), $rp + 1);
+        }
+    }
+    
+    /**
+     * Scales a float to $scale digits right of decimal point, like BCMath.
+     */
+    function scale($r, $scale) {
+        return sprintf('%.' . $scale . 'f', (float) $r);
+    }
+    
 }

tests/HTMLPurifier/AttrDef/CSS/LengthTest.php

@@ -31,12 +31,20 @@ class HTMLPurifier_AttrDef_CSS_LengthTest extends HTMLPurifier_AttrDefHarness
     
     function testNonNegative() {
         
-        $this->def = new HTMLPurifier_AttrDef_CSS_Length(true);
+        $this->def = new HTMLPurifier_AttrDef_CSS_Length('0');
         
         $this->assertDef('3cm');
         $this->assertDef('-3mm', false);
         
     }
     
+    function testBounding() {
+        $this->def = new HTMLPurifier_AttrDef_CSS_Length('-1in', '1in');
+        $this->assertDef('1cm');
+        $this->assertDef('-1cm');
+        $this->assertDef('0');
+        $this->assertDef('1em', false);
+    }
+    
 }
 

tests/HTMLPurifier/AttrDef/SwitchTest.php

@@ -0,0 +1,34 @@
+<?php
+
+require_once 'HTMLPurifier/AttrDef/Switch.php';
+
+class HTMLPurifier_AttrDef_SwitchTest extends HTMLPurifier_AttrDefHarness
+{
+    
+    var $with, $without;
+    
+    function setUp() {
+        parent::setUp();
+        generate_mock_once('HTMLPurifier_AttrDef');
+        $this->with = new HTMLPurifier_AttrDefMock();
+        $this->without = new HTMLPurifier_AttrDefMock();
+        $this->def = new HTMLPurifier_AttrDef_Switch('tag', $this->with, $this->without);
+    }
+    
+    function testWith() {
+        $token = new HTMLPurifier_Token_Start('tag');
+        $this->context->register('CurrentToken', $token);
+        $this->with->expectOnce('validate');
+        $this->with->setReturnValue('validate', 'foo');
+        $this->assertDef('bar', 'foo');
+    }
+    
+    function testWithout() {
+        $token = new HTMLPurifier_Token_Start('other-tag');
+        $this->context->register('CurrentToken', $token);
+        $this->without->expectOnce('validate');
+        $this->without->setReturnValue('validate', 'foo');
+        $this->assertDef('bar', 'foo');
+    }
+    
+}

tests/HTMLPurifier/LengthTest.php

@@ -7,14 +7,14 @@ class HTMLPurifier_LengthTest extends HTMLPurifier_Harness
     
     function testConstruct() {
         $l = new HTMLPurifier_Length('23', 'in');
-        $this->assertIdentical($l->n, '23');
+        $this->assertIdentical($l->getN(), '23');
-        $this->assertIdentical($l->unit, 'in');
+        $this->assertIdentical($l->getUnit(), 'in');
     }
     
     function testMake() {
         $l = HTMLPurifier_Length::make('+23.4in');
-        $this->assertIdentical($l->n, '+23.4');
+        $this->assertIdentical($l->getN(), '+23.4');
-        $this->assertIdentical($l->unit, 'in');
+        $this->assertIdentical($l->getUnit(), 'in');
     }
     
     function testToString() {
@@ -22,16 +22,18 @@ class HTMLPurifier_LengthTest extends HTMLPurifier_Harness
         $this->assertIdentical($l->toString(), '23in');
     }
     
-    function assertValidate($string, $expect = true, $disable_negative = false) {
+    function assertValidate($string, $expect = true) {
         if ($expect === true) $expect = $string;
         $l = HTMLPurifier_Length::make($string);
-        $result = $l->validate($disable_negative, $this->config, $this->context);
+        $result = $l->isValid();
         if ($result === false) $this->assertIdentical($expect, false);
         else $this->assertIdentical($l->toString(), $expect);
     }
     
     function testValidate() {
         $this->assertValidate('0');
+        $this->assertValidate('+0', '0');
+        $this->assertValidate('-0', '0');
         $this->assertValidate('0px');
         $this->assertValidate('4.5px');
         $this->assertValidate('-4.5px');
@@ -45,7 +47,27 @@ class HTMLPurifier_LengthTest extends HTMLPurifier_Harness
         $this->assertValidate('3PX', '3px');
         $this->assertValidate('3', false);
         $this->assertValidate('3miles', false);
-        $this->assertValidate('-3mm', false, true); // no-negatives
+    }
+    
+    /**
+     * @param $s1 First string to compare
+     * @param $s2 Second string to compare
+     * @param $expect 0 for $s1 == $s2, 1 for $s1 > $s2 and -1 for $s1 < $s2
+     */
+    function assertComparison($s1, $s2, $expect = 0) {
+        $l1 = HTMLPurifier_Length::make($s1);
+        $l2 = HTMLPurifier_Length::make($s2);
+        $r1 = $l1->compareTo($l2);
+        $r2 = $l2->compareTo($l1);
+        $this->assertIdentical($r1 == 0 ? 0 : ($r1 > 0 ? 1 : -1), $expect);
+        $this->assertIdentical($r2 == 0 ? 0 : ($r2 > 0 ? 1 : -1), - $expect);
+    }
+    
+    function testCompareTo() {
+        $this->assertComparison('12in', '12in');
+        $this->assertComparison('12in', '12mm', 1);
+        $this->assertComparison('1px', '1mm', -1);
+        $this->assertComparison(str_repeat('2', 38) . 'in', '100px', 1);
     }
     
 }

tests/HTMLPurifier/Strategy/ValidateAttributesTest.php

@@ -180,9 +180,44 @@ class HTMLPurifier_Strategy_ValidateAttributesTest extends
         );
     }
     
-    function testRemoveCSSWidthAndHeightOnImg() {
+    function testKeepAbsoluteCSSWidthAndHeightOnImg() {
         $this->assertResult(
-            '<img src="" alt="" style="width:10px;height:10px;border:1px solid #000;" />',
+            '<img src="" alt="" style="width:10px;height:10px;border:1px solid #000;" />'
+        );
+    }
+    
+    function testRemoveLargeCSSWidthAndHeightOnImg() {
+        $this->assertResult(
+            '<img src="" alt="" style="width:10000000px;height:10000000px;border:1px solid #000;" />',
+            '<img src="" alt="" style="border:1px solid #000;" />'
+        );
+    }
+    
+    function testRemoveLargeCSSWidthAndHeightOnImgWithUserConf() {
+        $this->config->set('CSS', 'MaxImgLength', '1px');
+        $this->assertResult(
+            '<img src="" alt="" style="width:1mm;height:1mm;border:1px solid #000;" />',
+            '<img src="" alt="" style="border:1px solid #000;" />'
+        );
+    }
+    
+    function testKeepLargeCSSWidthAndHeightOnImgWhenToldTo() {
+        $this->config->set('CSS', 'MaxImgLength', null);
+        $this->assertResult(
+            '<img src="" alt="" style="width:10000000px;height:10000000px;border:1px solid #000;" />'
+        );
+    }
+    
+    function testRemoveRelativeCSSWidthAndHeightOnImg() {
+        $this->assertResult(
+            '<img src="" alt="" style="width:10em;height:10em;border:1px solid #000;" />',
+            '<img src="" alt="" style="border:1px solid #000;" />'
+        );
+    }
+    
+    function testRemovePercentCSSWidthAndHeightOnImg() {
+        $this->assertResult(
+            '<img src="" alt="" style="width:100%;height:100%;border:1px solid #000;" />',
             '<img src="" alt="" style="border:1px solid #000;" />'
         );
     }

tests/HTMLPurifier/UnitConverterTest.php

@@ -5,12 +5,44 @@ require_once 'HTMLPurifier/UnitConverter.php';
 class HTMLPurifier_UnitConverterTest extends HTMLPurifier_Harness
 {
     
-    function assertConversion($input, $expect) {
+    function assertConversion($input, $expect, $unit = null, $test_negative = true) {
-        $input = HTMLPurifier_Length::make($input);
+        $length = HTMLPurifier_Length::make($input);
-        $expect = HTMLPurifier_Length::make($expect);
+        if ($expect !== false) $expectl = HTMLPurifier_Length::make($expect);
-        $converter = new HTMLPurifier_UnitConverter();
+        else $expectl = false;
-        $result = $converter->convert($input, $expect->unit);
+        $to_unit = $unit !== null ? $unit : $expectl->getUnit();
-        $this->assertIdentical($result, $expect);
+        
+        $converter = new HTMLPurifier_UnitConverter(4, 10);
+        $result = $converter->convert($length, $to_unit);
+        if (!$result || !$expectl) $this->assertIdentical($result, $expectl);
+        else $this->assertIdentical($result->toString(), $expectl->toString());
+        
+        $converter = new HTMLPurifier_UnitConverter(4, 10, true);
+        $result = $converter->convert($length, $to_unit);
+        if (!$result || !$expectl) $this->assertIdentical($result, $expectl);
+        else $this->assertIdentical($result->toString(), $expectl->toString(), 'BCMath substitute: %s');
+        
+        if ($test_negative) {
+            $this->assertConversion(
+                "-$input",
+                $expect === false ? false : "-$expect",
+                $unit,
+                false
+            );
+        }
+    }
+    
+    function testFail() {
+        $this->assertConversion('1in', false, 'foo');
+        $this->assertConversion('1foo', false, 'in');
+    }
+    
+    function testZero() {
+        $this->assertConversion('0', '0', 'in', false);
+        $this->assertConversion('-0', '0', 'in', false);
+        $this->assertConversion('0in', '0', 'in', false);
+        $this->assertConversion('-0in', '0', 'in', false);
+        $this->assertConversion('0in', '0', 'pt', false);
+        $this->assertConversion('-0in', '0', 'pt', false);
     }
     
     function testEnglish() {
@@ -26,6 +58,9 @@ class HTMLPurifier_UnitConverterTest extends HTMLPurifier_Harness
         $this->assertConversion('1pt', '0.01389in');
         $this->assertConversion('1.000pt', '0.01389in');
         $this->assertConversion('100000pt', '1389in');
+        
+        $this->assertConversion('1in', '96px');
+        $this->assertConversion('96px', '1in');
     }
     
     function testMetric() {
@@ -41,4 +76,52 @@ class HTMLPurifier_UnitConverterTest extends HTMLPurifier_Harness
         $this->assertConversion('0.3937in', '1cm');
     }
     
+    function testRoundingMinPrecision() {
+        // One sig-fig, modified to be four, conversion rounds up
+        $this->assertConversion('100pt', '1.389in');
+        $this->assertConversion('1000pt', '13.89in');
+        $this->assertConversion('10000pt', '138.9in');
+        $this->assertConversion('100000pt', '1389in');
+        $this->assertConversion('1000000pt', '13890in');
+    }
+    
+    function testRoundingUserPrecision() {
+        // Five sig-figs, conversion rounds down
+        $this->assertConversion('11112000pt', '154330in');
+        $this->assertConversion('1111200pt', '15433in');
+        $this->assertConversion('111120pt', '1543.3in');
+        $this->assertConversion('11112pt', '154.33in');
+        $this->assertConversion('1111.2pt', '15.433in');
+        $this->assertConversion('111.12pt', '1.5433in');
+        $this->assertConversion('11.112pt', '0.15433in');
+    }
+    
+    function assertSigFig($n, $sigfigs) {
+        $converter = new HTMLPurifier_UnitConverter();
+        $result = $converter->getSigFigs($n);
+        $this->assertIdentical($result, $sigfigs);
+    }
+    
+    function test_getSigFigs() {
+        $this->assertSigFig('0', 0);
+        $this->assertSigFig('1', 1);
+        $this->assertSigFig('-1', 1);
+        $this->assertSigFig('+1', 1);
+        $this->assertSigFig('01', 1);
+        $this->assertSigFig('001', 1);
+        $this->assertSigFig('12', 2);
+        $this->assertSigFig('012', 2);
+        $this->assertSigFig('10', 1);
+        $this->assertSigFig('10.', 2);
+        $this->assertSigFig('100.', 3);
+        $this->assertSigFig('103', 3);
+        $this->assertSigFig('130', 2);
+        $this->assertSigFig('.1', 1);
+        $this->assertSigFig('0.1', 1);
+        $this->assertSigFig('00.1', 1);
+        $this->assertSigFig('0.01', 1);
+        $this->assertSigFig('0.010', 2);
+        $this->assertSigFig('0.012', 2);
+    }
+    
 }