Merge pull request #295 from oat-sa/develop

Merge for release 16.1.8 fix: XML-encode a value before inserting it into DOM
oat-sa · Jul 16, 2021 · 99b522a · 99b522a
2 parents 661f119 + dfeaecd
commit 99b522a
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 15 deletions.
diff --git a/src/qtism/data/storage/xml/Utils.php b/src/qtism/data/storage/xml/Utils.php
@@ -124,7 +124,7 @@ public static function anonimizeElement(DOMElement $element)
             $node = $stack->pop();
 
             if ($node->nodeType === XML_ELEMENT_NODE && $node->childNodes->length > 0 && in_array($node, $traversed, true) === false) {
-                array_push($traversed, $node);
+                $traversed[] = $node;
                 $stack->push($node);
 
                 for ($i = 0; $i < $node->childNodes->length; $i++) {
@@ -144,9 +144,9 @@ public static function anonimizeElement(DOMElement $element)
                     $newNode->appendChild(array_pop($children));
                 }
 
-                array_push($children, $newNode);
+                $children[] = $newNode;
             } else {
-                array_push($children, $node->cloneNode());
+                $children[] = $node->cloneNode();
             }
         }
 
@@ -207,17 +207,14 @@ public static function importAttributes(DOMElement $from, DOMElement $into)
      */
     public static function escapeXmlSpecialChars($string, $isAttribute = false)
     {
-        if ($isAttribute === false) {
-            $fullSearch = ['"', "'", '<', '>'];
-            $fullReplace = ['&quot;', '&apos;', '&lt;', '&gt;'];
-            $string = str_replace('&', '&amp;', $string);
-            $string = str_replace($fullSearch, $fullReplace, $string);
-            return $string;
-        } else {
-            $string = str_replace('&', '&amp;', $string);
-            $string = str_replace('"', '&quot;', $string);
-            return $string;
+        if ($isAttribute !== false) {
+            return str_replace(['&', '"'], ['&amp;', '&quot;'], $string);
         }
+
+        $fullSearch = ['&', '"', "'", '<', '>'];
+        $fullReplace = ['&amp;', '&quot;', '&apos;', '&lt;', '&gt;'];
+
+        return str_replace($fullSearch, $fullReplace, $string);
     }
 
     /**
@@ -300,7 +297,8 @@ public static function getDOMElementAttributeAs(DOMElement $element, string $att
                 return $attr === 'true';
         }
 
-        if (in_array(Enumeration::class, class_implements($datatype), true)){
+        if (in_array(Enumeration::class, class_implements($datatype), true)) {
+            /** @var Enumeration $datatype */
             if ($attr !== null) {
                 $constant = $datatype::getConstantByName($attr);
                 // Returns the original value when it's unknown in the enumeration.
@@ -352,7 +350,7 @@ public static function valueAsString($value)
         if (is_bool($value)) {
             return $value === true ? 'true' : 'false';
         }
-        return (string)$value;
+        return htmlspecialchars($value, ENT_XML1, 'UTF-8');
     }
 
     /**

diff --git a/test/qtismtest/data/storage/xml/marshalling/TemplateDeclarationMarshallerTest.php b/test/qtismtest/data/storage/xml/marshalling/TemplateDeclarationMarshallerTest.php
@@ -3,6 +3,7 @@
 namespace qtismtest\data\storage\xml\marshalling;
 
 use DOMDocument;
+use qtism\common\collections\AbstractCollection;
 use qtism\common\enums\BaseType;
 use qtism\common\enums\Cardinality;
 use qtism\data\state\DefaultValue;
@@ -48,6 +49,37 @@ public function testUnmarshall21()
         $this::assertEquals('tplx', $values[0]->getValue());
     }
 
+    public function testMarshallHtmlEntities21()
+    {
+        $values = new ValueCollection([new Value('non&nbsp;breaking&nbsp;space', BaseType::STRING)]);
+        $defaultValue = new DefaultValue($values);
+        $templateDeclaration = new TemplateDeclaration('tpl1', BaseType::STRING, Cardinality::SINGLE, $defaultValue);
+        $element = $this->getMarshallerFactory('2.1.0')->createMarshaller($templateDeclaration)->marshall($templateDeclaration);
+
+        $dom = new DOMDocument('1.0', 'UTF-8');
+        $element = $dom->importNode($element, true);
+        $this::assertEquals('<templateDeclaration identifier="tpl1" cardinality="single" baseType="string"><defaultValue><value>non&amp;nbsp;breaking&amp;nbsp;space</value></defaultValue></templateDeclaration>', $dom->saveXML($element));
+    }
+
+    public function testUnmarshallHtmlEntities21()
+    {
+        $element = $this->createDOMElement('
+	        <templateDeclaration identifier="tpl1" cardinality="single" baseType="string"><defaultValue><value>non&amp;nbsp;breaking&amp;nbsp;space</value></defaultValue></templateDeclaration>
+	    ');
+
+        $component = $this->getMarshallerFactory('2.1.0')->createMarshaller($element)->unmarshall($element);
+        $this::assertInstanceOf(TemplateDeclaration::class, $component);
+        $this::assertEquals('tpl1', $component->getIdentifier());
+        $this::assertEquals(Cardinality::SINGLE, $component->getCardinality());
+        $this::assertEquals(BaseType::STRING, $component->getBaseType());
+
+        $default = $component->getDefaultValue();
+        $this::assertInstanceOf(DefaultValue::class, $default);
+        $values = $default->getValues();
+        $this::assertCount(1, $values);
+        $this::assertEquals('non&nbsp;breaking&nbsp;space', $values[0]->getValue());
+    }
+
     public function testMarshall20()
     {
         // Make sure that paramVariable and mathVariable appear in output even