diff --git a/core/htmlsanitizer.class.inc.php b/core/htmlsanitizer.class.inc.php index 55de50f62..18bb2574d 100644 --- a/core/htmlsanitizer.class.inc.php +++ b/core/htmlsanitizer.class.inc.php @@ -116,6 +116,18 @@ abstract class DOMSanitizer extends HTMLSanitizer { /** @var DOMDocument */ protected $oDoc; + /** + * @var string Class to use for InlineImage static method calls + * @used-by \Combodo\iTop\Test\UnitTest\Core\Sanitizer\HTMLDOMSanitizerTest::testDoSanitizeCallInlineImageProcessImageTag + */ + protected $sInlineImageClassName; + + public function __construct($sInlineImageClassName = InlineImage::class) + { + parent::__construct(); + + $this->sInlineImageClassName = $sInlineImageClassName; + } abstract public function GetTagsWhiteList(); @@ -211,7 +223,7 @@ abstract class DOMSanitizer extends HTMLSanitizer // Recurse $this->CleanNode($oNode); if (($oNode instanceof DOMElement) && (strtolower($oNode->tagName) == 'img')) { - InlineImage::ProcessImageTag($oNode); + $this->sInlineImageClassName::ProcessImageTag($oNode); } } } @@ -347,6 +359,30 @@ class HTMLDOMSanitizer extends DOMSanitizer 'white-space', ); + public function __construct($sInlineImageClassName = InlineImage::class) + { + parent::__construct($sInlineImageClassName); + + // Building href validation pattern from url and email validation patterns as the patterns are not used the same way in HTML content than in standard attributes value. + // eg. "foo@bar.com" vs "mailto:foo@bar.com?subject=Title&body=Hello%20world" + if (!array_key_exists('href', self::$aAttrsWhiteList)) { + // Regular urls + $sUrlPattern = utils::GetConfig()->Get('url_validation_pattern'); + + // Mailto urls + $sMailtoPattern = '(mailto:('.utils::GetConfig()->Get('email_validation_pattern').')(?:\?(?:subject|body)=([a-zA-Z0-9+\$_.-]*)(?:&(?:subject|body)=([a-zA-Z0-9+\$_.-]*))?)?)'; + + // Notification placeholders + // eg. $this->caller_id$, $this->hyperlink()$, $this->hyperlink(portal)$, $APP_URL$, $MODULES_URL$, ... + // Note: Authorize both $xxx$ and %24xxx%24 as the latter one is encoded when used in HTML attributes (eg. a[href]) + $sPlaceholderPattern = '(\$|%24)[\w-]*(->[\w]*(\([\w-]*?\))?)?(\$|%24)'; + + $sPattern = $sUrlPattern.'|'.$sMailtoPattern.'|'.$sPlaceholderPattern; + $sPattern = '/'.str_replace('/', '\/', $sPattern).'/i'; + self::$aAttrsWhiteList['href'] = $sPattern; + } + } + public function GetTagsWhiteList() { return static::$aTagsWhiteList; @@ -372,30 +408,6 @@ class HTMLDOMSanitizer extends DOMSanitizer return static::$aStylesWhiteList; } - public function __construct() - { - parent::__construct(); - - // Building href validation pattern from url and email validation patterns as the patterns are not used the same way in HTML content than in standard attributes value. - // eg. "foo@bar.com" vs "mailto:foo@bar.com?subject=Title&body=Hello%20world" - if (!array_key_exists('href', self::$aAttrsWhiteList)) { - // Regular urls - $sUrlPattern = utils::GetConfig()->Get('url_validation_pattern'); - - // Mailto urls - $sMailtoPattern = '(mailto:('.utils::GetConfig()->Get('email_validation_pattern').')(?:\?(?:subject|body)=([a-zA-Z0-9+\$_.-]*)(?:&(?:subject|body)=([a-zA-Z0-9+\$_.-]*))?)?)'; - - // Notification placeholders - // eg. $this->caller_id$, $this->hyperlink()$, $this->hyperlink(portal)$, $APP_URL$, $MODULES_URL$, ... - // Note: Authorize both $xxx$ and %24xxx%24 as the latter one is encoded when used in HTML attributes (eg. a[href]) - $sPlaceholderPattern = '(\$|%24)[\w-]*(->[\w]*(\([\w-]*?\))?)?(\$|%24)'; - - $sPattern = $sUrlPattern.'|'.$sMailtoPattern.'|'.$sPlaceholderPattern; - $sPattern = '/'.str_replace('/', '\/', $sPattern).'/i'; - self::$aAttrsWhiteList['href'] = $sPattern; - } - } - public function LoadDoc($sHTML) { @$this->oDoc->loadHTML(''.$sHTML); // For loading HTML chunks where the character set is not specified diff --git a/test/application/UtilsTest.php b/test/application/UtilsTest.php index 1c800d57f..e09b7d25c 100644 --- a/test/application/UtilsTest.php +++ b/test/application/UtilsTest.php @@ -19,10 +19,12 @@ * */ +use Combodo\iTop\Test\UnitTest\ItopTestCase; + /** * @covers utils */ -class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopDataTestCase +class UtilsTest extends ItopTestCase { public function testEndsWith() { @@ -466,20 +468,27 @@ class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopDataTestCase public function sanitizerDataProvider() { return [ - 'good integer' => ['integer', '2565', '2565'], - 'bad integer' => ['integer', 'a2656', '2656'], - 'good class' => ['class', 'UserRequest', 'UserRequest'], - 'bad class' => ['class', 'MyUserRequest',null], - 'good string' => ['string', 'Is Peter smart and funny?', 'Is Peter smart and funny?'], - 'bad string' => ['string', 'Is Peter & funny?', 'Is Peter <smart> & funny?'], - 'good transaction_id' => ['transaction_id', '8965.-dd', '8965.-dd'], - 'bad transaction_id' => ['transaction_id', '8965.-dd+', null], - 'good parameter' => ['parameter', 'JU8965-dd=_', 'JU8965-dd=_'], - 'bad parameter' => ['parameter', '8965.-dd+', null], - 'good field_name' => ['field_name', 'Name->bUzz38', 'Name->bUzz38'], - 'bad field_name' => ['field_name', 'name-buzz', null], - 'good context_param' => ['context_param', '%dssD25_=%:+-', '%dssD25_=%:+-'], - 'bad context_param' => ['context_param', '%dssD,25_=%:+-', null], + 'good integer' => ['integer', '2565', '2565'], + 'bad integer' => ['integer', 'a2656', '2656'], + /** + * 'class' filter needs a loaded datamodel... and is only an indirection to \MetaModel::IsValidClass so might very important to test ! + * If we switch this class to ItopDataTestCase then we are seeing : + * - the class now takes 18s to process instead of... 459ms when using ItopTestCase !!! + * - multiple errors are thrown in testGetAbsoluteUrlAppRootPersistency :( + * We decided it wasn't worse the effort to test the 'class' filter ! + */ + // 'good class' => ['class', 'UserRequest', 'UserRequest'], + // 'bad class' => ['class', 'MyUserRequest',null], + 'good string' => ['string', 'Is Peter smart and funny?', 'Is Peter smart and funny?'], + 'bad string' => ['string', 'Is Peter & funny?', 'Is Peter <smart> & funny?'], + 'good transaction_id' => ['transaction_id', '8965.-dd', '8965.-dd'], + 'bad transaction_id' => ['transaction_id', '8965.-dd+', null], + 'good parameter' => ['parameter', 'JU8965-dd=_', 'JU8965-dd=_'], + 'bad parameter' => ['parameter', '8965.-dd+', null], + 'good field_name' => ['field_name', 'Name->bUzz38', 'Name->bUzz38'], + 'bad field_name' => ['field_name', 'name-buzz', null], + 'good context_param' => ['context_param', '%dssD25_=%:+-', '%dssD25_=%:+-'], + 'bad context_param' => ['context_param', '%dssD,25_=%:+-', null], 'good element_identifier' => ['element_identifier', 'AD05nb', 'AD05nb'], 'bad element_identifier' => ['element_identifier', 'AD05nb+', 'AD05nb'], 'good url' => ['url', 'https://www.w3schools.com', 'https://www.w3schools.com'], diff --git a/test/core/sanitizer/AbstractDOMSanitizerTest.php b/test/core/sanitizer/AbstractDOMSanitizerTest.php index 082f54e01..a9fb5d945 100644 --- a/test/core/sanitizer/AbstractDOMSanitizerTest.php +++ b/test/core/sanitizer/AbstractDOMSanitizerTest.php @@ -14,7 +14,6 @@ abstract class AbstractDOMSanitizerTest extends ItopTestCase parent::setUp(); require_once(APPROOT.'application/utils.inc.php'); require_once(APPROOT.'core/htmlsanitizer.class.inc.php'); - require_once(APPROOT.'test/core/sanitizer/InlineImageMock.php'); } protected function ReadTestFile($sFileToTest, $sFolderName) diff --git a/test/core/sanitizer/HTMLDOMSanitizerTest.php b/test/core/sanitizer/HTMLDOMSanitizerTest.php index bec05e87d..28df546ab 100644 --- a/test/core/sanitizer/HTMLDOMSanitizerTest.php +++ b/test/core/sanitizer/HTMLDOMSanitizerTest.php @@ -2,16 +2,12 @@ namespace Combodo\iTop\Test\UnitTest\Core\Sanitizer; use HTMLDOMSanitizer; +use InlineImageMock; require_once __DIR__.'/AbstractDOMSanitizerTest.php'; -/** - * @runTestsInSeparateProcesses - * @preserveGlobalState disabled - * @backupGlobals disabled - */ class HTMLDOMSanitizerTest extends AbstractDOMSanitizerTest { /** @@ -222,15 +218,17 @@ class HTMLDOMSanitizerTest extends AbstractDOMSanitizerTest /** * @dataProvider CallInlineImageProcessImageTagProvider + * @uses \InlineImageMock */ public function testDoSanitizeCallInlineImageProcessImageTag($sHtml, $iExpectedCount) { require_once APPROOT.'test/core/sanitizer/InlineImageMock.php'; + InlineImageMock::ResetCallCounter(); - $oSanitizer = new HTMLDOMSanitizer(); + $oSanitizer = new HTMLDOMSanitizer(InlineImageMock::class); $oSanitizer->DoSanitize($sHtml); - $iCalledCount = \InlineImage::GetCallCounter(); + $iCalledCount = \InlineImageMock::GetCallCounter(); $this->assertEquals($iExpectedCount, $iCalledCount); } diff --git a/test/core/sanitizer/InlineImageMock.php b/test/core/sanitizer/InlineImageMock.php index 9ebcda852..289470648 100644 --- a/test/core/sanitizer/InlineImageMock.php +++ b/test/core/sanitizer/InlineImageMock.php @@ -1,4 +1,6 @@