nextcloud
/
bookmarks
mirror of https://github.com/nextcloud/bookmarks
0
0
Fork 0
Code Issues Releases Wiki Activity

Merge pull request #479 from nextcloud/fix/add-url-normalization 

Normalize URLs before writing to or querying db
This commit is contained in:
blizzz 2018-05-11 11:42:39 +02:00 committed by GitHub
parent c348fba2d2 3286963427
commit 32b840908d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 74 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
composer.json
View File

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

29
controller/lib/bookmarks.php
View File

@ -49,6 +49,9 @@ class Bookmarks {
/** @var EventDispatcherInterface */
private $eventDispatcher;
/** @var UrlNormalizer */
private $urlNormalizer;
/** @var ILogger */
private $logger;
@ -57,6 +60,7 @@ class Bookmarks {
IConfig $config,
IL10N $l,
LinkExplorer $linkExplorer,
UrlNormalizer $urlNormalizer,
EventDispatcherInterface $eventDispatcher,
ILogger $logger
) {
@ -65,6 +69,7 @@ class Bookmarks {
$this->l = $l;
$this->linkExplorer = $linkExplorer;
$this->eventDispatcher = $eventDispatcher;
$this->urlNormalizer = $urlNormalizer;
$this->logger = $logger;
}
@ -130,6 +135,10 @@ class Bookmarks {
*/
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 @@ class Bookmarks {
$isPublic = $isPublic ? 1 : 0;
// normalize url
$url = $this->urlNormalizer->normalize($url);
// Update the record
$qb = $this->db->getQueryBuilder();
@ -513,12 +525,15 @@ class Bookmarks {
}
// 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 @@ class Bookmarks {
$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) {

24
controller/lib/urlnormalizer.php Normal file
View File

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

36
tests/bookmarkcontroller_test.php
View File

@ -5,6 +5,7 @@ namespace OCA\Bookmarks\Tests;
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 @@ class Test_BookmarkController extends TestCase {
$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 @@ class Test_BookmarkController extends TestCase {
$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 @@ class Test_BookmarkController extends TestCase {
$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 @@ class Test_BookmarkController extends TestCase {
$data = $output->getData();
$this->assertEquals(2, count($data['data']));
}
function testPublicQuery() {
$this->cleanDB();
$this->setupBookmarks();
@ -109,7 +111,7 @@ class Test_BookmarkController extends TestCase {
$data = $output->getData();
$this->assertEquals(1, count($data['data']));
}
function testPublicCreate() {
$this->cleanDB();
$this->setupBookmarks();
@ -127,15 +129,15 @@ class Test_BookmarkController extends TestCase {
$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 @@ class Test_BookmarkController extends TestCase {
$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));
}

14
tests/lib_bookmark_test.php
View File

@ -4,6 +4,7 @@ namespace OCA\Bookmarks\Tests;
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 @@ class Test_LibBookmarks_Bookmarks extends TestCase {
$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 @@ class Test_LibBookmarks_Bookmarks extends TestCase {
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 @@ class Test_LibBookmarks_Bookmarks extends TestCase {
// 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']);
}