[BUGFIX] Form values with newlines escaped in email
authorHelmut Hummel <helmut.hummel@typo3.org>
Wed, 17 Oct 2012 08:29:56 +0000 (10:29 +0200)
committerDmitry Dulepov <dmitry@typo3.org>
Tue, 27 Nov 2012 11:43:01 +0000 (12:43 +0100)
nl2br() is executed before htmlspecialchars(). Needs to be
the other way round so the <br/> is not encoded as well.

Change-Id: I8006e27393105dd425a17d14e7d9553e15d797be
Fixes: #32515
Releases: 4.6, 4.7, 6.0
Reviewed-on: http://review.typo3.org/9020
Reviewed-by: Dmitry Dulepov
Tested-by: Dmitry Dulepov
typo3/sysext/form/Classes/View/Mail/Html/Element/AbstractElementView.php
typo3/sysext/form/Tests/Unit/View/Mail/Html/Element/AbstractElementViewTest.php [new file with mode: 0644]

index e6ad541..1de456f 100644 (file)
@@ -70,10 +70,10 @@ abstract class AbstractElementView {
         * Parse the XML of a view object,
         * check the node type and name
         * and add the proper XML part of child tags
-        * to the DOMDocument of the current tag
+        * to the \DOMDocument of the current tag
         *
-        * @param DOMDocument $dom
-        * @param DOMNode $reference Current XML structure
+        * @param \DOMDocument $dom
+        * @param \DOMNode $reference Current XML structure
         * @param boolean $emptyElement
         * @return boolean
         */
@@ -167,11 +167,11 @@ abstract class AbstractElementView {
        }
 
        /**
-        * Get the content for the current object as DOMDocument
+        * Get the content for the current object as \DOMDocument
         *
         * @param string $type Type of element for layout
-        * @param boolean $returnFirstChild If TRUE, the first child will be returned instead of the DOMDocument
-        * @return mixed DOMDocument/DOMNode XML part of the view object
+        * @param boolean $returnFirstChild If TRUE, the first child will be returned instead of the \DOMDocument
+        * @return \DOMDocument|\DOMNode XML part of the view object
         */
        public function render($type = 'element', $returnFirstChild = TRUE) {
                $useLayout = $this->getLayout((string) $type);
@@ -198,6 +198,7 @@ abstract class AbstractElementView {
        public function getLayout($type) {
                /** @var $layoutHandler \TYPO3\CMS\Form\Layout */
                $layoutHandler = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Form\\Layout');
+               $layout = '';
                switch ($type) {
                case 'element':
                        $layoutDefault = $this->layout;
@@ -219,9 +220,9 @@ abstract class AbstractElementView {
        /**
         * Replace the current node with a document fragment
         *
-        * @param DOMDocument $dom
-        * @param DOMNode $node Current Node
-        * @param DOMNode $value Value to import
+        * @param \DOMDocument $dom
+        * @param \DOMNode $node Current Node
+        * @param \DOMNode $value Value to import
         * @return void
         */
        public function replaceNodeWithFragment(\DOMDocument $dom, \DOMNode $node, \DOMNode $value) {
@@ -235,7 +236,7 @@ abstract class AbstractElementView {
         * Set the attributes on the html tags according to the attributes that are
         * assigned in the model for a certain element
         *
-        * @param DOMElement $domElement DOM element of the specific HTML tag
+        * @param \DOMElement $domElement DOM element of the specific HTML tag
         * @return void
         */
        public function setAttributes(\DOMElement $domElement) {
@@ -253,7 +254,7 @@ abstract class AbstractElementView {
        /**
         * Set a single attribute of a HTML tag specified by key
         *
-        * @param DOMElement $domElement DOM element of the specific HTML tag
+        * @param \DOMElement $domElement DOM element of the specific HTML tag
         * @param string $key Attribute key
         * @return void
         */
@@ -268,10 +269,10 @@ abstract class AbstractElementView {
         * Sets the value of an attribute with the value of another attribute,
         * for instance equalizing the name and id attributes for the form tag
         *
-        * @param DOMElement $domElement DOM element of the specific HTML tag
+        * @param \DOMElement $domElement DOM element of the specific HTML tag
         * @param string $key Key of the attribute which needs to be changed
         * @param string $other Key of the attribute to take the value from
-        * @return unknown_type
+        * @return void
         */
        public function setAttributeWithValueofOtherAttribute(\DOMElement $domElement, $key, $other) {
                $value = htmlspecialchars($this->model->getAttributeValue((string) $other), ENT_QUOTES);
@@ -296,20 +297,26 @@ abstract class AbstractElementView {
         * Create additional object by key and render the content
         *
         * @param string $key Type of additional
-        * @return DOMNode
+        * @return \DOMNode
         */
        public function getAdditional($key) {
                $additional = $this->createAdditional($key);
                return $additional->render();
        }
 
+       /**
+        * @return string
+        */
        public function getInputValue() {
                if (method_exists($this->model, 'getData')) {
-                       $inputValue = nl2br($this->model->getData(), TRUE);
+                       $inputValue = $this->model->getData();
                } else {
                        $inputValue = $this->model->getAttributeValue('value');
                }
-               return htmlspecialchars($inputValue, ENT_QUOTES);
+
+               $inputValue = htmlspecialchars($inputValue, ENT_QUOTES);
+               $inputValue = nl2br($inputValue, TRUE);
+               return $inputValue;
        }
 
        /**
@@ -363,4 +370,4 @@ abstract class AbstractElementView {
 }
 
 
-?>
\ No newline at end of file
+?>
diff --git a/typo3/sysext/form/Tests/Unit/View/Mail/Html/Element/AbstractElementViewTest.php b/typo3/sysext/form/Tests/Unit/View/Mail/Html/Element/AbstractElementViewTest.php
new file mode 100644 (file)
index 0000000..faca885
--- /dev/null
@@ -0,0 +1,63 @@
+<?php
+/***************************************************************
+ *  Copyright notice
+ *
+ *  (c) 2012 Helmut Hummel <helmut.hummel@typo3.org>
+ *  All rights reserved
+ *
+ *  This script is part of the TYPO3 project. The TYPO3 project is
+ *  free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  The GNU General Public License can be found at
+ *  http://www.gnu.org/copyleft/gpl.html.
+ *
+ *  This script 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 General Public License for more details.
+ *
+ *  This copyright notice MUST APPEAR in all copies of the script!
+ ***************************************************************/
+
+namespace TYPO3\CMS\Form\Tests\View\Mail\Html\Element;
+
+/**
+ * Test case for class \TYPO3\CMS\Form\View\Mail\Html\Element\AbstractElementView.
+ *
+ * @author Stefan Neufeind
+ * @package TYPO3
+ * @subpackage form
+ */
+class AbstractElementViewTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
+
+       /**
+        * @var \TYPO3\CMS\Form\View\Confirmation\Element\AbstractElementView
+        */
+       protected $fixture;
+
+       /**
+        * Sets up the test.
+        *
+        * @return void
+        */
+       public function setUp() {
+               $model = $this->getMock('TYPO3\\CMS\\Form\\Domain\\Model\\Element\\AbstractElement');
+               $model->expects($this->once())->method('getAttributeValue')->with('value')->will($this->returnValue('a&' . LF . 'b'));
+               $this->fixture = $this->getMock('TYPO3\\CMS\\Form\\View\\Mail\\Html\\Element\\AbstractElementView', array('dummy'), array($model));
+       }
+
+       /**
+        * Tests that HTML escaping works correctly when mails are sent in HTML
+        * format.
+        *
+        * @see http://forge.typo3.org/issues/32515
+        * @test
+        */
+       public function htmlspecialcharsIsAppliedBeforeNl2br() {
+               $this->assertSame('a&amp;<br />' . LF . 'b', $this->fixture->getInputValue());
+       }
+}
+?>
\ No newline at end of file