From 7291f19347b05f5833421eaf5152558bfbd2b454 Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Fri, 16 Mar 2012 23:12:16 -0400 Subject: [PATCH] Fix problem where stacked AttrTransforms clobber each other. Signed-off-by: Edward Z. Yang --- NEWS | 4 ++++ configdoc/usage.xml | 8 ++++---- library/HTMLPurifier/ElementDef.php | 20 ++++++++++++++++---- library/HTMLPurifier/HTMLModule/Bdo.php | 2 +- library/HTMLPurifier/HTMLModule/Name.php | 2 +- library/HTMLPurifier/HTMLModule/Scripting.php | 4 ++-- tests/HTMLPurifier/ElementDefTest.php | 11 +++++++++++ 7 files changed, 39 insertions(+), 12 deletions(-) diff --git a/NEWS b/NEWS index 5d3e17d0..058c14d1 100644 --- a/NEWS +++ b/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. diff --git a/configdoc/usage.xml b/configdoc/usage.xml index 1c77b8f4..dc79d45f 100644 --- a/configdoc/usage.xml +++ b/configdoc/usage.xml @@ -254,7 +254,7 @@ - 59 + 60 12 @@ -262,7 +262,7 @@ - 69 + 70 81 @@ -270,12 +270,12 @@ - 70 + 71 - 77 + 78 diff --git a/library/HTMLPurifier/ElementDef.php b/library/HTMLPurifier/ElementDef.php index 5498d956..10f7ab7f 100644 --- a/library/HTMLPurifier/ElementDef.php +++ b/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 = diff --git a/library/HTMLPurifier/HTMLModule/Bdo.php b/library/HTMLPurifier/HTMLModule/Bdo.php index 3d66f1b4..23ac3da3 100644 --- a/library/HTMLPurifier/HTMLModule/Bdo.php +++ b/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'; } diff --git a/library/HTMLPurifier/HTMLModule/Name.php b/library/HTMLPurifier/HTMLModule/Name.php index 05694b45..3a1271a9 100644 --- a/library/HTMLPurifier/HTMLModule/Name.php +++ b/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(); } } } diff --git a/library/HTMLPurifier/HTMLModule/Scripting.php b/library/HTMLPurifier/HTMLModule/Scripting.php index cecdea6c..2ac0d802 100644 --- a/library/HTMLPurifier/HTMLModule/Scripting.php +++ b/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_post['type'] = + $this->info['script']->attr_transform_pre[] = + $this->info['script']->attr_transform_post[] = new HTMLPurifier_AttrTransform_ScriptRequired(); } } diff --git a/tests/HTMLPurifier/ElementDefTest.php b/tests/HTMLPurifier/ElementDefTest.php index 500312b3..8f93ea3a 100644 --- a/tests/HTMLPurifier/ElementDefTest.php +++ b/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); -- 2.11.4.GIT