Normalize URLs before writing to or querying db

composer.json

@@ -1,5 +1,6 @@
 {
 	"require": {
-		"marcelklehr/link-preview": "^2"
+		"marcelklehr/link-preview": "^2",
+		"glenscott/url-normalizer": "^1.4"
   }
 }

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) {

controller/lib/urlnormalizer.php

@@ -0,0 +1,24 @@
+<?php
+namespace OCA\Bookmarks\Controller\Lib;
+
+use URL\Normalizer;
+
+class UrlNormalizer {
+
+	private $normalizer;
+
+	public function __construct() {
+		$this->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();
+	}
+
+}

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));
 	}

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,21 +201,21 @@ 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']));
 
 		// 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']);
 	}