Bug 385771 - disallow item-annotations for invalid item ids. r=dietrich.

toolkit/components/places/src/nsAnnotationService.cpp

@@ -49,6 +49,7 @@
 #include "nsIVariant.h"
 #include "nsString.h"
 #include "nsVariant.h"
+#include "nsNavBookmarks.h"
 
 const PRInt32 nsAnnotationService::kAnnoIndex_ID = 0;
 const PRInt32 nsAnnotationService::kAnnoIndex_PageOrItem = 1;
@@ -1624,6 +1625,14 @@ nsAnnotationService::StartSetAnnotation(PRInt64 aFkId,
                                         PRUint16 aType,
                                         mozIStorageStatement** aStatement)
 {
+  // Disallow setting item-annotations on invalid item ids
+  if (aIsItemAnnotation) {
+    nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService();
+    NS_ENSURE_STATE(bookmarks);
+    if (!bookmarks->ItemExists(aFkId))
+      return NS_ERROR_INVALID_ARG;
+  }
+
   PRBool hasAnnotation;
   PRInt64 annotationID;
   nsresult rv = HasAnnotationInternal(aFkId, aIsItemAnnotation, aName,

toolkit/components/places/src/nsNavBookmarks.cpp

@@ -2594,3 +2594,19 @@ nsNavBookmarks::OnItemAnnotationRemoved(PRInt64 aItemId, const nsACString& aName
 
   return NS_OK;
 }
+
+PRBool
+nsNavBookmarks::ItemExists(PRInt64 aItemId) {
+  mozStorageStatementScoper scope(mDBGetItemProperties);
+  nsresult rv = mDBGetItemProperties->BindInt64Parameter(0, aItemId);
+  NS_ENSURE_SUCCESS(rv, PR_FALSE);
+
+  PRBool results;
+  rv = mDBGetItemProperties->ExecuteStep(&results);
+  NS_ENSURE_SUCCESS(rv, PR_FALSE);
+
+  if (!results)
+    return PR_FALSE;
+
+  return PR_TRUE;
+}

toolkit/components/places/src/nsNavBookmarks.h

@@ -110,6 +110,8 @@ public:
   nsresult BeginUpdateBatch();
   nsresult EndUpdateBatch();
 
+  PRBool ItemExists(PRInt64 aItemId);
+
 private:
   static nsNavBookmarks *sInstance;
 

toolkit/components/places/tests/unit/test_annotations.js

@@ -355,5 +355,15 @@ function run_test() {
   do_check_eq(annosvc.getItemsWithAnnotation(int32Key, { }).length, 0);
   do_check_eq(annosvc.getPagesWithAnnotation(int32Key, { }).length, 0);
 
+  // Setting item annotations on invalid item ids should throw
+  var invalidIds = [-1, 0, 37643];
+  for each (var id in invalidIds) {
+    try {
+      annosvc.setItemAnnotationString(id, "foo", "bar", 0, 0);
+      do_throw("setItemAnnotation* should throw for invalid item id: " + id)
+    }
+    catch(ex) { }
+  }
+
   annosvc.removeObserver(annoObserver);
 }