Bug 86122 - IndexedDB: Index key paths that yield invalid keys should not fail an add/put
Summary: IndexedDB: Index key paths that yield invalid keys should not fail an add/put
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joshua Bell
URL:
Keywords:
Depends on:
Blocks: 86123
  Show dependency treegraph
 
Reported: 2012-05-10 11:49 PDT by Joshua Bell
Modified: 2012-05-21 12:27 PDT (History)
4 users (show)

See Also:


Attachments
Patch (18.65 KB, patch)
2012-05-18 16:16 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch for landing (18.66 KB, patch)
2012-05-21 11:32 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua Bell 2012-05-10 11:49:51 PDT
WebKit's IDB implementation enforces an older spec restriction, namely that during the call to IDBOBjectStore.add/put(), if evaluating the key path of any index yields an invalid key the add/put should - instead, they should be ignored.

The spec was updated to drop this restriction (indexes key paths are not checked at all during the add/put call). The steps for storing a record now say:

http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#dfn-steps-for-storing-a-record-into-an-object-store

7.2 Evaluate index's key path on value. If this does not yield a value, take no further actions for this index. Otherwise set the result to index key.
7.3 If index's multiEntry flag is false or if index key is not an Array, and if index key is not a valid key, take no further actions for this index.
7.4 If index's multiEntry flag is true, and index key is an Array, remove any elements from index key that are not valid keys and remove any duplicate elements from index key such that only one instance of the duplicate value remains.
Comment 1 Joshua Bell 2012-05-18 16:16:09 PDT
Created attachment 142814 [details]
Patch
Comment 2 Joshua Bell 2012-05-18 16:18:29 PDT
tony@ - r?
Comment 3 Tony Chang 2012-05-18 16:25:58 PDT
Comment on attachment 142814 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142814&action=review

> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:289
> +            indexKeys.append(PassRefPtr<IDBKey>());

Nit: Should this be using a IDBKey::create function?  Creating a PassRefPtr explicitly feels weird.
Comment 4 Joshua Bell 2012-05-18 16:28:37 PDT
Comment on attachment 142814 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142814&action=review

>> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:289
>> +            indexKeys.append(PassRefPtr<IDBKey>());
> 
> Nit: Should this be using a IDBKey::create function?  Creating a PassRefPtr explicitly feels weird.

So does calling release() on a known-empty RefPtr (the previous code). I'll see if I can come up with something cleaner without a big restructure.
Comment 5 Joshua Bell 2012-05-21 11:32:24 PDT
Created attachment 143065 [details]
Patch for landing
Comment 6 WebKit Review Bot 2012-05-21 12:27:19 PDT
Comment on attachment 143065 [details]
Patch for landing

Clearing flags on attachment: 143065

Committed r117808: <http://trac.webkit.org/changeset/117808>
Comment 7 WebKit Review Bot 2012-05-21 12:27:23 PDT
All reviewed patches have been landed.  Closing bug.