Fix problem where stacked AttrTransforms clobber each other.

Signed-off-by: Edward Z. Yang <ezyang@mit.edu>

NEWS

@@ -10,6 +10,10 @@ NEWS ( CHANGELOG and HISTORY )                                     HTMLPurifier
 ==========================
 
 4.5.0, unknown release date
+# Fix bug where stacked attribute transforms clobber each other;
+  this also means it's no longer possible to override attribute
+  transforms in later modules.  No internal code was using this
+  but this may break some clients.
 
 4.4.0, released 2012-01-18
 # Removed PEARSax3 handler.

configdoc/usage.xml

@@ -254,7 +254,7 @@
  </directive>
  <directive id="URI.">
   <file name="HTMLPurifier/URIDefinition.php">
-   <line>59</line>
+   <line>60</line>
   </file>
   <file name="HTMLPurifier/URIFilter/Munge.php">
    <line>12</line>
@@ -262,7 +262,7 @@
  </directive>
  <directive id="URI.Host">
   <file name="HTMLPurifier/URIDefinition.php">
-   <line>69</line>
+   <line>70</line>
   </file>
   <file name="HTMLPurifier/URIScheme.php">
    <line>81</line>
@@ -270,12 +270,12 @@
  </directive>
  <directive id="URI.Base">
   <file name="HTMLPurifier/URIDefinition.php">
-   <line>70</line>
+   <line>71</line>
   </file>
  </directive>
  <directive id="URI.DefaultScheme">
   <file name="HTMLPurifier/URIDefinition.php">
-   <line>77</line>
+   <line>78</line>
   </file>
  </directive>
  <directive id="URI.AllowedSchemes">

library/HTMLPurifier/ElementDef.php

@@ -30,13 +30,25 @@ class HTMLPurifier_ElementDef
      */
     public $attr = array();
 
+    // XXX: Design note: currently, it's not possible to override
+    // previously defined AttrTransforms without messing around with
+    // the final generated config. This is by design; a previous version
+    // used an associated list of attr_transform, but it was extremely
+    // easy to accidentally override other attribute transforms by
+    // forgetting to specify an index (and just using 0.)  While we
+    // could check this by checking the index number and complaining,
+    // there is a second problem which is that it is not at all easy to
+    // tell when something is getting overridden. Combine this with a
+    // codebase where this isn't really being used, and it's perfect for
+    // nuking.
+
     /**
-     * Indexed list of tag's HTMLPurifier_AttrTransform to be done before validation
+     * List of tags HTMLPurifier_AttrTransform to be done before validation
      */
     public $attr_transform_pre = array();
 
     /**
-     * Indexed list of tag's HTMLPurifier_AttrTransform to be done after validation
+     * List of tags HTMLPurifier_AttrTransform to be done after validation
      */
     public $attr_transform_post = array();
 
@@ -144,9 +156,9 @@ class HTMLPurifier_ElementDef
             }
             $this->attr[$k] = $v;
         }
-        $this->_mergeAssocArray($this->attr_transform_pre, $def->attr_transform_pre);
-        $this->_mergeAssocArray($this->attr_transform_post, $def->attr_transform_post);
         $this->_mergeAssocArray($this->excludes, $def->excludes);
+        $this->attr_transform_pre = array_merge($this->attr_transform_pre, $def->attr_transform_pre);
+        $this->attr_transform_post = array_merge($this->attr_transform_post, $def->attr_transform_post);
 
         if(!empty($def->content_model)) {
             $this->content_model =

library/HTMLPurifier/HTMLModule/Bdo.php

@@ -21,7 +21,7 @@ class HTMLPurifier_HTMLModule_Bdo extends HTMLPurifier_HTMLModule
                 // inclusions wrong for bdo: bdo allows Lang
             )
         );
-        $bdo->attr_transform_post['required-dir'] = new HTMLPurifier_AttrTransform_BdoDir();
+        $bdo->attr_transform_post[] = new HTMLPurifier_AttrTransform_BdoDir();
 
         $this->attr_collections['I18N']['dir'] = 'Enum#ltr,rtl';
     }

library/HTMLPurifier/HTMLModule/Name.php

@@ -11,7 +11,7 @@ class HTMLPurifier_HTMLModule_Name extends HTMLPurifier_HTMLModule
             $element = $this->addBlankElement($name);
             $element->attr['name'] = 'CDATA';
             if (!$config->get('HTML.Attr.Name.UseCDATA')) {
-                $element->attr_transform_post['NameSync'] = new HTMLPurifier_AttrTransform_NameSync();
+                $element->attr_transform_post[] = new HTMLPurifier_AttrTransform_NameSync();
             }
         }
     }

library/HTMLPurifier/HTMLModule/Scripting.php

@@ -45,8 +45,8 @@ class HTMLPurifier_HTMLModule_Scripting extends HTMLPurifier_HTMLModule
         );
         $this->info['script']->content_model = '#PCDATA';
         $this->info['script']->content_model_type = 'optional';
-        $this->info['script']->attr_transform_pre['type'] =
+        $this->info['script']->attr_transform_pre[] =
-        $this->info['script']->attr_transform_post['type'] =
+        $this->info['script']->attr_transform_post[] =
             new HTMLPurifier_AttrTransform_ScriptRequired();
     }
 }

tests/HTMLPurifier/ElementDefTest.php

@@ -22,12 +22,16 @@ class HTMLPurifier_ElementDefTest extends HTMLPurifier_Harness
             'overloaded-attr' => $overloaded_old,
             'removed-attr' => $removed,
         );
+        /*
         $def1->attr_transform_pre =
         $def1->attr_transform_post = array(
             'old-transform' => $old,
             'overloaded-transform' => $overloaded_old,
             'removed-transform' => $removed,
         );
+         */
+        $def1->attr_transform_pre[] = $old;
+        $def1->attr_transform_post[] = $old;
         $def1->child = $overloaded_old;
         $def1->content_model = 'old';
         $def1->content_model_type = $overloaded_old;
@@ -44,12 +48,16 @@ class HTMLPurifier_ElementDefTest extends HTMLPurifier_Harness
             'overloaded-attr' => $overloaded_new,
             'removed-attr' => false,
         );
+        /*
         $def2->attr_transform_pre =
         $def2->attr_transform_post = array(
             'new-transform' => $new,
             'overloaded-transform' => $overloaded_new,
             'removed-transform' => false,
         );
+         */
+        $def2->attr_transform_pre[] = $new;
+        $def2->attr_transform_post[] = $new;
         $def2->child = $new;
         $def2->content_model = '#SUPER | new';
         $def2->content_model_type = $overloaded_new;
@@ -70,11 +78,14 @@ class HTMLPurifier_ElementDefTest extends HTMLPurifier_Harness
             'new-attr' => $new,
         ));
         $this->assertIdentical($def1->attr_transform_pre, $def1->attr_transform_post);
+        $this->assertIdentical($def1->attr_transform_pre, array($old, $new));
+        /*
         $this->assertIdentical($def1->attr_transform_pre, array(
             'old-transform' => $old,
             'overloaded-transform' => $overloaded_new,
             'new-transform' => $new,
         ));
+         */
         $this->assertIdentical($def1->child, $new);
         $this->assertIdentical($def1->content_model, 'old | new');
         $this->assertIdentical($def1->content_model_type, $overloaded_new);