composer/iTop
6
0
Fork 0
mirror of https://github.com/Combodo/iTop.git synced 2026-02-12 23:14:18 +01:00
Code Issues Packages Projects Releases Wiki Activity

N°6405 - Fix XML displayed corrupted when using \DesignerLongTextField due to usage of \utils::EscapeHtml() without double encoding

Browse Source
This commit is contained in:
Molkobain
2023-06-21 11:32:52 +02:00
parent 2074a0fa0d
commit 216a1b95b1
5 changed files with 133 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
application/dashlet.class.inc.php
View File

@@ -667,7 +667,7 @@ class DashletUnknown extends Dashlet
*/
public function GetPropertiesFields(DesignerForm $oForm)
{
$oField = new DesignerLongTextField('xml', Dict::S('UI:DashletUnknown:Prop-XMLConfiguration'), $this->sOriginalDashletXML);
$oField = new DesignerXMLField('xml', Dict::S('UI:DashletUnknown:Prop-XMLConfiguration'), $this->sOriginalDashletXML);
$oForm->AddField($oField);
}

32
application/forms.class.inc.php
View File

@@ -1110,13 +1110,41 @@ $('#$sId').on('change keyup validate', function() { ValidateWithPattern('$sId',
}
EOF
);
$sValue = "<textarea $sCSSClasses id=\"$sId\" name=\"$sName\">".utils::EscapeHtml($this->defaultValue)."</textarea>";
$sValue = "<textarea $sCSSClasses id=\"$sId\" name=\"$sName\">".$this->PrepareValueForRendering()."</textarea>";
}
else {
$sValue = "<div $sCSSClasses id=\"$sId\">".utils::EscapeHtml($this->defaultValue)."</div>";
$sValue = "<div $sCSSClasses id=\"$sId\">".$this->PrepareValueForRendering()."</div>";
}
return array('label' => $this->sLabel, 'value' => $sValue);
}
/**
* @return string|null The value itself as expected for rendering. May it be encoded, escaped or else.
* @since 3.1.0 N°6405
*/
protected function PrepareValueForRendering(): ?string
{
return utils::EscapeHtml($this->defaultValue);
}
}
/**
* Class DesignerXMLField
*
* Field to display XML content
*
* @author Guillaume Lajarige <guillaume.lajarige@combodo.com>
* @since 3.1.0 N°6405
*/
class DesignerXMLField extends DesignerLongTextField
{
/**
* @inheritDoc
*/
protected function PrepareValueForRendering(): ?string
{
return utils::EscapeHtml($this->defaultValue, true);
}
}
class DesignerIntegerField extends DesignerFormField

6
application/utils.inc.php
View File

@@ -1970,6 +1970,7 @@ SQL;
/**
* @param string $sValue
* @param bool $bDoubleEncode Whether to double encode the value or not
*
* @return string passed value with only characters having a special meaning in HTML escaped as entities
* Since 3.0.0 we were using for this {@link HtmlEntities} but it was overkill and leads to double escaping !
@@ -1977,14 +1978,15 @@ SQL;
* @uses \htmlspecialchars()
* @link https://www.php.net/manual/fr/function.htmlspecialchars.php
* @since 3.0.0 N°3623
* @since 3.1.0 N°6405 Add $bDoubleEncode parameter
*/
public static function EscapeHtml($sValue)
public static function EscapeHtml($sValue, bool $bDoubleEncode = false)
{
return htmlspecialchars(
$sValue ?? '',
ENT_QUOTES | ENT_DISALLOWED | ENT_HTML5,
WebPage::PAGES_CHARSET,
false
$bDoubleEncode
);
}

82
tests/php-unit-tests/unitary-tests/application/DesignerFormFieldTest.php Normal file
View File

@@ -0,0 +1,82 @@
<?php
/**
* Copyright (C) 2018 Dennis Lassiter
*
* This file is part of iTop.
*
* iTop is free software; you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* iTop is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with iTop. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace Combodo\iTop\Test\UnitTest\Application;
use Combodo\iTop\Test\UnitTest\ItopTestCase;
use DesignerXMLField;
use utils;
/**
* @covers DesignerFormField
*/
class DesignerFormFieldTest extends ItopTestCase
{
/**
* @param string $sFieldFQCN
* @param string $sInputValue
* @param string $sExpectedValue
*
* @return void
* @throws \ReflectionException
* @covers DesignerLongTextField::PrepareValueForRendering
* @dataProvider PrepareValueForRenderingProvider
*/
public function testPrepareValueForRendering(string $sFieldFQCN, string $sInputValue, string $sExpectedValue)
{
$oField = new $sFieldFQCN('field_code', 'Field label', $sInputValue);
$sTestedValue = $this->InvokeNonPublicMethod($sFieldFQCN, 'PrepareValueForRendering', $oField, []);
$this->assertEquals($sExpectedValue, $sTestedValue);
}
public function PrepareValueForRenderingProvider(): array
{
return [
'DesignerLongTextField should not double encode XML' => [
'\\DesignerLongTextField',
<<<XML
<root>
<title id="title">Foo &amp; Bar</title>
</root>
XML,
<<<HTML
&lt;root&gt;
&lt;title id=&quot;title&quot;&gt;Foo &amp; Bar&lt;/title&gt;
&lt;/root&gt;
HTML
],
'DesignerXMLField should double encode XML' => [
'\\DesignerXMLField',
<<<XML
<root>
<title id="title">Foo &amp; Bar</title>
</root>
XML,
<<<HTML
&lt;root&gt;
&lt;title id=&quot;title&quot;&gt;Foo &amp;amp; Bar&lt;/title&gt;
&lt;/root&gt;
HTML
],
];
}
}

21
tests/php-unit-tests/unitary-tests/application/utilsTest.php
View File

@@ -841,17 +841,19 @@ class utilsTest extends ItopTestCase
*
* @dataProvider escapeHtmlProvider
*/
public function testEscapeHtml($sInput, $sExpectedEscaped)
public function testEscapeHtml($sInput, $sExpectedEscaped, $bDoubleEncode = false)
{
if (is_null($sExpectedEscaped)) {
$sExpectedEscaped = $sInput;
}
$sEscaped = utils::EscapeHtml($sInput);
$sEscaped = utils::EscapeHtml($sInput, $bDoubleEncode);
self::assertSame($sExpectedEscaped, $sEscaped);
$sEscapedDecoded = utils::EscapedHtmlDecode($sEscaped);
self::assertSame($sInput, $sEscapedDecoded);
if (false === $bDoubleEncode) {
self::assertSame($sInput, $sEscapedDecoded);
}
}
public function escapeHtmlProvider()
@@ -859,8 +861,17 @@ class utilsTest extends ItopTestCase
return [
'no escape' => ['abcdefghijklmnop', null],
'&amp;' => ['abcdefghijklmnop&0123456789', 'abcdefghijklmnop&amp;0123456789'],
['"double quotes"', '&quot;double quotes&quot;'],
["'simple quotes'", '&apos;simple quotes&apos;'],
'double quotes' => ['"double quotes"', '&quot;double quotes&quot;'],
'simple quotes' => ["'simple quotes'", '&apos;simple quotes&apos;'],
'no double encode' => [
'<root><title>Foo & Bar</title></root>',
'&lt;root&gt;&lt;title&gt;Foo &amp; Bar&lt;/title&gt;&lt;/root&gt;'
],
'double encode forced (for XML mostly)' => [
'<root><title>Foo &amp; Bar</title></root>',
'&lt;root&gt;&lt;title&gt;Foo &amp;amp; Bar&lt;/title&gt;&lt;/root&gt;',
true
],
];
}
}