Change semantics of %Attr.AllowedInputTypes directive

Empty array means that no explicit 'type' attribute is allowed on input element.
xemlock · Aug 7, 2022 · a65324d · a65324d
1 parent 204f048
commit a65324d
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 109 deletions.
diff --git a/README.md b/README.md
@@ -62,7 +62,7 @@ Apart from HTML Purifier's built-in [configuration directives](http://htmlpurifi
   Type: [Lookup](http://htmlpurifier.org/live/configdoc/plain.html#type-lookup) (or null)\
   Default: `null`
 
-  List of allowed input types, chosen from the types defined in the spec. By default, the setting is `null`, meaning there is no restriction on allowed types. Empty array means that no input types are allowed, effectively removing `input` elements from the purified output.
+  List of allowed input types, chosen from the types defined in the spec. By default, the setting is `null`, meaning there is no restriction on allowed types. Empty array means that no explicit `type` attributes are allowed, effectively making all inputs a text inputs.
 
 * __HTML.Forms__
 

diff --git a/library/HTMLPurifier/AttrDef/HTML5/InputType.php b/library/HTMLPurifier/AttrDef/HTML5/InputType.php
@@ -21,11 +21,38 @@ class HTMLPurifier_AttrDef_HTML5_InputType extends HTMLPurifier_AttrDef
     );
 
     /**
-     * @return array
+     * Lookup for input types allowed in current configuration
+     * @var array
      */
-    public static function values()
+    protected $allowed;
+
+    protected $allowedFromConfig;
+
+    protected function setupAllowed(HTMLPurifier_Config $config)
     {
-        return self::$values;
+        $allowedFromConfig = isset($config->def->info['Attr.AllowedInputTypes'])
+            ? $config->get('Attr.AllowedInputTypes')
+            : null;
+
+        // Check if current allowed value is based on the latest value from config.
+        // Comparing with '===' shouldn't be a performance bottleneck, because the
+        // value retrieved from the config is never changed after being stored.
+        // PHP's copy-on-write mechanism prevents making unnecessary array copies,
+        // allowing this particular array comparison to be made in O(1) time, when
+        // the corresponding value in config hasn't changed, and in O(n) time after
+        // each change.
+        if ($this->allowed !== null && $this->allowedFromConfig === $allowedFromConfig) {
+            return;
+        }
+
+        if (is_array($allowedFromConfig)) {
+            $allowed = array_intersect_key($allowedFromConfig, self::$values);
+        } else {
+            $allowed = self::$values;
+        }
+
+        $this->allowed = $allowed;
+        $this->allowedFromConfig = $allowedFromConfig;
     }
 
     /**
@@ -36,12 +63,17 @@ public static function values()
      */
     public function validate($string, $config, $context)
     {
+        $this->setupAllowed($config);
         $value = strtolower($this->parseCDATA($string));
 
         if (!isset(self::$values[$value])) {
             return false;
         }
 
+        if (!isset($this->allowed[$value])) {
+            return false;
+        }
+
         return $value;
     }
 }
diff --git a/library/HTMLPurifier/AttrTransform/HTML5/Input.php b/library/HTMLPurifier/AttrTransform/HTML5/Input.php
@@ -216,80 +216,18 @@ class HTMLPurifier_AttrTransform_HTML5_Input extends HTMLPurifier_AttrTransform
         ),
     );
 
