diff --git a/composer.json b/composer.json index aa7fb8f26..600ce77d8 100644 --- a/composer.json +++ b/composer.json @@ -1,5 +1,6 @@ { "require": { - "marcelklehr/link-preview": "^2" + "marcelklehr/link-preview": "^2", + "glenscott/url-normalizer": "^1.4" } } diff --git a/controller/lib/bookmarks.php b/controller/lib/bookmarks.php index 81a870b2e..4ec183cf5 100644 --- a/controller/lib/bookmarks.php +++ b/controller/lib/bookmarks.php @@ -49,6 +49,9 @@ class Bookmarks { /** @var EventDispatcherInterface */ private $eventDispatcher; + /** @var UrlNormalizer */ + private $urlNormalizer; + /** @var ILogger */ private $logger; @@ -57,6 +60,7 @@ public function __construct( IConfig $config, IL10N $l, LinkExplorer $linkExplorer, + UrlNormalizer $urlNormalizer, EventDispatcherInterface $eventDispatcher, ILogger $logger ) { @@ -65,6 +69,7 @@ public function __construct( $this->l = $l; $this->linkExplorer = $linkExplorer; $this->eventDispatcher = $eventDispatcher; + $this->urlNormalizer = $urlNormalizer; $this->logger = $logger; } @@ -130,6 +135,10 @@ public function findUniqueBookmark($id, $userId) { */ public function bookmarkExists($url, $userId) { $encodedUrl = htmlspecialchars_decode($url); + + // normalize url + $encodedUrl = $this->urlNormalizer->normalize($encodedUrl); + $qb = $this->db->getQueryBuilder(); $qb ->select('id') @@ -420,6 +429,9 @@ public function editBookmark($userid, $id, $url, $title, $tags = [], $descriptio $isPublic = $isPublic ? 1 : 0; + // normalize url + $url = $this->urlNormalizer->normalize($url); + // Update the record $qb = $this->db->getQueryBuilder(); @@ -513,12 +525,15 @@ public function addBookmark($userid, $url, $title, $tags = array(), $description } // Check if it is a valid URL (after adding http(s) prefix) - $urlData = parse_url($url); + $urlData = parse_url($url); if(!$this->isProperURL($urlData)) { throw new \InvalidArgumentException('Invalid URL supplied'); } - $urlWithoutPrefix = trim(substr($url, strpos($url, "://") + 3)); // Removes everything from the url before the "://" pattern (included) + // normalize url + $url = $this->urlNormalizer->normalize($url); + + $urlWithoutPrefix = trim(substr($url, strpos($url, "://") + 3)); // Removes everything from the url before the "://" pattern (included) $decodedUrlNoPrefix = htmlspecialchars_decode($urlWithoutPrefix); $decodedUrl = htmlspecialchars_decode($url); @@ -531,12 +546,10 @@ public function addBookmark($userid, $url, $title, $tags = array(), $description $qb ->select('*') ->from('bookmarks') - ->where($qb->expr()->like('url', $qb->createParameter('url'))) // Find url in the db independantly from its protocol - ->andWhere($qb->expr()->eq('user_id', $qb->createParameter('userID'))); - $qb->setParameters([ - 'userID' => $userid, - 'url' => '%' . $this->db->escapeLikeParameter($decodedUrlNoPrefix) - ]); + ->where($qb->expr()->like('url', $qb->createPositionalParameter( + '%' . $this->db->escapeLikeParameter($decodedUrlNoPrefix) + ))) // Find url in the db independantly from its protocol + ->andWhere($qb->expr()->eq('user_id', $qb->createPositionalParameter($userid))); $row = $qb->execute()->fetch(); if ($row) { diff --git a/controller/lib/urlnormalizer.php b/controller/lib/urlnormalizer.php new file mode 100644 index 000000000..aff60ddbf --- /dev/null +++ b/controller/lib/urlnormalizer.php @@ -0,0 +1,24 @@ +normalizer = new Normalizer(); + } + + /** + * @brief Normalize Url + * @param string $url Url to load and analyze + * @return string Normalized url; + */ + public function normalize($url) { + $this->normalizer->setUrl($url); + return $this->normalizer->normalize(); + } + +} diff --git a/tests/bookmarkcontroller_test.php b/tests/bookmarkcontroller_test.php index 4cd173b35..3abc91ed0 100644 --- a/tests/bookmarkcontroller_test.php +++ b/tests/bookmarkcontroller_test.php @@ -5,6 +5,7 @@ use OCA\Bookmarks\Controller\Rest\BookmarkController; use OCA\Bookmarks\Controller\Lib\Bookmarks; use OCA\Bookmarks\Controller\Lib\LinkExplorer; +use OCA\Bookmarks\Controller\Lib\UrlNormalizer; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; @@ -36,18 +37,19 @@ protected function setUp() { $this->db = \OC::$server->getDatabaseConnection(); $this->userManager = \OC::$server->getUserManager(); if (!$this->userManager->userExists($this->userid)) { - $this->userManager->createUser($this->userid, 'password'); + $this->userManager->createUser($this->userid, 'password'); } if (!$this->userManager->userExists($this->otherUser)) { - $this->userManager->createUser($this->otherUser, 'password'); + $this->userManager->createUser($this->otherUser, 'password'); } $config = \OC::$server->getConfig(); $l = \OC::$server->getL10N('bookmarks'); $linkExplorer = \OC::$server->query(LinkExplorer::class); + $urlNormalizer = \OC::$server->query(UrlNormalizer::class); $event = \OC::$server->getEventDispatcher(); $logger = \OC::$server->getLogger(); - $this->libBookmarks = new Bookmarks($this->db, $config, $l, $linkExplorer, $event, $logger); + $this->libBookmarks = new Bookmarks($this->db, $config, $l, $linkExplorer, $urlNormalizer, $event, $logger); $this->controller = new BookmarkController("bookmarks", $this->request, $this->userid, $this->db, $l, $this->libBookmarks, $this->userManager); $this->publicController = new BookmarkController("bookmarks", $this->request, $this->otherUser, $this->db, $l, $this->libBookmarks, $this->userManager); @@ -57,14 +59,14 @@ function setupBookmarks() { $this->testSubjectPrivateBmId = $this->libBookmarks->addBookmark($this->userid, "https://www.golem.de", "Golem", array("four"), "PublicNoTag", false); $this->testSubjectPublicBmId = $this->libBookmarks->addBookmark($this->userid, "https://9gag.com", "9gag", array("two", "three"), "PublicTag", true); } - + function testPrivateRead() { $this->cleanDB(); $this->setupBookmarks(); $output = $this->controller->getSingleBookmark($this->testSubjectPublicBmId); $data = $output->getData(); $this->assertEquals('success', $data['status']); - $this->assertEquals("https://9gag.com", $data['item']['url']); + $this->assertEquals("https://9gag.com/", $data['item']['url']); } function testPublicReadSuccess() { @@ -73,9 +75,9 @@ function testPublicReadSuccess() { $output = $this->publicController->getSingleBookmark($this->testSubjectPublicBmId, $this->userid); $data = $output->getData(); $this->assertEquals('success', $data['status']); - $this->assertEquals("https://9gag.com", $data['item']['url']); + $this->assertEquals("https://9gag.com/", $data['item']['url']); } - + function testPublicReadFailure() { $this->cleanDB(); $this->setupBookmarks(); @@ -100,7 +102,7 @@ function testPrivateQuery() { $data = $output->getData(); $this->assertEquals(2, count($data['data'])); } - + function testPublicQuery() { $this->cleanDB(); $this->setupBookmarks(); @@ -109,7 +111,7 @@ function testPublicQuery() { $data = $output->getData(); $this->assertEquals(1, count($data['data'])); } - + function testPublicCreate() { $this->cleanDB(); $this->setupBookmarks(); @@ -127,15 +129,15 @@ function testPublicCreate() { $data = $output->getData(); $this->assertEquals(2, count($data['data'])); } - + function testPrivateCreate() { $this->cleanDB(); $this->setupBookmarks(); $this->controller->newBookmark("https://www.heise.de", array("tags"=> array("four")), "Heise", false, "PublicNoTag"); - + // the bookmark should exist $this->assertNotEquals(false, $this->libBookmarks->bookmarkExists("https://www.heise.de", $this->userid)); - + // user should see this bookmark $output = $this->controller->getBookmarks(); $data = $output->getData(); @@ -146,23 +148,23 @@ function testPrivateCreate() { $data = $output->getData(); $this->assertEquals(1, count($data['data'])); } - + function testPrivateEditBookmark() { $this->cleanDB(); $this->setupBookmarks(); $id = $this->libBookmarks->addBookmark($this->userid, "https://www.heise.de", "Golem", array("four"), "PublicNoTag", true); $this->controller->editBookmark($id, 'https://www.heise.de', null, '', true, $id, ''); - + $bookmark = $this->libBookmarks->findUniqueBookmark($id, $this->userid); - $this->assertEquals("https://www.heise.de", $bookmark['url']); + $this->assertEquals("https://www.heise.de/", $bookmark['url']); // normalized URL } - + function testPrivateDeleteBookmark() { $this->cleanDB(); $this->setupBookmarks(); $id = $this->libBookmarks->addBookmark($this->userid, "https://www.google.com", "Heise", array("one", "two"), "PrivatTag", false); - + $this->controller->deleteBookmark($id); $this->assertFalse($this->libBookmarks->bookmarkExists("https://www.google.com", $this->userid)); } diff --git a/tests/lib_bookmark_test.php b/tests/lib_bookmark_test.php index ae3ee4dbc..d2fc95b70 100644 --- a/tests/lib_bookmark_test.php +++ b/tests/lib_bookmark_test.php @@ -4,6 +4,7 @@ use OCA\Bookmarks\Controller\Lib\Bookmarks; use OCA\Bookmarks\Controller\Lib\LinkExplorer; +use OCA\Bookmarks\Controller\Lib\UrlNormalizer; use OCP\User; /** @@ -27,11 +28,12 @@ protected function setUp() { $config = \OC::$server->getConfig(); $l = \OC::$server->getL10N('bookmarks'); $linkExplorer = \OC::$server->query(LinkExplorer::class); + $urlNormalizer = \OC::$server->query(UrlNormalizer::class); $event = \OC::$server->getEventDispatcher(); $logger = \OC::$server->getLogger(); - $this->libBookmarks = new Bookmarks($db, $config, $l, $linkExplorer, $event, $logger); + $this->libBookmarks = new Bookmarks($db, $config, $l, $linkExplorer, $urlNormalizer, $event, $logger); - $this->otherUser = "otheruser"; + $this->otherUser = "otheruser"; $this->userManager = \OC::$server->getUserManager(); if (!$this->userManager->userExists($this->otherUser)) { $this->userManager->createUser($this->otherUser, 'password'); @@ -199,13 +201,13 @@ function testFindUniqueBookmark() { function testEditBookmark() { $this->cleanDB(); - $control_bm_id = $this->libBookmarks->addBookmark($this->userid, "https://www.golem.de", "Golem", array("four"), "PublicNoTag", true); + $control_bm_id = $this->libBookmarks->addBookmark($this->userid, "https://www.golem.de/", "Golem", array("four"), "PublicNoTag", true); $this->libBookmarks->addBookmark($this->userid, "https://9gag.com", "9gag", array("two", "three"), "PublicTag", true); $id = $this->libBookmarks->addBookmark($this->userid, "https://www.heise.de", "Heise", array("one", "two"), "PrivatTag", false); - $this->libBookmarks->editBookmark($this->userid, $id, "https://www.google.de", "NewTitle", array("three", "four")); + $this->libBookmarks->editBookmark($this->userid, $id, "https://www.google.de/", "NewTitle", array("three", "four")); $bookmark = $this->libBookmarks->findUniqueBookmark($id, $this->userid); $this->assertEquals("NewTitle", $bookmark['title']); - $this->assertEquals("https://www.google.de", $bookmark['url']); + $this->assertEquals("https://www.google.de/", $bookmark['url']); $this->assertCount(2, $bookmark['tags']); $this->assertTrue(in_array('four', $bookmark['tags'])); $this->assertTrue(in_array('three', $bookmark['tags'])); @@ -213,7 +215,7 @@ function testEditBookmark() { // Make sure nothing else changed $control_bookmark = $this->libBookmarks->findUniqueBookmark($control_bm_id, $this->userid); $this->assertEquals("Golem", $control_bookmark['title']); - $this->assertEquals("https://www.golem.de", $control_bookmark['url']); + $this->assertEquals("https://www.golem.de/", $control_bookmark['url']); $this->assertEquals($control_bookmark['tags'], ['four']); }