Is there a reason you double up on htmlPurifier in parser?

[Flitterkill]

SNIP

/* Clean HTML */
		if ( $value and $this->htmlPurifier )
		{
			$value = $this->htmlPurifier->purify( $value );
		}
		
/* BBCode, Profanity, etc. */
		if ( $value )
		{
			$value = $this->_parseContent( $value );
		}
						
/* Clean HTML */
		if ( $value and $this->htmlPurifier )
		{
			$value = $this->htmlPurifier->purify( $value );
		}
 
SNIP

Am I missing something or is this just a mistake? Seems inefficient... 4.1.13.2

[teraßyte]

Indeed, just taking a quick look at the code you posted this below would be more efficient:

SNIP
		
        /* BBCode, Profanity, etc. */
        if ( $value )
        {
                $value = $this->_parseContent( $value );

                /* Clean HTML */
                if ( $value and $this->htmlPurifier )
                {
                	$value = $this->htmlPurifier->purify( $value );
                }
        }
 
SNIP

 

But I have not looked at the whole class/method the code is from so there might be a reason. Or it's just a copy/paste mistake, they moved it after bbcode parsing and they forgot to delete the one before.

[Charles]

It's done a second time because BBCode is archaic and can introduce all sorts of possible issues.

[CodingJungle]

On 7/26/2016 at 0:17 PM, Charles said:

It's done a second time because BBCode is archaic and can introduce all sorts of possible issues.

wouldn't it make more sense (and be more efficient) to parse the BBCode first, then send it to the htmlpurifier?

[bfarber]

It's because we use DomDocument, so we have to have well formed HTML to start with. HTMLPurifier does this, so we parse once which removes "bad" stuff and ensures everything is well formed, then we loop through nodes using DomDocument to do whatever bbcode-style processing is needed, and then we run HTML Purifier one more time to be safe (since bbcode replacements could introduce something bad).