Bug 692627: Support complex keyPaths in IndexedDB. r=bent,jorendorff

dom/indexedDB/IDBDatabase.cpp

@@ -512,6 +512,10 @@ IDBDatabase::CreateObjectStore(const nsAString& aName,
     autoIncrement = !!boolVal;
   }
 
+  if (!IDBObjectStore::IsValidKeyPath(aCx, keyPath)) {
+    return NS_ERROR_DOM_SYNTAX_ERR;
+  }
+
   nsAutoPtr<ObjectStoreInfo> newInfo(new ObjectStoreInfo());
 
   newInfo->name = aName;

dom/indexedDB/IDBObjectStore.cpp

@@ -43,6 +43,7 @@
 
 #include "jsclone.h"
 #include "mozilla/storage.h"
+#include "nsCharSeparatedTokenizer.h"
 #include "nsContentUtils.h"
 #include "nsDOMClassInfo.h"
 #include "nsEventDispatcher.h"
@@ -411,24 +412,51 @@ private:
 };
 
 inline
-nsresult
-GetKeyFromObject(JSContext* aCx,
-                 JSObject* aObj,
-                 const nsString& aKeyPath,
-                 Key& aKey)
+bool
+IgnoreWhitespace(PRUnichar c)
 {
-  NS_PRECONDITION(aCx && aObj, "Null pointers!");
-  NS_ASSERTION(!aKeyPath.IsVoid(), "This will explode!");
+  return false;
+}
 
-  const jschar* keyPathChars = reinterpret_cast<const jschar*>(aKeyPath.get());
-  const size_t keyPathLen = aKeyPath.Length();
+typedef nsCharSeparatedTokenizerTemplate<IgnoreWhitespace> KeyPathTokenizer;
 
