From: Jeremy Benoist <jeremy.benoist@gmail.com>
Date: Mon, 14 Jan 2019 16:01:21 +0000 (+0100)
Subject: Avoid error when a bad `order` parameter is given
X-Git-Tag: 2.3.7~12^2
X-Git-Url: https://git.immae.eu/?p=github%2Fwallabag%2Fwallabag.git;a=commitdiff_plain;h=78e3fafa3fab86638295fe1ee2a05a559bf56ab1

Avoid error when a bad `order` parameter is given

Only allowed parameter are asc & desc
---

diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
index 0b4e74a0..b2bad406 100644
--- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php
+++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
@@ -9,6 +9,7 @@ use Nelmio\ApiDocBundle\Annotation\ApiDoc;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 use Wallabag\CoreBundle\Entity\Entry;
@@ -98,24 +99,28 @@ class EntryRestController extends WallabagRestController
         $isArchived = (null === $request->query->get('archive')) ? null : (bool) $request->query->get('archive');
         $isStarred = (null === $request->query->get('starred')) ? null : (bool) $request->query->get('starred');
         $isPublic = (null === $request->query->get('public')) ? null : (bool) $request->query->get('public');
-        $sort = $request->query->get('sort', 'created');
-        $order = $request->query->get('order', 'desc');
+        $sort = strtolower($request->query->get('sort', 'created'));
+        $order = strtolower($request->query->get('order', 'desc'));
         $page = (int) $request->query->get('page', 1);
         $perPage = (int) $request->query->get('perPage', 30);
         $tags = \is_array($request->query->get('tags')) ? '' : (string) $request->query->get('tags', '');
         $since = $request->query->get('since', 0);
 
-        /** @var \Pagerfanta\Pagerfanta $pager */
-        $pager = $this->get('wallabag_core.entry_repository')->findEntries(
-            $this->getUser()->getId(),
-            $isArchived,
-            $isStarred,
-            $isPublic,
-            $sort,
-            $order,
-            $since,
-            $tags
-        );
+        try {
+            /** @var \Pagerfanta\Pagerfanta $pager */
+            $pager = $this->get('wallabag_core.entry_repository')->findEntries(
+                $this->getUser()->getId(),
+                $isArchived,
+                $isStarred,
+                $isPublic,
+                $sort,
+                $order,
+                $since,
+                $tags
+            );
+        } catch (\Exception $e) {
+            throw new BadRequestHttpException($e->getMessage());
+        }
 
         $pager->setMaxPerPage($perPage);
         $pager->setCurrentPage($page);
diff --git a/src/Wallabag/CoreBundle/Repository/EntryRepository.php b/src/Wallabag/CoreBundle/Repository/EntryRepository.php
index 83379998..cebce714 100644
--- a/src/Wallabag/CoreBundle/Repository/EntryRepository.php
+++ b/src/Wallabag/CoreBundle/Repository/EntryRepository.php
@@ -142,7 +142,7 @@ class EntryRepository extends EntityRepository
      *
      * @return Pagerfanta
      */
-    public function findEntries($userId, $isArchived = null, $isStarred = null, $isPublic = null, $sort = 'created', $order = 'ASC', $since = 0, $tags = '')
+    public function findEntries($userId, $isArchived = null, $isStarred = null, $isPublic = null, $sort = 'created', $order = 'asc', $since = 0, $tags = '')
     {
         $qb = $this->createQueryBuilder('e')
             ->leftJoin('e.tags', 't')
@@ -185,6 +185,10 @@ class EntryRepository extends EntityRepository
             }
         }
 
+        if (!\in_array(strtolower($order), ['asc', 'desc'], true)) {
+            throw new \Exception('Order "' . $order . '" parameter is wrong, allowed: asc or desc');
+        }
+
         if ('created' === $sort) {
             $qb->orderBy('e.id', $order);
         } elseif ('updated' === $sort) {
diff --git a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
index 58b617f3..2a1d2e15 100644
--- a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
+++ b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
@@ -242,6 +242,15 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $this->assertSame(2, $content['limit']);
     }
 
+    public function testGetStarredEntriesWithBadSort()
+    {
+        $this->client->request('GET', '/api/entries', ['starred' => 1, 'sort' => 'updated', 'order' => 'unknown']);
+
+        $this->assertSame(400, $this->client->getResponse()->getStatusCode());
+
+        $this->assertSame('application/json', $this->client->getResponse()->headers->get('Content-Type'));
+    }
+
     public function testGetStarredEntries()
     {
         $this->client->request('GET', '/api/entries', ['starred' => 1, 'sort' => 'updated']);