Make it clear whether addMessages expects string or Message object

With this, it's way easier to anaylize code for potential problems with escaping. Signed-off-by: Michal Čihař <michal@cihar.com>
phpmyadmin · Jun 16, 2016 · a7cd193 · a7cd193
1 parent a87c164
commit a7cd193
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 10 deletions.
diff --git a/libraries/Message.php b/libraries/Message.php
@@ -473,15 +473,32 @@ public function addString($string, $separator = ' ')
     /**
      * add a bunch of messages at once
      *
-     * @param array  $messages  to be added
-     * @param string $separator to use between this and previous string/message
+     * @param Message[] $messages  to be added
+     * @param string    $separator to use between this and previous string/message
      *
      * @return void
      */
     public function addMessages($messages, $separator = ' ')
     {
         foreach ($messages as $message) {
-            $this->addMessage($message, $separator);
+            $this->addedMessages[] = $separator;
+            $this->addedMessages[] = $message;
+        }
+    }
+
+    /**
+     * add a bunch of messages at once
+     *
+     * @param string[] $messages  to be added
+     * @param string   $separator to use between this and previous string/message
+     *
+     * @return void
+     */
+    public function addMessagesString($messages, $separator = ' ')
+    {
+        foreach ($messages as $message) {
+            $this->addedMessages[] = $separator;
+            $this->addedMessages[] = Message::notice(Message::sanitize($message));
         }
     }
 

diff --git a/libraries/insert_edit.lib.php b/libraries/insert_edit.lib.php
@@ -1959,7 +1959,7 @@ function PMA_executeSqlQuery($url_params, $query)
             $result = $GLOBALS['dbi']->query($single_query);
         }
         if (! $result) {
-            $error_messages[] = Message::sanitize($GLOBALS['dbi']->getError());
+            $error_messages[] = $GLOBALS['dbi']->getError();
         } else {
             // The next line contains a real assignment, it's not a typo
             if ($tmp = @$GLOBALS['dbi']->affectedRows()) {

diff --git a/tbl_operations.php b/tbl_operations.php
@@ -225,7 +225,7 @@
     }
     if (! empty($warning_messages)) {
         $_message = new PMA\libraries\Message;
-        $_message->addMessages($warning_messages);
+        $_message->addMessagesString($warning_messages);
         $_message->isError(true);
         if (isset($GLOBALS['ajax_request'])
             && $GLOBALS['ajax_request'] == true

diff --git a/tbl_replace.php b/tbl_replace.php
@@ -349,11 +349,11 @@
 $message->addMessages($last_messages, '<br />');
 
 if (! empty($warning_messages)) {
-    $message->addMessages($warning_messages, '<br />');
+    $message->addMessagesString($warning_messages, '<br />');
     $message->isError(true);
 }
 if (! empty($error_messages)) {
-    $message->addMessages($error_messages);
+    $message->addMessagesString($error_messages);
     $message->isError(true);
 }
 unset(

diff --git a/test/classes/MessageTest.php b/test/classes/MessageTest.php
@@ -298,21 +298,52 @@ public function testAddMessage()
     public function testAddMessages()
     {
         $messages = array();
-        $messages[] = "Test1";
+        $messages[] = new PMA\libraries\Message("Test1");
         $messages[] = new PMA\libraries\Message("PMA_Test2", PMA\libraries\Message::ERROR);
-        $messages[] = "Test3";
+        $messages[] = new PMA\libraries\Message("Test3");
         $this->object->addMessages($messages, '');
 
         $this->assertEquals(
             array(
+                '',
                 PMA\libraries\Message::notice('Test1'),
+                '',
                 PMA\libraries\Message::error("PMA_Test2"),
+                '',
                 PMA\libraries\Message::notice('Test3')
             ),
             $this->object->getAddedMessages()
         );
     }
 
+    /**
+     * testing add messages method
+     *
+     * @return void
+     */
+    public function testAddMessagesString()
+    {
+        $messages = array('test1', 'test<b>', 'test2');
+        $this->object->addMessagesString($messages, '');
+
+        $this->assertEquals(
+            array(
+                '',
+                PMA\libraries\Message::notice('test1'),
+                '',
+                PMA\libraries\Message::notice('test&lt;b&gt;'),
+                '',
+                PMA\libraries\Message::notice('test2')
+            ),
+            $this->object->getAddedMessages()
+        );
+
+        $this->assertEquals(
+            'test1test&lt;b&gt;test2',
+            $this->object->getMessage()
+        );
+    }
+
     /**
      * testing setter of params
      *

diff --git a/view_operations.php b/view_operations.php
@@ -78,7 +78,7 @@
     }
     if (! empty($warning_messages)) {
         $_message = new PMA\libraries\Message;
-        $_message->addMessages($warning_messages);
+        $_message->addMessagesString($warning_messages);
         $_message->isError(true);
         unset($warning_messages);
     }