do not add bookmarks with an invalid URL

2 files changed, 19 insertions, 2 deletions

diff --git a/src/SemanticScuttle/Service/Bookmark.php b/src/SemanticScuttle/Service/Bookmark.php
index a30ad5f..919ca7a 100644
--- a/src/SemanticScuttle/Service/Bookmark.php
+++ b/src/SemanticScuttle/Service/Bookmark.php
@@ -435,6 +435,10 @@ class SemanticScuttle_Service_Bookmark extends SemanticScuttle_DbService
     /**
      * Adds a bookmark to the database.
      *
+     * Security checks are being made here, but no error reasons will be
+     * returned. It is the responsibility of the code that calls
+     * addBookmark() to verify the data.
+     *
      * @param string  $address     Full URL of the bookmark
      * @param string  $title       Bookmark title
      * @param string  $description Long bookmark description
@@ -453,7 +457,8 @@ class SemanticScuttle_Service_Bookmark extends SemanticScuttle_DbService
      * @param boolean $fromImport  True when the bookmark is from an import.
      * @param integer $sId         ID of user who creates the bookmark.
      *
-     * @return integer Bookmark ID
+     * @return mixed Integer bookmark ID if saving succeeded, false in
+     *               case of an error. Error reasons are not returned.
      */
     public function addBookmark(
         $address, $title, $description, $privateNote, $status, $tags,
@@ -466,6 +471,9 @@ class SemanticScuttle_Service_Bookmark extends SemanticScuttle_DbService
         }
 
         $address = $this->normalize($address);
+        if (!SemanticScuttle_Model_Bookmark::isValidUrl($address)) {
+            return false;
+        }
 
         /*
          * Note that if date is NULL, then it's added with a date and
diff --git a/tests/BookmarkTest.php b/tests/BookmarkTest.php
index e7ce488..7533f3a 100644
--- a/tests/BookmarkTest.php
+++ b/tests/BookmarkTest.php
@@ -65,7 +65,16 @@ class BookmarkTest extends TestBase
         $this->assertEquals('myShortName', $bm['bShort']);
     }
 
-    public function testHardCharactersInBookmarks()
+    public function testAddBookmarkInvalidUrl()
+    {
+        $retval = $this->bs->addBookmark(
+            'javascript:alert(123)', 'title', 'desc', 'priv',
+            0, array()
+        );
+        $this->assertFalse($retval, 'Bookmark with invalid URL was accepted');
+    }
+
+    public function testAddBookmarkWithSpecialCharacters()
     {
         $bs = $this->bs;
         $title = "title&é\"'(-è_çà)=";