diff options
author | Jérémy Benoist <j0k3r@users.noreply.github.com> | 2019-05-28 14:18:33 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-05-28 14:18:33 +0200 |
commit | 2cbee36a0184786644470a84fdd8c720cfcac58e (patch) | |
tree | 33c6040c050f85c537f8dbf5e91d8c281db092cd | |
parent | 48d136d3a08d7f4ca8e0d434d8104c746d31957d (diff) | |
parent | 629a3797bcef33943df8ef5631328e05d12634ed (diff) | |
download | wallabag-2cbee36a0184786644470a84fdd8c720cfcac58e.tar.gz wallabag-2cbee36a0184786644470a84fdd8c720cfcac58e.tar.zst wallabag-2cbee36a0184786644470a84fdd8c720cfcac58e.zip |
Merge pull request #3944 from shtrom/always-hash-exists-url
Always hash exists url
5 files changed, 80 insertions, 47 deletions
diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index d9d99c85..aaacdcdc 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php | |||
@@ -14,6 +14,7 @@ use Wallabag\CoreBundle\Entity\Entry; | |||
14 | use Wallabag\CoreBundle\Entity\Tag; | 14 | use Wallabag\CoreBundle\Entity\Tag; |
15 | use Wallabag\CoreBundle\Event\EntryDeletedEvent; | 15 | use Wallabag\CoreBundle\Event\EntryDeletedEvent; |
16 | use Wallabag\CoreBundle\Event\EntrySavedEvent; | 16 | use Wallabag\CoreBundle\Event\EntrySavedEvent; |
17 | use Wallabag\CoreBundle\Helper\UrlHasher; | ||
17 | 18 | ||
18 | class EntryRestController extends WallabagRestController | 19 | class EntryRestController extends WallabagRestController |
19 | { | 20 | { |
@@ -43,50 +44,45 @@ class EntryRestController extends WallabagRestController | |||
43 | 44 | ||
44 | $returnId = (null === $request->query->get('return_id')) ? false : (bool) $request->query->get('return_id'); | 45 | $returnId = (null === $request->query->get('return_id')) ? false : (bool) $request->query->get('return_id'); |
45 | 46 | ||
46 | $urls = $request->query->get('urls', []); | ||
47 | $hashedUrls = $request->query->get('hashed_urls', []); | 47 | $hashedUrls = $request->query->get('hashed_urls', []); |
48 | $hashedUrl = $request->query->get('hashed_url', ''); | ||
49 | if (!empty($hashedUrl)) { | ||
50 | $hashedUrls[] = $hashedUrl; | ||
51 | } | ||
48 | 52 | ||
49 | // handle multiple urls first | 53 | $urls = $request->query->get('urls', []); |
50 | if (!empty($hashedUrls)) { | 54 | $url = $request->query->get('url', ''); |
51 | $results = []; | 55 | if (!empty($url)) { |
52 | foreach ($hashedUrls as $hashedUrl) { | 56 | $urls[] = $url; |
53 | $res = $repo->findByHashedUrlAndUserId($hashedUrl, $this->getUser()->getId()); | 57 | } |
54 | |||
55 | $results[$hashedUrl] = $this->returnExistInformation($res, $returnId); | ||
56 | } | ||
57 | 58 | ||
58 | return $this->sendResponse($results); | 59 | $urlHashMap = []; |
60 | foreach ($urls as $urlToHash) { | ||
61 | $urlHash = UrlHasher::hashUrl($urlToHash); | ||
62 | $hashedUrls[] = $urlHash; | ||
63 | $urlHashMap[$urlHash] = $urlToHash; | ||
59 | } | 64 | } |
60 | 65 | ||
61 | // @deprecated, to be remove in 3.0 | 66 | if (empty($hashedUrls)) { |
62 | if (!empty($urls)) { | 67 | throw $this->createAccessDeniedException('URL is empty?, logged user id: ' . $this->getUser()->getId()); |
63 | $results = []; | 68 | } |
64 | foreach ($urls as $url) { | ||
65 | $res = $repo->findByUrlAndUserId($url, $this->getUser()->getId()); | ||
66 | 69 | ||
67 | $results[$url] = $this->returnExistInformation($res, $returnId); | 70 | $results = []; |
68 | } | 71 | foreach ($hashedUrls as $hashedUrlToSearch) { |
72 | $res = $repo->findByHashedUrlAndUserId($hashedUrlToSearch, $this->getUser()->getId()); | ||
69 | 73 | ||
70 | return $this->sendResponse($results); | 74 | $results[$hashedUrlToSearch] = $this->returnExistInformation($res, $returnId); |
71 | } | 75 | } |
72 | 76 | ||
73 | // let's see if it is a simple url? | 77 | $results = $this->replaceUrlHashes($results, $urlHashMap); |
74 | $url = $request->query->get('url', ''); | ||
75 | $hashedUrl = $request->query->get('hashed_url', ''); | ||
76 | 78 | ||
77 | if (empty($url) && empty($hashedUrl)) { | 79 | if (!empty($url) || !empty($hashedUrl)) { |
78 | throw $this->createAccessDeniedException('URL is empty?, logged user id: ' . $this->getUser()->getId()); | 80 | $hu = array_keys($results)[0]; |
79 | } | ||
80 | 81 | ||
81 | $method = 'findByUrlAndUserId'; | 82 | return $this->sendResponse(['exists' => $results[$hu]]); |
82 | if (!empty($hashedUrl)) { | ||
83 | $method = 'findByHashedUrlAndUserId'; | ||
84 | $url = $hashedUrl; | ||
85 | } | 83 | } |
86 | 84 | ||
87 | $res = $repo->$method($url, $this->getUser()->getId()); | 85 | return $this->sendResponse($results); |
88 | |||
89 | return $this->sendResponse(['exists' => $this->returnExistInformation($res, $returnId)]); | ||
90 | } | 86 | } |
91 | 87 | ||
92 | /** | 88 | /** |
@@ -805,6 +801,24 @@ class EntryRestController extends WallabagRestController | |||
805 | } | 801 | } |
806 | 802 | ||
807 | /** | 803 | /** |
804 | * Replace the hashedUrl keys in $results with the unhashed URL from the | ||
805 | * request, as recorded in $urlHashMap. | ||
806 | */ | ||
807 | private function replaceUrlHashes(array $results, array $urlHashMap) | ||
808 | { | ||
809 | $newResults = []; | ||
810 | foreach ($results as $hash => $res) { | ||
811 | if (isset($urlHashMap[$hash])) { | ||
812 | $newResults[$urlHashMap[$hash]] = $res; | ||
813 | } else { | ||
814 | $newResults[$hash] = $res; | ||
815 | } | ||
816 | } | ||
817 | |||
818 | return $newResults; | ||
819 | } | ||
820 | |||
821 | /** | ||
808 | * Retrieve value from the request. | 822 | * Retrieve value from the request. |
809 | * Used for POST & PATCH on a an entry. | 823 | * Used for POST & PATCH on a an entry. |
810 | * | 824 | * |
@@ -832,8 +846,8 @@ class EntryRestController extends WallabagRestController | |||
832 | /** | 846 | /** |
833 | * Return information about the entry if it exist and depending on the id or not. | 847 | * Return information about the entry if it exist and depending on the id or not. |
834 | * | 848 | * |
835 | * @param Entry|null $entry | 849 | * @param Entry|bool|null $entry |
836 | * @param bool $returnId | 850 | * @param bool $returnId |
837 | * | 851 | * |
838 | * @return bool|int | 852 | * @return bool|int |
839 | */ | 853 | */ |
diff --git a/src/Wallabag/CoreBundle/Command/GenerateUrlHashesCommand.php b/src/Wallabag/CoreBundle/Command/GenerateUrlHashesCommand.php index 45bd8c5f..8f2bff11 100644 --- a/src/Wallabag/CoreBundle/Command/GenerateUrlHashesCommand.php +++ b/src/Wallabag/CoreBundle/Command/GenerateUrlHashesCommand.php | |||
@@ -7,6 +7,7 @@ use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand; | |||
7 | use Symfony\Component\Console\Input\InputArgument; | 7 | use Symfony\Component\Console\Input\InputArgument; |
8 | use Symfony\Component\Console\Input\InputInterface; | 8 | use Symfony\Component\Console\Input\InputInterface; |
9 | use Symfony\Component\Console\Output\OutputInterface; | 9 | use Symfony\Component\Console\Output\OutputInterface; |
10 | use Wallabag\CoreBundle\Helper\UrlHasher; | ||
10 | use Wallabag\UserBundle\Entity\User; | 11 | use Wallabag\UserBundle\Entity\User; |
11 | 12 | ||
12 | class GenerateUrlHashesCommand extends ContainerAwareCommand | 13 | class GenerateUrlHashesCommand extends ContainerAwareCommand |
@@ -65,7 +66,7 @@ class GenerateUrlHashesCommand extends ContainerAwareCommand | |||
65 | 66 | ||
66 | $i = 1; | 67 | $i = 1; |
67 | foreach ($entries as $entry) { | 68 | foreach ($entries as $entry) { |
68 | $entry->setHashedUrl(hash('sha1', $entry->getUrl())); | 69 | $entry->setHashedUrl(UrlHasher::hashUrl($entry->getUrl())); |
69 | $em->persist($entry); | 70 | $em->persist($entry); |
70 | 71 | ||
71 | if (0 === ($i % 20)) { | 72 | if (0 === ($i % 20)) { |
@@ -84,7 +85,7 @@ class GenerateUrlHashesCommand extends ContainerAwareCommand | |||
84 | * | 85 | * |
85 | * @param string $username | 86 | * @param string $username |
86 | * | 87 | * |
87 | * @return \Wallabag\UserBundle\Entity\User | 88 | * @return User |
88 | */ | 89 | */ |
89 | private function getUser($username) | 90 | private function getUser($username) |
90 | { | 91 | { |
diff --git a/src/Wallabag/CoreBundle/Entity/Entry.php b/src/Wallabag/CoreBundle/Entity/Entry.php index c3fb87d2..1b4367fd 100644 --- a/src/Wallabag/CoreBundle/Entity/Entry.php +++ b/src/Wallabag/CoreBundle/Entity/Entry.php | |||
@@ -13,6 +13,7 @@ use JMS\Serializer\Annotation\XmlRoot; | |||
13 | use Symfony\Component\Validator\Constraints as Assert; | 13 | use Symfony\Component\Validator\Constraints as Assert; |
14 | use Wallabag\AnnotationBundle\Entity\Annotation; | 14 | use Wallabag\AnnotationBundle\Entity\Annotation; |
15 | use Wallabag\CoreBundle\Helper\EntityTimestampsTrait; | 15 | use Wallabag\CoreBundle\Helper\EntityTimestampsTrait; |
16 | use Wallabag\CoreBundle\Helper\UrlHasher; | ||
16 | use Wallabag\UserBundle\Entity\User; | 17 | use Wallabag\UserBundle\Entity\User; |
17 | 18 | ||
18 | /** | 19 | /** |
@@ -324,7 +325,7 @@ class Entry | |||
324 | public function setUrl($url) | 325 | public function setUrl($url) |
325 | { | 326 | { |
326 | $this->url = $url; | 327 | $this->url = $url; |
327 | $this->hashedUrl = hash('sha1', $url); | 328 | $this->hashedUrl = UrlHasher::hashUrl($url); |
328 | 329 | ||
329 | return $this; | 330 | return $this; |
330 | } | 331 | } |
diff --git a/src/Wallabag/CoreBundle/Helper/UrlHasher.php b/src/Wallabag/CoreBundle/Helper/UrlHasher.php new file mode 100644 index 00000000..d123eaba --- /dev/null +++ b/src/Wallabag/CoreBundle/Helper/UrlHasher.php | |||
@@ -0,0 +1,23 @@ | |||
1 | <?php | ||
2 | |||
3 | namespace Wallabag\CoreBundle\Helper; | ||
4 | |||
5 | /** | ||
6 | * Hash URLs for privacy and performance. | ||
7 | */ | ||
8 | class UrlHasher | ||
9 | { | ||
10 | /** | ||
11 | * Hash the given url using the given algorithm. | ||
12 | * Hashed url are faster to be retrieved in the database than the real url. | ||
13 | * | ||
14 | * @param string $url | ||
15 | * @param string $algorithm | ||
16 | * | ||
17 | * @return string | ||
18 | */ | ||
19 | public static function hashUrl(string $url, $algorithm = 'sha1') | ||
20 | { | ||
21 | return hash($algorithm, urldecode($url)); | ||
22 | } | ||
23 | } | ||
diff --git a/src/Wallabag/CoreBundle/Repository/EntryRepository.php b/src/Wallabag/CoreBundle/Repository/EntryRepository.php index 3990932e..880e7c65 100644 --- a/src/Wallabag/CoreBundle/Repository/EntryRepository.php +++ b/src/Wallabag/CoreBundle/Repository/EntryRepository.php | |||
@@ -9,6 +9,7 @@ use Pagerfanta\Adapter\DoctrineORMAdapter; | |||
9 | use Pagerfanta\Pagerfanta; | 9 | use Pagerfanta\Pagerfanta; |
10 | use Wallabag\CoreBundle\Entity\Entry; | 10 | use Wallabag\CoreBundle\Entity\Entry; |
11 | use Wallabag\CoreBundle\Entity\Tag; | 11 | use Wallabag\CoreBundle\Entity\Tag; |
12 | use Wallabag\CoreBundle\Helper\UrlHasher; | ||
12 | 13 | ||
13 | class EntryRepository extends EntityRepository | 14 | class EntryRepository extends EntityRepository |
14 | { | 15 | { |
@@ -348,17 +349,10 @@ class EntryRepository extends EntityRepository | |||
348 | */ | 349 | */ |
349 | public function findByUrlAndUserId($url, $userId) | 350 | public function findByUrlAndUserId($url, $userId) |
350 | { | 351 | { |
351 | $res = $this->createQueryBuilder('e') | 352 | return $this->findByHashedUrlAndUserId( |
352 | ->where('e.url = :url')->setParameter('url', urldecode($url)) | 353 | UrlHasher::hashUrl($url), |
353 | ->andWhere('e.user = :user_id')->setParameter('user_id', $userId) | 354 | $userId |
354 | ->getQuery() | 355 | ); |
355 | ->getResult(); | ||
356 | |||
357 | if (\count($res)) { | ||
358 | return current($res); | ||
359 | } | ||
360 | |||
361 | return false; | ||
362 | } | 356 | } |
363 | 357 | ||
364 | /** | 358 | /** |