-    /**
-     * Lookup for input types allowed in current configuration
-     * @var array
-     */
-    protected $allowedInputTypes;
-
-    protected $allowedInputTypesFromConfig;
-
-    /**
-     * @var HTMLPurifier_AttrDef_HTML5_InputType
-     */
-    protected $inputType;
-
-    public function __construct()
-    {
-        $this->inputType = new HTMLPurifier_AttrDef_HTML5_InputType();
-    }
-
-    protected function setupAllowedInputTypes(HTMLPurifier_Config $config)
-    {
-        $allowedInputTypesFromConfig = isset($config->def->info['Attr.AllowedInputTypes'])
-            ? $config->get('Attr.AllowedInputTypes')
-            : null;
-
-        // Check if current allowedInputTypes value is based on the latest value from config.
-        // Comparing with '===' shouldn't be a performance bottleneck, because the
-        // value retrieved from the config is never changed after being stored.
-        // PHP's copy-on-write mechanism prevents making unnecessary array copies,
-        // allowing this particular array comparison to be made in O(1) time, when
-        // the corresponding value in config hasn't changed, and in O(n) time after
-        // each change.
-        if ($this->allowedInputTypes !== null && $this->allowedInputTypesFromConfig === $allowedInputTypesFromConfig) {
-            return;
-        }
-
-        if (is_array($allowedInputTypesFromConfig)) {
-            $allowedInputTypes = array_intersect_key(
-                $allowedInputTypesFromConfig,
-                HTMLPurifier_AttrDef_HTML5_InputType::values()
-            );
-        } else {
-            $allowedInputTypes = HTMLPurifier_AttrDef_HTML5_InputType::values();
-        }
-
-        $this->allowedInputTypes = $allowedInputTypes;
-        $this->allowedInputTypesFromConfig = $allowedInputTypesFromConfig;
-    }
-
     /**
      * @param array $attr
      * @param HTMLPurifier_Config $config
      * @param HTMLPurifier_Context $context
-     * @return array|bool
+     * @return array
      */
     public function transform($attr, $config, $context)
     {
-        if (isset($attr['type'])) {
-            $t = $this->inputType->validate($attr['type'], $config, $context);
-        } else {
+        if (empty($attr['type'])) {
             $t = 'text';
-        }
-
-        // If an unrecognized input type is provided, use 'text' value instead
-        // and remove the 'type' attribute
-        if ($t === false) {
-            unset($attr['type']);
-            $t = 'text';
-        }
-
-        // If type doesn't pass %Attr.AllowedInputTypes validation, remove the element
-        // from the output
-        $this->setupAllowedInputTypes($config);
-        if (!isset($this->allowedInputTypes[$t])) {
-            return false;
+        } else {
+            $t = strtolower($attr['type']);
         }
 
         // Remove attributes not allowed for detected input type

diff --git a/library/HTMLPurifier/HTML5Config.php b/library/HTMLPurifier/HTML5Config.php
@@ -2,7 +2,7 @@
 
 class HTMLPurifier_HTML5Config extends HTMLPurifier_Config
 {
-    const REVISION = 2022080601;
+    const REVISION = 2022080701;
 
     /**
      * @param  string|array|HTMLPurifier_Config $config

diff --git a/tests/HTMLPurifier/AttrDef/HTML5/InputTypeTest.php b/tests/HTMLPurifier/AttrDef/HTML5/InputTypeTest.php
@@ -20,10 +20,40 @@ public function testValidInputType()
     public function testInvalidInputType()
     {
         $this->assertValidate('foo', false);
+        $this->assertValidate('', false);
     }
 
     public function testUppercaseInputType()
     {
         $this->assertValidate('TEXT', 'text');
     }
+
+    public function testNullAllowedInputTypes()
+    {
+        $this->config->set('Attr.AllowedInputTypes', null);
+        $this->assertValidate('text');
+        $this->assertValidate('password');
+    }
+
+    public function testEmptyAllowedInputTypes()
+    {
+        $this->config->set('Attr.AllowedInputTypes', array());
+        $this->assertValidate('text', false);
+        $this->assertValidate('password', false);
+    }
+
+    public function testTextInputTypeNotAllowed()
+    {
+        $this->config->set('Attr.AllowedInputTypes', array('password'));
+
+        $this->assertValidate('text', false);
+        $this->assertValidate('password');
+    }
+
+    public function testInvalidAllowedInputTypes()
+    {
+        $this->config->set('Attr.AllowedInputTypes', array('foo'));
+        $this->assertValidate('foo', false);
+        $this->assertValidate('text', false);
+    }
 }
diff --git a/tests/HTMLPurifier/AttrTransform/HTML5/InputTest.php b/tests/HTMLPurifier/AttrTransform/HTML5/InputTest.php
@@ -96,44 +96,17 @@ public function testMissingValue()
         ));
     }
 
-    public function testTextInputTypeNotAllowed()
+    public function testImageAlt()
     {
-        $this->config->set('Attr.AllowedInputTypes', array('password'));
-
-        $this->assertResult(array('type' => 'text'), false);
-        $this->assertResult(array('type' => 'password'));
-    }
-
-    public function testNullAllowedInputTypes()
-    {
-        $this->config->set('Attr.AllowedInputTypes', null);
-
-        $this->assertResult(array('type' => 'text'));
-        $this->assertResult(array('type' => 'password'));
-    }
-
-    public function testEmptyAllowedInputTypes()
-    {
-        $this->config->set('Attr.AllowedInputTypes', array());
-
-        $this->assertResult(array('type' => 'text'), false);
-        $this->assertResult(array('type' => 'password'), false);
-    }
-
-    public function testInvalidAllowedInputTypes()
-    {
-        $this->config->set('Attr.AllowedInputTypes', array('foo'));
-
-        // 'foo' type is converted to 'text', and 'text' type is not allowed
-        $this->assertResult(array('type' => 'foo'), false);
-        $this->assertResult(array('type' => 'text'), false);
-        $this->assertResult(array('type' => 'password'), false);
-    }
-
-    public function testInvalidOrEmptyInputType()
-    {
-        $this->assertResult(array('type' => 'foo'), array());
-        $this->assertResult(array('type' => ''), array());
-        $this->assertResult(array(), array());
+        $this->assertResult(array(
+            'type' => 'image',
+            'alt' => 'foo',
+        ));
+        $this->assertResult(array(
+            'type' => 'image',
+        ), array(
+            'type' => 'image',
+            'alt' => 'image',
+        ));
     }
 }