From e6dcb38e2539557fb8e62401b4074fd7c2700068 Mon Sep 17 00:00:00 2001 From: TtuxX Date: Fri, 26 Dec 2014 13:14:13 +0100 Subject: [PATCH 1/6] Fix http / https protocol requirement Fix the http / https prefix requirement when adding a bookmark by : 1 - Constructing URLs for both protocols, 2 - Retrieving both pages titles, 3 - Choosing the one which works, with a systematic preference for https. --- controller/rest/bookmarkcontroller.php | 30 ++++++++++++++++++++------ 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/controller/rest/bookmarkcontroller.php b/controller/rest/bookmarkcontroller.php index 557a0bfa..a0354989 100644 --- a/controller/rest/bookmarkcontroller.php +++ b/controller/rest/bookmarkcontroller.php @@ -68,19 +68,35 @@ class BookmarkController extends ApiController { */ public function newBookmark($url = "", $item = array(), $from_own = 0, $title = "", $is_public = false, $description = "") { - // Check if it is a valid URL + if ($from_own == 0) { + // allow only http(s) and (s)ftp + $protocols = '/^[hs]{0,1}[tf]{0,1}tp[s]{0,1}\:\/\//i'; + // if not (allowed) protocol is given, assume http and https (and fetch both) + if (! preg_match($protocols, $url)) { + // append https to url and fetch it + $url_https = 'https://' . $url; + $datas_https = Bookmarks::getURLMetadata($url_https); + // append http to url and fetch it + $url_http = 'http://' . $url; + $datas_http = Bookmarks::getURLMetadata($url_http); + } + // adopt https if it works (switch to http if it doesn't) + if (isset($datas_https['title'])) { // test if https works + $title = $datas_https['title']; + $url = $url_https; + } elseif (isset($datas_http['title'])) { // otherwise test http for results + $title = $datas_http['title']; + $url = $url_http; + } + } + + // Check if it is a valid URL (after adding http(s) prefix) if (filter_var($url, FILTER_VALIDATE_URL) === FALSE) { return new JSONResponse(array('status' => 'error'), Http::STATUS_BAD_REQUEST); } $tags = isset($item['tags']) ? $item['tags'] : array(); - if ($from_own == 0) { - $datas = Bookmarks::getURLMetadata($url); - if (isset($datas['title'])) { - $title = $datas['title']; - } - } $id = Bookmarks::addBookmark($this->userId, $this->db, $url, $title, $tags, $description, $is_public); $bm = Bookmarks::findUniqueBookmark($id, $this->userId, $this->db); return new JSONResponse(array('item' => $bm, 'status' => 'success')); From a08b2525648b3f31759d06d5c874ef8ac9621a57 Mon Sep 17 00:00:00 2001 From: TtuxX Date: Fri, 26 Dec 2014 13:53:53 +0100 Subject: [PATCH 2/6] Update bookmarkcontroller.php --- controller/rest/bookmarkcontroller.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/controller/rest/bookmarkcontroller.php b/controller/rest/bookmarkcontroller.php index a0354989..21cb9e2c 100644 --- a/controller/rest/bookmarkcontroller.php +++ b/controller/rest/bookmarkcontroller.php @@ -71,8 +71,10 @@ class BookmarkController extends ApiController { if ($from_own == 0) { // allow only http(s) and (s)ftp $protocols = '/^[hs]{0,1}[tf]{0,1}tp[s]{0,1}\:\/\//i'; + if (preg_match($protocols, $url)) { + $datas = Bookmarks::getURLMetadata($url); // if not (allowed) protocol is given, assume http and https (and fetch both) - if (! preg_match($protocols, $url)) { + } elseif (! preg_match($protocols, $url)) { // append https to url and fetch it $url_https = 'https://' . $url; $datas_https = Bookmarks::getURLMetadata($url_https); @@ -80,8 +82,11 @@ class BookmarkController extends ApiController { $url_http = 'http://' . $url; $datas_http = Bookmarks::getURLMetadata($url_http); } - // adopt https if it works (switch to http if it doesn't) - if (isset($datas_https['title'])) { // test if https works + + if (isset($datas['title'])) { // prefer original url if working + $title = $datas['title']; + //url remains unchanged + } elseif (isset($datas_https['title'])) { // test if https works $title = $datas_https['title']; $url = $url_https; } elseif (isset($datas_http['title'])) { // otherwise test http for results From b6048dd3a843f5f2768827cbc4affe623288edab Mon Sep 17 00:00:00 2001 From: TtuxX Date: Mon, 5 Jan 2015 18:50:32 +0100 Subject: [PATCH 3/6] Update bookmarkcontroller.php --- controller/rest/bookmarkcontroller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/rest/bookmarkcontroller.php b/controller/rest/bookmarkcontroller.php index 21cb9e2c..b58f4996 100644 --- a/controller/rest/bookmarkcontroller.php +++ b/controller/rest/bookmarkcontroller.php @@ -74,7 +74,7 @@ class BookmarkController extends ApiController { if (preg_match($protocols, $url)) { $datas = Bookmarks::getURLMetadata($url); // if not (allowed) protocol is given, assume http and https (and fetch both) - } elseif (! preg_match($protocols, $url)) { + } else { // append https to url and fetch it $url_https = 'https://' . $url; $datas_https = Bookmarks::getURLMetadata($url_https); From 0c1a52e4d502c0d7d032ff7b591d9dfaa507286f Mon Sep 17 00:00:00 2001 From: TtuxX Date: Mon, 5 Jan 2015 22:22:43 +0100 Subject: [PATCH 4/6] Check if URL already exists before adding Changed the way `addBookmark` function checks if url already exists. Now the `addBookmark` function checks wether the url one want to bookmark exists or not, independantly from its protocol. In order to achieve this, the script : - Removes everything from the url before the `://` part (included) - Uses a `like` statement in the SQL query instead of `=` - Adds a `%` to the url at the query execution --- controller/lib/bookmarks.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/controller/lib/bookmarks.php b/controller/lib/bookmarks.php index 712b3cd6..d2adb952 100644 --- a/controller/lib/bookmarks.php +++ b/controller/lib/bookmarks.php @@ -426,11 +426,12 @@ class Bookmarks { */ public static function addBookmark($userid, IDb $db, $url, $title, $tags = array(), $description = '', $is_public = false) { $public = $is_public ? 1 : 0; - $enc_url = htmlspecialchars_decode($url); + $url_without_prefix = substr($url, strpos($url, "://") + 3); // Removes everything from the url before the "://" pattern (included) + $enc_url = htmlspecialchars_decode($url_without_prefix); // Change lastmodified date if the record if already exists - $sql = "SELECT * from `*PREFIX*bookmarks` WHERE `url` = ? AND `user_id` = ?"; + $sql = "SELECT * from `*PREFIX*bookmarks` WHERE `url` like ? AND `user_id` = ?"; $query = $db->prepareQuery($sql, 1); - $result = $query->execute(array($enc_url, $userid)); + $result = $query->execute(array('%'.$enc_url, $userid)); // Find url in the db independantly from its protocol if ($row = $result->fetchRow()) { $params = array(); $title_str = ''; From bc1b9daa1d02ae9aa53c9071c27c3cb00d007711 Mon Sep 17 00:00:00 2001 From: TtuxX Date: Sat, 14 Feb 2015 17:01:41 +0100 Subject: [PATCH 5/6] Fix protocol cut off + Attempt URL https upgrade - Fix protocol cut off bug - Automatically attemps to upgrade existing URL from http to https if user add it a second time to bookmarks application (without specifying protocol) --- controller/lib/bookmarks.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/controller/lib/bookmarks.php b/controller/lib/bookmarks.php index d2adb952..9f5a93ee 100644 --- a/controller/lib/bookmarks.php +++ b/controller/lib/bookmarks.php @@ -427,11 +427,12 @@ class Bookmarks { public static function addBookmark($userid, IDb $db, $url, $title, $tags = array(), $description = '', $is_public = false) { $public = $is_public ? 1 : 0; $url_without_prefix = substr($url, strpos($url, "://") + 3); // Removes everything from the url before the "://" pattern (included) - $enc_url = htmlspecialchars_decode($url_without_prefix); + $enc_url_noprefix = htmlspecialchars_decode($url_without_prefix); + $enc_url = htmlspecialchars_decode($url); // Change lastmodified date if the record if already exists $sql = "SELECT * from `*PREFIX*bookmarks` WHERE `url` like ? AND `user_id` = ?"; $query = $db->prepareQuery($sql, 1); - $result = $query->execute(array('%'.$enc_url, $userid)); // Find url in the db independantly from its protocol + $result = $query->execute(array('%'.$enc_url_noprefix, $userid)); // Find url in the db independantly from its protocol if ($row = $result->fetchRow()) { $params = array(); $title_str = ''; @@ -445,8 +446,9 @@ class Bookmarks { $params[] = $description; } $sql = "UPDATE `*PREFIX*bookmarks` SET `lastmodified` = " - . "UNIX_TIMESTAMP() $title_str $desc_str WHERE `url` = ? and `user_id` = ?"; + . "UNIX_TIMESTAMP() $title_str $desc_str , `url` = ? WHERE `url` like ? and `user_id` = ?"; $params[] = $enc_url; + $params[] = '%'.$enc_url_noprefix; $params[] = $userid; $query = $db->prepareQuery($sql); $query->execute($params); From e85ecbed55d8c13a68654fe84a4e811fdfe2ac93 Mon Sep 17 00:00:00 2001 From: TtuxX Date: Mon, 9 Mar 2015 20:31:38 +0100 Subject: [PATCH 6/6] Improve protocols regex --- controller/rest/bookmarkcontroller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/rest/bookmarkcontroller.php b/controller/rest/bookmarkcontroller.php index b58f4996..c90bbb40 100644 --- a/controller/rest/bookmarkcontroller.php +++ b/controller/rest/bookmarkcontroller.php @@ -70,7 +70,7 @@ class BookmarkController extends ApiController { if ($from_own == 0) { // allow only http(s) and (s)ftp - $protocols = '/^[hs]{0,1}[tf]{0,1}tp[s]{0,1}\:\/\//i'; + $protocols = '/^(https?|s?ftp)\:\/\//i'; if (preg_match($protocols, $url)) { $datas = Bookmarks::getURLMetadata($url); // if not (allowed) protocol is given, assume http and https (and fetch both)