From f5371bbad456a46f9042ce248e4f74f11b5301d5 Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Tue, 2 Oct 2007 01:19:46 +0000 Subject: [PATCH] [2.1.3] - Buggy treatment of end tags of elements that have required attributes fixed (does not manifest on default tag-set) - Spurious internal content reorganization error suppressed . Error unit tests can now specify the expectation of no errors. Future iterations of the harness will be extremely strict about what errors are allowed git-svn-id: http://htmlpurifier.org/svnroot/htmlpurifier/trunk@1424 48356398-32a2-884e-a903-53898d9a118a --- NEWS | 6 ++++++ library/HTMLPurifier/ChildDef/Optional.php | 5 ++++- library/HTMLPurifier/Strategy/FixNesting.php | 2 +- .../HTMLPurifier/Strategy/RemoveForeignElements.php | 2 +- tests/HTMLPurifier/ChildDef/OptionalTest.php | 4 ++++ tests/HTMLPurifier/ErrorsHarness.php | 11 ++++++++++- tests/HTMLPurifier/Strategy/FixNesting_ErrorsTest.php | 5 +++++ .../Strategy/RemoveForeignElementsTest.php | 9 +++++++++ 8 files changed, 40 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 2dce9d37..b8fc3d79 100644 --- a/NEWS +++ b/NEWS @@ -22,8 +22,14 @@ NEWS ( CHANGELOG and HISTORY ) HTMLPurifier - Further refine AutoParagraph injector. Behavior inside of elements allowing paragraph tags clarified: only inline content delimeted by double newlines (not block elements) are paragraphed. +- Buggy treatment of end tags of elements that have required attributes + fixed (does not manifest on default tag-set) +- Spurious internal content reorganization error suppressed . %Core.AcceptFullDocuments renamed to %Core.ConvertDocumentToFragment to better communicate its purpose +. Error unit tests can now specify the expectation of no errors. Future + iterations of the harness will be extremely strict about what errors + are allowed 2.1.2, released 2007-09-03 ! Implemented Object module for trusted users diff --git a/library/HTMLPurifier/ChildDef/Optional.php b/library/HTMLPurifier/ChildDef/Optional.php index 779a7f06..e9f14edf 100644 --- a/library/HTMLPurifier/ChildDef/Optional.php +++ b/library/HTMLPurifier/ChildDef/Optional.php @@ -15,7 +15,10 @@ class HTMLPurifier_ChildDef_Optional extends HTMLPurifier_ChildDef_Required var $type = 'optional'; function validateChildren($tokens_of_children, $config, &$context) { $result = parent::validateChildren($tokens_of_children, $config, $context); - if ($result === false) return array(); + if ($result === false) { + if (empty($tokens_of_children)) return true; + else return array(); + } return $result; } } diff --git a/library/HTMLPurifier/Strategy/FixNesting.php b/library/HTMLPurifier/Strategy/FixNesting.php index 51a14a78..25e9f8ac 100644 --- a/library/HTMLPurifier/Strategy/FixNesting.php +++ b/library/HTMLPurifier/Strategy/FixNesting.php @@ -195,7 +195,7 @@ class HTMLPurifier_Strategy_FixNesting extends HTMLPurifier_Strategy //################################################################// // Process result by interpreting $result - if ($result === true) { + if ($result === true || $child_tokens === $result) { // leave the node as is // register start token as a parental node start diff --git a/library/HTMLPurifier/Strategy/RemoveForeignElements.php b/library/HTMLPurifier/Strategy/RemoveForeignElements.php index 2c280b23..5d26e4f5 100644 --- a/library/HTMLPurifier/Strategy/RemoveForeignElements.php +++ b/library/HTMLPurifier/Strategy/RemoveForeignElements.php @@ -116,6 +116,7 @@ class HTMLPurifier_Strategy_RemoveForeignElements extends HTMLPurifier_Strategy // mostly everything's good, but // we need to make sure required attributes are in order if ( + ($token->type === 'start' || $token->type === 'empty') && $definition->info[$token->name]->required_attr && ($token->name != 'img' || $remove_invalid_img) // ensure config option still works ) { @@ -134,7 +135,6 @@ class HTMLPurifier_Strategy_RemoveForeignElements extends HTMLPurifier_Strategy $token->armor['ValidateAttributes'] = true; } - // CAN BE GENERICIZED if (isset($hidden_elements[$token->name]) && $token->type == 'start') { $textify_comments = $token->name; } elseif ($token->name === $textify_comments && $token->type == 'end') { diff --git a/tests/HTMLPurifier/ChildDef/OptionalTest.php b/tests/HTMLPurifier/ChildDef/OptionalTest.php index 154353df..40dc17ee 100644 --- a/tests/HTMLPurifier/ChildDef/OptionalTest.php +++ b/tests/HTMLPurifier/ChildDef/OptionalTest.php @@ -19,5 +19,9 @@ class HTMLPurifier_ChildDef_OptionalTest extends HTMLPurifier_ChildDefHarness $this->assertResult('Not allowed text', ''); } + function testEmpty() { + $this->assertResult(''); + } + } diff --git a/tests/HTMLPurifier/ErrorsHarness.php b/tests/HTMLPurifier/ErrorsHarness.php index 67f7c6b3..9ab204e7 100644 --- a/tests/HTMLPurifier/ErrorsHarness.php +++ b/tests/HTMLPurifier/ErrorsHarness.php @@ -3,11 +3,15 @@ require_once 'HTMLPurifier/ErrorCollectorEMock.php'; require_once 'HTMLPurifier/Lexer/DirectLex.php'; +/** + * @todo Make the callCount variable actually work, so we can precisely + * specify what errors we want: no more, no less + */ class HTMLPurifier_ErrorsHarness extends HTMLPurifier_Harness { var $config, $context; - var $collector, $generator; + var $collector, $generator, $callCount; function setup() { $this->config = HTMLPurifier_Config::create(array('Core.CollectErrors' => true)); @@ -16,6 +20,11 @@ class HTMLPurifier_ErrorsHarness extends HTMLPurifier_Harness $this->collector = new HTMLPurifier_ErrorCollectorEMock(); $this->collector->prepare($this->context); $this->context->register('ErrorCollector', $this->collector); + $this->callCount = 0; + } + + function expectNoErrorCollection() { + $this->collector->expectNever('send'); } function expectErrorCollection() { diff --git a/tests/HTMLPurifier/Strategy/FixNesting_ErrorsTest.php b/tests/HTMLPurifier/Strategy/FixNesting_ErrorsTest.php index 4ba30f36..db5b989f 100644 --- a/tests/HTMLPurifier/Strategy/FixNesting_ErrorsTest.php +++ b/tests/HTMLPurifier/Strategy/FixNesting_ErrorsTest.php @@ -28,6 +28,11 @@ class HTMLPurifier_Strategy_FixNesting_ErrorsTest extends HTMLPurifier_Strategy_ $this->invoke("Valid
Invalid
"); } + function testNoNodeReorganizedForEmptyNode() { + $this->expectNoErrorCollection(); + $this->invoke(""); + } + function testNodeContentsRemoved() { $this->expectErrorCollection(E_ERROR, 'Strategy_FixNesting: Node contents removed'); $this->expectContext('CurrentToken', new HTMLPurifier_Token_Start('span', array(), 1)); diff --git a/tests/HTMLPurifier/Strategy/RemoveForeignElementsTest.php b/tests/HTMLPurifier/Strategy/RemoveForeignElementsTest.php index 19a37b24..4b397b09 100644 --- a/tests/HTMLPurifier/Strategy/RemoveForeignElementsTest.php +++ b/tests/HTMLPurifier/Strategy/RemoveForeignElementsTest.php @@ -82,5 +82,14 @@ alert(<b>bold</b>); ); } + function testRequiredAttributesTestNotPerformedOnEndTag() { + $this->config->set('HTML', 'DefinitionID', + 'HTMLPurifier_Strategy_RemoveForeignElementsTest'. + '->testRequiredAttributesTestNotPerformedOnEndTag'); + $def =& $this->config->getHTMLDefinition(true); + $def->addElement('f', 'Block', 'Optional: #PCDATA', false, array('req*' => 'Text')); + $this->assertResult('Foo Bar'); + } + }