-  jsval key;
-  JSBool ok = JS_GetUCProperty(aCx, aObj, keyPathChars, keyPathLen, &key);
-  NS_ENSURE_TRUE(ok, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+inline
+nsresult
+GetKeyFromValue(JSContext* aCx,
+                jsval aVal,
+                const nsAString& aKeyPath,
+                Key& aKey)
+{
+  NS_ASSERTION(aCx, "Null pointer!");
+  NS_ASSERTION(!JSVAL_IS_PRIMITIVE(aVal), "Why are we here!?");
+  NS_ASSERTION(IDBObjectStore::IsValidKeyPath(aCx, aKeyPath),
+               "This will explode!");
 
-  nsresult rv = aKey.SetFromJSVal(aCx, key);
-  NS_ENSURE_SUCCESS(rv, rv);
+  KeyPathTokenizer tokenizer(aKeyPath, '.');
+
+  jsval intermediate = aVal;
+  while (tokenizer.hasMoreTokens()) {
+    nsString token(tokenizer.nextToken());
+
+    if (!token.Length()) {
+      return NS_ERROR_DOM_SYNTAX_ERR;
+    }
+
+    const jschar* keyPathChars = token.get();
+    const size_t keyPathLen = token.Length();
+
+    if (JSVAL_IS_PRIMITIVE(intermediate)) {
+      return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
+    }
+
+    JSBool ok = JS_GetUCProperty(aCx, JSVAL_TO_OBJECT(intermediate),
+                                 keyPathChars, keyPathLen, &intermediate);
+    NS_ENSURE_TRUE(ok, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+  }
+
+  if (NS_FAILED(aKey.SetFromJSVal(aCx, intermediate))) {
+    aKey.Unset();
+  }
 
   return NS_OK;
 }
@@ -502,6 +530,46 @@ IDBObjectStore::Create(IDBTransaction* aTransaction,
   return objectStore.forget();
 }
 
+// static
+bool
+IDBObjectStore::IsValidKeyPath(JSContext* aCx,
+                               const nsAString& aKeyPath)
+{
+  NS_ASSERTION(!aKeyPath.IsVoid(), "What?");
+
+  KeyPathTokenizer tokenizer(aKeyPath, '.');
+
+  while (tokenizer.hasMoreTokens()) {
+    nsString token(tokenizer.nextToken());
+
+    if (!token.Length()) {
+      return false;
+    }
+
+    jsval stringVal;
+    if (!xpc_qsStringToJsval(aCx, token, &stringVal)) {
+      return false;
+    }
+
+    NS_ASSERTION(JSVAL_IS_STRING(stringVal), "This should never happen");
+    JSString* str = JSVAL_TO_STRING(stringVal);
+
+    JSBool isIdentifier = JS_FALSE;
+    if (!JS_IsIdentifier(aCx, str, &isIdentifier) || !isIdentifier) {
+      return false;
+    }
+  }
+
+  // If the very last character was a '.', the tokenizer won't give us an empty
+  // token, but the keyPath is still invalid.
+  if (!aKeyPath.IsEmpty() &&
+      aKeyPath.CharAt(aKeyPath.Length() - 1) == '.') {
+    return false;
+  }
+
+  return true;
+}
+
 // static
 nsresult
 IDBObjectStore::GetKeyPathValueFromStructuredData(const PRUint8* aData,
@@ -530,22 +598,8 @@ IDBObjectStore::GetKeyPathValueFromStructuredData(const PRUint8* aData,
     return NS_OK;
   }
 
-  JSObject* obj = JSVAL_TO_OBJECT(clone);
-
-  const jschar* keyPathChars =
-    reinterpret_cast<const jschar*>(aKeyPath.BeginReading());
-  const size_t keyPathLen = aKeyPath.Length();
-
-  jsval keyVal;
-  JSBool ok = JS_GetUCProperty(aCx, obj, keyPathChars, keyPathLen, &keyVal);
-  NS_ENSURE_TRUE(ok, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
-
-  nsresult rv = aValue.SetFromJSVal(aCx, keyVal);
-  if (NS_FAILED(rv)) {
-    // If the object doesn't have a value that we can use for our index then we
-    // leave it unset.
-    aValue.Unset();
-  }
+  nsresult rv = GetKeyFromValue(aCx, clone, aKeyPath, aValue);
+  NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
@@ -566,23 +620,14 @@ IDBObjectStore::GetIndexUpdateInfo(ObjectStoreInfo* aObjectStoreInfo,
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
-    cloneObj = JSVAL_TO_OBJECT(aObject);
-
     for (PRUint32 indexesIndex = 0; indexesIndex < count; indexesIndex++) {
       const IndexInfo& indexInfo = aObjectStoreInfo->indexes[indexesIndex];
 
-      const jschar* keyPathChars =
-        reinterpret_cast<const jschar*>(indexInfo.keyPath.BeginReading());
-      const size_t keyPathLen = indexInfo.keyPath.Length();
-
-      jsval keyPathValue;
-      JSBool ok = JS_GetUCProperty(aCx, cloneObj, keyPathChars, keyPathLen,
-                                   &keyPathValue);
-      NS_ENSURE_TRUE(ok, NS_ERROR_FAILURE);
-
       Key value;
-      nsresult rv = value.SetFromJSVal(aCx, keyPathValue);
-      if (NS_FAILED(rv) || value.IsUnset()) {
+      nsresult rv = GetKeyFromValue(aCx, aObject, indexInfo.keyPath, value);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      if (value.IsUnset()) {
         // Not a value we can do anything with, ignore it.
         continue;
       }
@@ -850,7 +895,7 @@ IDBObjectStore::GetAddInfo(JSContext* aCx,
       return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
     }
 
-    rv = GetKeyFromObject(aCx, JSVAL_TO_OBJECT(aValue), mKeyPath, aKey);
+    rv = GetKeyFromValue(aCx, aValue, mKeyPath, aKey);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -1269,6 +1314,10 @@ IDBObjectStore::CreateIndex(const nsAString& aName,
     return NS_ERROR_DOM_INDEXEDDB_NON_TRANSIENT_ERR;
   }
 
+  if (!IsValidKeyPath(aCx, aKeyPath)) {
+    return NS_ERROR_DOM_SYNTAX_ERR;
+  }
+
   IDBTransaction* transaction = AsyncConnectionHelper::GetCurrentTransaction();
 
   if (!transaction ||

dom/indexedDB/IDBObjectStore.h

@@ -72,6 +72,9 @@ public:
   Create(IDBTransaction* aTransaction,
          const ObjectStoreInfo* aInfo);
 
+  static bool
+  IsValidKeyPath(JSContext* aCx, const nsAString& aKeyPath);
+
   static nsresult
   GetKeyPathValueFromStructuredData(const PRUint8* aData,
                                     PRUint32 aDataLength,

dom/indexedDB/test/Makefile.in

@@ -59,6 +59,7 @@ TEST_FILES = \
   test_bfcache.html \
   test_clear.html \
   test_cmp.html \
+  test_complex_keyPaths.html \
   test_count.html \
   test_create_index.html \
   test_create_index_with_integer_keys.html \

dom/indexedDB/test/test_complex_keyPaths.html

@@ -0,0 +1,175 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <title>Indexed Database Property Test</title>
+
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+
+  <script type="text/javascript;version=1.7">
+    function testSteps()
+    {
+      const nsIIDBObjectStore = Components.interfaces.nsIIDBObjectStore;
+      const nsIIDBTransaction = Components.interfaces.nsIIDBTransaction;
+
+      // Test object stores
+
+      const name = window.location.pathname;
+      const objectStoreInfo = [
+        { name: "a", options: { keyPath: "id"} },
+        { name: "b", options: { keyPath: "foo.id"} },
+        { name: "c", options: { keyPath: ""} },
+        { name: "d", options: { keyPath: "foo..id"}, exception: true },
+        { name: "e", options: { keyPath: "foo."}, exception: true },
+        { name: "f", options: { keyPath: "foo.bar" } },
+        { name: "g", options: { keyPath: "fo o" }, exception: true},
+        { name: "h", options: { keyPath: "foo " }, exception: true},
+        { name: "i", options: { keyPath: "foo[bar]" }, exception: true },
+        { name: "j", options: { keyPath: "$('id').stuff" }, exception: true },
+        { name: "k", options: { keyPath: "foo.2.bar" }, exception: true }
+      ];
+
+      const indexInfo = [
+        { name: "1", keyPath: "id" },
+        { name: "2", keyPath: "foo..id", exception: true },
+        { name: "3", keyPath: "foo.", exception: true },
+        { name: "4", keyPath: "foo.baz" },
+        { name: "5", keyPath: "fo o", exception: true },
+        { name: "6", keyPath: "foo ", exception: true },
+        { name: "7", keyPath: "foo[bar]", exception: true },
+        { name: "8", keyPath: "$('id').stuff", exception: true },
+        { name: "9", keyPath: "foo.2.bar", exception: true },
+        { name: "10", keyPath: "foo.bork" },
+      ];
+
+      let request = mozIndexedDB.open(name, 1);
+      request.onerror = errorHandler;
+      request.onupgradeneeded = grabEventAndContinueHandler;
+      request.onsuccess = unexpectedSuccessHandler;
+      let event = yield;
+      let db = event.target.result;
+
+      for (let i = 0; i < objectStoreInfo.length; i++) {
+        let info = objectStoreInfo[i];
+        try {
+          let objectStore = info.hasOwnProperty("options") ?
+                            db.createObjectStore(info.name, info.options) :
+                            db.createObjectStore(info.name);
+          ok(!info.hasOwnProperty("exception"), "expected exception behavior observed");
+        } catch (e) {
+          ok(info.hasOwnProperty("exception"), "expected exception behavior observed");
+          ok(e instanceof DOMException, "Got a DOM Exception");
+          is(e.code, DOMException.SYNTAX_ERR, "expect a syntax error");
+        }
+      }
+
+      request.onsuccess = grabEventAndContinueHandler;
+      yield;
+
+      let trans = db.transaction(["f"], IDBTransaction.READ_WRITE);
+      let objectStore = trans.objectStore("f");
+
+      objectStore.put({foo: {baz: -1, bar: 72, bork: true}});
+      objectStore.put({foo: {baz: 2, bar: 1, bork: false}});
+
+      try {
+        objectStore.put({foo: {}});
+        ok(false, "Should have thrown!");
+      } catch (e) {
+        ok(true, "Putting an object without the key should throw");
+      }
+
+      trans.onerror = errorHandler;
+      trans.oncomplete = grabEventAndContinueHandler;
+
+      yield;
+
+      let trans = db.transaction(["f"], IDBTransaction.READ);
+      let objectStore = trans.objectStore("f");
+      let request = objectStore.openCursor();
+
+      request.onerror = errorHandler;
+      request.onsuccess = grabEventAndContinueHandler;
+
+      let event = yield;
+
+      let cursor = event.target.result;
+      is(cursor.value.foo.baz, 2, "got things in the right order");
+
+      cursor.continue();
+
+      let event = yield
+
+      let cursor = event.target.result;
+      is(cursor.value.foo.baz, -1, "got things in the right order");
+
+      db.close();
+
+      // Test indexes
+
+      let request = mozIndexedDB.open(name, 2);
+      request.onerror = errorHandler;
+      request.onupgradeneeded = grabEventAndContinueHandler;
+
+      let event = yield;
+      let db = event.target.result;
+
+      let trans = event.target.transaction;
+      let objectStore = trans.objectStore("f");
+
+      let indexes = [];
+      for (let i = 0; i < indexInfo.length; i++) {
+        let info = indexInfo[i];
+        try {
+          indexes[i] = info.hasOwnProperty("options") ?
+                       objectStore.createIndex(info.name, info.keyPath, info.options) :
+                       objectStore.createIndex(info.name, info.keyPath);
+          ok(!info.hasOwnProperty("exception"), "expected exception behavior observed");
+        } catch (e) {
+          ok(info.hasOwnProperty("exception"), "expected exception behavior observed");
+          ok(e instanceof DOMException, "Got a DOM Exception");
+          is(e.code, DOMException.SYNTAX_ERR, "expect a syntax error");
+        }
+      }
+      request.onsuccess = grabEventAndContinueHandler;
+
+      yield;
+
+      let trans = db.transaction(["f"], IDBTransaction.READ);
+      let objectStore = trans.objectStore("f");
+
+      let request = objectStore.index("4").openCursor();
+      request.onsuccess = grabEventAndContinueHandler;
+
+      let event = yield;
+
+      let cursor = event.target.result;
+
+      is(cursor.value.foo.bar, 72, "got things in the right order");
+
+      cursor.continue();
+      yield;
+
+      is(cursor.value.foo.bar, 1, "got things in the right order");
+
+      let request = objectStore.index("10").openCursor();
+      request.onerror = errorHandler;
+      request.onsuccess = grabEventAndContinueHandler;
+
+      let event = yield;
+
+      is(event.target.result, null, "should have no results");
+
+      finishTest();
+      yield;
+    }
+  </script>
+  <script type="text/javascript;version=1.7" src="helpers.js"></script>
+</head>
+
+<body onload="runTest();"></body>
+
+</html>

js/src/jsapi.cpp

@@ -6484,6 +6484,19 @@ JS_IndexToId(JSContext *cx, uint32 index, jsid *id)
     return IndexToId(cx, index, id);
 }
 
+JS_PUBLIC_API(JSBool)
+JS_IsIdentifier(JSContext *cx, JSString *str, JSBool *isIdentifier)
+{
+    assertSameCompartment(cx, str);
+
+    JSLinearString* linearStr = str->ensureLinear(cx);
+    if (!linearStr)
+        return false;
+
+    *isIdentifier = js::IsIdentifier(linearStr);
+    return true;
+}
+
 #ifdef JS_THREADSAFE
 static PRStatus
 CallOnce(void *func)

js/src/jsapi.h

@@ -5020,6 +5020,12 @@ JS_ScheduleGC(JSContext *cx, uint32 count, JSBool compartment);
 extern JS_PUBLIC_API(JSBool)
 JS_IndexToId(JSContext *cx, uint32 index, jsid *id);
 
+/*
+ *  Test if the given string is a valid ECMAScript identifier
+ */
+extern JS_PUBLIC_API(JSBool)
+JS_IsIdentifier(JSContext *cx, JSString *str, JSBool *isIdentifier);
+
 JS_END_EXTERN_C
 
 #endif /* jsapi_h___ */