[3.1.2] Implement comments when %HTML.Trusted is on.

Some implementation notes: not all comments are valid; HTML makes sure
double-hyphens and trailing hyphens are not found in comments. In addition,
two new localizable messages were added.

Requested-by: Waldo Jaquith <waldo@vqronline.org>
Signed-off-by: Edward Z. Yang <edwardzyang@thewritingpot.com>

NEWS

@@ -14,6 +14,7 @@ NEWS ( CHANGELOG and HISTORY )                                     HTMLPurifier
 3.1.2, unknown release date
 ! %Output.AttrSort for when you need your attributes in alphabetical order to
   deal with a bug in FCKEditor. Requested by frank farmer.
+! Enable HTML comments when %HTML.Trusted is on. Requested by Waldo Jaquith.
 
 3.1.1, released 2008-06-19
 # %URI.Munge now, by default, does not munge resources (for example, <img src="">)

library/HTMLPurifier/Language/messages/en.php

@@ -30,6 +30,8 @@ $messages = array(
 'Strategy_RemoveForeignElements: Comment removed'            => 'Comment containing "$CurrentToken.Data" removed',
 'Strategy_RemoveForeignElements: Foreign meta element removed' => 'Unrecognized $CurrentToken.Serialized meta tag and all descendants removed',
 'Strategy_RemoveForeignElements: Token removed to end'       => 'Tags and text starting from $1 element where removed to end',
+'Strategy_RemoveForeignElements: Trailing hyphen in comment removed' => 'Trailing hyphen(s) in comment removed',
+'Strategy_RemoveForeignElements: Hyphens in comment collapsed' => 'Double hyphens in comments are not allowed, and were collapsed into single hyphens',
 
 'Strategy_MakeWellFormed: Unnecessary end tag removed' => 'Unnecessary $CurrentToken.Serialized tag removed',
 'Strategy_MakeWellFormed: Unnecessary end tag to text' => 'Unnecessary $CurrentToken.Serialized tag converted to text',

library/HTMLPurifier/Strategy/RemoveForeignElements.php

@@ -19,6 +19,9 @@ class HTMLPurifier_Strategy_RemoveForeignElements extends HTMLPurifier_Strategy
         $escape_invalid_tags = $config->get('Core', 'EscapeInvalidTags');
         $remove_invalid_img  = $config->get('Core', 'RemoveInvalidImg');
         
+        // currently only used to determine if comments should be kept
+        $trusted = $config->get('HTML', 'Trusted');
+        
         $remove_script_contents = $config->get('Core', 'RemoveScriptContents');
         $hidden_elements     = $config->get('Core', 'HiddenElements');
         
@@ -125,6 +128,23 @@ class HTMLPurifier_Strategy_RemoveForeignElements extends HTMLPurifier_Strategy
                 if ($textify_comments !== false) {
                     $data = $token->data;
                     $token = new HTMLPurifier_Token_Text($data);
+                } elseif ($trusted) {
+                    // keep, but perform comment cleaning
+                    if ($e) {
+                        // perform check whether or not there's a trailing hyphen
+                        if (substr($token->data, -1) == '-') {
+                            $e->send(E_NOTICE, 'Strategy_RemoveForeignElements: Trailing hyphen in comment removed');
+                        }
+                    }
+                    $token->data = rtrim($token->data, '-');
+                    $found_double_hyphen = false;
+                    while (strpos($token->data, '--') !== false) {
+                        if ($e && !$found_double_hyphen) {
+                            $e->send(E_NOTICE, 'Strategy_RemoveForeignElements: Hyphens in comment collapsed');
+                        }
+                        $found_double_hyphen = true; // prevent double-erroring
+                        $token->data = str_replace('--', '-', $token->data);
+                    }
                 } else {
                     // strip comments
                     if ($e) $e->send(E_NOTICE, 'Strategy_RemoveForeignElements: Comment removed');

tests/HTMLPurifier/Strategy/RemoveForeignElementsTest.php

@@ -88,5 +88,20 @@ alert(<b>bold</b>);
         $this->assertResult('<f req="text">Foo</f> Bar');
     }
     
+    function testPreserveCommentsWithHTMLTrusted() {
+        $this->config->set('HTML', 'Trusted', true);
+        $this->assertResult('<!-- foo -->');
+    }
+    
+    function testRemoveTrailingHyphensInComment() {
+        $this->config->set('HTML', 'Trusted', true);
+        $this->assertResult('<!-- foo ----->', '<!-- foo -->');
+    }
+    
+    function testCollapseDoubleHyphensInComment() {
+        $this->config->set('HTML', 'Trusted', true);
+        $this->assertResult('<!-- bo --- asdf--as -->', '<!-- bo - asdf-as -->');
+    }
+    
 }
 

tests/HTMLPurifier/Strategy/RemoveForeignElements_ErrorsTest.php

@@ -45,6 +45,20 @@ class HTMLPurifier_Strategy_RemoveForeignElements_ErrorsTest extends HTMLPurifie
         $this->invoke('<!-- test -->');
     }
     
+    function testTrailingHyphenInCommentRemoved() {
+        $this->config->set('HTML', 'Trusted', true);
+        $this->expectErrorCollection(E_NOTICE, 'Strategy_RemoveForeignElements: Trailing hyphen in comment removed');
+        $this->expectContext('CurrentToken', new HTMLPurifier_Token_Comment(' test --', 1));
+        $this->invoke('<!-- test ---->');
+    }
+    
+    function testDoubleHyphenInCommentRemoved() {
+        $this->config->set('HTML', 'Trusted', true);
+        $this->expectErrorCollection(E_NOTICE, 'Strategy_RemoveForeignElements: Hyphens in comment collapsed');
+        $this->expectContext('CurrentToken', new HTMLPurifier_Token_Comment(' test --- test -- test ', 1));
+        $this->invoke('<!-- test --- test -- test -->');
+    }
+    
     function testForeignMetaElementRemoved() {
         $this->collector->expectAt(0, 'send', array(E_ERROR, 'Strategy_RemoveForeignElements: Foreign meta element removed'));
         $this->collector->expectContextAt(0, 'CurrentToken', new HTMLPurifier_Token_Start('script', array(), 1));