Search on unknown string returns "Array" as search key

[remy]

A search on unknown string results in

"Sorry, nothing was found for Array"

[remy]

You do a preg_split() if $quicksearch is not empty in app/controllers/explorer_controller.php: quicksearch().

You should reset $quicksearch to the original input value at the end...

[remy]

Also, entering the string '0' is interpreted as being "false".

[remy]

Escaping of user input is not done too... XSS problem:

Try using '<B>Hi</B>'

[remy]

I got a unified diff as a patch for this. I'm not that familiar to phpCake, so I hope I made the correct assumptions that I have to use import to use the Sanitize class. I tested it and it seems to work.

--- phtagr/app/controllers/explorer_controller.php      2010-05-08 14:53:29.000000000 +0200
+++ /www/vhosts/vakantie/htdocs/app/controllers/explorer_controller.php 2010-06-01 19:35:33.000000000 +0200
@@ -21,6 +21,8 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */

+App::import('Sanitize');
+
 class ExplorerController extends AppController
 {
   var $components = array('RequestHandler', 'FilterManager', 'Search', 'QueryBuilder');
@@ -76,13 +78,13 @@
       // Split the query so that we have a list of tags/categories/locations.
       // For now we split at whitespaces, improvements could be made to not
       // split multiple tags/categories/locations enclosed in quotation marks
-      $quicksearch = preg_split('/\s+/', trim($quicksearch));
+      $search = preg_split('/\s+/', trim($quicksearch));

       // Reduce results to 6
       $this->Search->setShow(6);

       // Add tag to the query
-      $this->Search->addTags($quicksearch);
+      $this->Search->addTags($search);
       // Set variable dataTags for view
       $this->Search->setTagOp('OR');
       $this->set('dataTags', $this->Search->paginate());
@@ -90,13 +92,13 @@
       $this->Search->delTags();
       $this->Search->delTagOp();

-      $this->Search->addCategories($quicksearch);
+      $this->Search->addCategories($search);
       $this->Search->setCategoryOp('OR');
       $this->set('dataCategories', $this->Search->paginate());
       $this->Search->delCategories();
       $this->Search->delCategoryOp();

-      $this->Search->addLocations($quicksearch);
+      $this->Search->addLocations($search);
       $this->Search->setLocationOp('OR');
       $this->set('dataLocations', $this->Search->paginate());
       $this->Search->delLocations();
@@ -106,7 +108,7 @@
       $this->set('dataCategories', array());
       $this->set('dataLocations', array());
     }
-    $this->set('quicksearch', $quicksearch);
+    $this->set('quicksearch', Sanitize::html($quicksearch));
   }

   function query() {

[sebastian]

untouch quicksearch input and escape of output

[sebastian]

Thank you for uncover this issue. The assignment of $quicksearch = preg_split('/\s+/', trim($quicksearch)); is a bit strange which splits the input into words. So I introduced the variable $words instead of $search which reflects more the splitting.

Further the output could be escaped in the view views/explorer/quicksearch.ctp with the nice CakePHP's function h(), which wraps htmlspecialchars() (see  http://book.cakephp.org/view/121/Global-Functions).

Here is my proposed solution:

Index: controllers/explorer_controller.php
===================================================================
--- controllers/explorer_controller.php	(revision 544)
+++ controllers/explorer_controller.php	(working copy)
@@ -76,13 +76,13 @@
       // Split the query so that we have a list of tags/categories/locations.
       // For now we split at whitespaces, improvements could be made to not
       // split multiple tags/categories/locations enclosed in quotation marks
-      $quicksearch = preg_split('/\s+/', trim($quicksearch));
+      $words = preg_split('/\s+/', trim($quicksearch));
 
       // Reduce results to 6 
       $this->Search->setShow(6);
 
       // Add tag to the query
-      $this->Search->addTags($quicksearch);
+      $this->Search->addTags($words);
       // Set variable dataTags for view
       $this->Search->setTagOp('OR');
       $this->set('dataTags', $this->Search->paginate());
@@ -90,13 +90,13 @@
       $this->Search->delTags();
       $this->Search->delTagOp();
 
-      $this->Search->addCategories($quicksearch);
+      $this->Search->addCategories($words);
       $this->Search->setCategoryOp('OR');
       $this->set('dataCategories', $this->Search->paginate());
       $this->Search->delCategories();
       $this->Search->delCategoryOp();
 
-      $this->Search->addLocations($quicksearch);
+      $this->Search->addLocations($words);
       $this->Search->setLocationOp('OR');
       $this->set('dataLocations', $this->Search->paginate());
       $this->Search->delLocations();
Index: views/explorer/quicksearch.ctp
===================================================================
--- views/explorer/quicksearch.ctp	(revision 544)
+++ views/explorer/quicksearch.ctp	(working copy)
@@ -12,7 +12,7 @@
 
 if (count($dataTags) + count($dataCategories) + count($dataLocations) == 0): ?>
 <div class="info">
-Sorry, nothing was found for <?php echo $quicksearch; ?>
+Sorry, nothing was found for <?php echo h($quicksearch); ?>
 </div>
 <?php endif; ?>

I uploaded the patch file as attachment:svn.ticket93.search-on-unknown-string.patch which is easier to handle.

@remy: If you are OK with this patch I would submit it to trunk

And thank you again for submiting a patch! Great work!

[remy]

It's fine by me...

[sebastian]

Fixed and commited in r547