Ticket #91 (closed bug: worksforme)
Search on unknown string returns "Array" as search key
Reported by: | remy | Owned by: | |
---|---|---|---|
Priority: | trivial | Milestone: | 2.2 |
Component: | explorer | Version: | trunk |
Keywords: | Cc: |
Description
A search on unknown string results in
"Sorry, nothing was found for Array"
Attachments
Change History
comment:2 Changed 8 years ago by remy
Also, entering the string '0' is interpreted as being "false".
comment:3 Changed 8 years ago by remy
Escaping of user input is not done too... XSS problem:
Try using '<B>Hi</B>'
comment:4 Changed 8 years ago by 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() {
Changed 8 years ago by sebastian
- Attachment svn.ticket93.search-on-unknown-string.patch added
untouch quicksearch input and escape of output
comment:5 Changed 8 years ago by sebastian
- Status changed from new to closed
- Resolution set to worksforme
- Milestone set to 2.2
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!
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...