Bug 58613 - IndexedDB put() should fail if second (key) parameter is null
Summary: IndexedDB put() should fail if second (key) parameter is null
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Nobody
URL:
Keywords:
: 58611 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-04-14 17:32 PDT by Mark Pilgrim (Google)
Modified: 2011-04-28 19:00 PDT (History)
6 users (show)

See Also:


Attachments
test case (1.66 KB, text/html)
2011-04-14 17:32 PDT, Mark Pilgrim (Google)
no flags Details
Patch (7.69 KB, patch)
2011-04-28 04:06 PDT, Mark Pilgrim (Google)
tony: review-
tony: commit-queue-
Details | Formatted Diff | Diff
Patch with added changelog comments (8.18 KB, patch)
2011-04-28 10:58 PDT, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Pilgrim (Google) 2011-04-14 17:32:17 PDT
http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBObjectStore-put states that the second (key) parameter to put() is optional, but if specified it must not be null. Mozilla correctly throws in this case (test: http://mxr.mozilla.org/mozilla2.0/source/dom/indexedDB/test/test_key_requirements.html?force=1#204 ) but WebKit does not throw.
Comment 1 Mark Pilgrim (Google) 2011-04-14 17:32:34 PDT
Created attachment 89699 [details]
test case
Comment 2 Mark Pilgrim (Google) 2011-04-28 04:06:57 PDT
Created attachment 91466 [details]
Patch
Comment 3 Mark Pilgrim (Google) 2011-04-28 04:12:00 PDT
Straightforward fix -- if key is not specified, the prpKey ends up as a null pointer in ::put(). However, if the key is specified but is null, the prpKey ends up as a valid IDBKey which has a null key type. As it happens, we need to be able to detect the difference between these cases (the key arg is optional but if specified must not be null), and we can! Yay.

There was one existing layouttest that needed tweaking, plus the new layouttest ported from Mozilla's test suite. All other existing layouttests pass, including 5 that use put() without the second (key) arg. (I know it's exactly 5 because my first attempt crashed these 5 tests.)
Comment 4 Hans Wennborg 2011-04-28 04:18:34 PDT
(In reply to comment #3)
> Straightforward fix -- if key is not specified, the prpKey ends up as a null pointer in ::put(). However, if the key is specified but is null, the prpKey ends up as a valid IDBKey which has a null key type. As it happens, we need to be able to detect the difference between these cases (the key arg is optional but if specified must not be null), and we can! Yay.
> 
> There was one existing layouttest that needed tweaking, plus the new layouttest ported from Mozilla's test suite. All other existing layouttests pass, including 5 that use put() without the second (key) arg. (I know it's exactly 5 because my first attempt crashed these 5 tests.)

Thanks, Mark!

Unofficial r+.
Comment 5 Tony Chang 2011-04-28 10:18:19 PDT
Comment on attachment 91466 [details]
Patch

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

r- so you can upload a new version that I can cq+

> Source/WebCore/ChangeLog:7
> +

Can you put some of the first paragraph of comment #3 here?

> LayoutTests/ChangeLog:11
> +        * storage/indexeddb/objectstore-basics.html:

You could add a one line description of why this needed tweaking here.
Comment 6 Mark Pilgrim (Google) 2011-04-28 10:58:48 PDT
Created attachment 91517 [details]
Patch with added changelog comments
Comment 7 WebKit Commit Bot 2011-04-28 13:50:02 PDT
Comment on attachment 91517 [details]
Patch with added changelog comments

Clearing flags on attachment: 91517

Committed r85234: <http://trac.webkit.org/changeset/85234>
Comment 8 WebKit Commit Bot 2011-04-28 13:50:07 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Mark Pilgrim (Google) 2011-04-28 19:00:45 PDT
*** Bug 58611 has been marked as a duplicate of this bug. ***