Bug 58365

Summary: IndexedDB createIndex should fail if name arg is null
Product: WebKit Reporter: Mark Pilgrim (Google) <pilgrim>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dgrogan, fishd, hans, pilgrim, tony
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
test case
none
Patch
none
Throw NON_TRANSIENT_ERR instead
none
Patch for landing
none
Patch for landing
none
Patch for landing none

Mark Pilgrim (Google)
Reported 2011-04-12 12:46:01 PDT
Created attachment 89248 [details] test case According to http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBObjectStore-createIndex , the name argument of createIndex must not be null, and objectStore.createIndex(null, null) must throw a CONSTRAINT_ERR. WebKit does not throw an exception. Layout test attached.
Attachments
test case (1.71 KB, text/html)
2011-04-12 12:46 PDT, Mark Pilgrim (Google) 		no flags
Details
Patch (6.72 KB, patch)
2011-04-27 18:52 PDT, Mark Pilgrim (Google) 		no flags
DetailsFormatted DiffDiff
Throw NON_TRANSIENT_ERR instead (6.73 KB, patch)
2011-04-27 20:16 PDT, Mark Pilgrim (Google) 		no flags
DetailsFormatted DiffDiff
Patch for landing (5.47 KB, patch)
2011-04-28 11:21 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch for landing (7.78 KB, patch)
2011-04-28 11:26 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Patch for landing (7.01 KB, patch)
2011-04-28 11:43 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Mark Pilgrim (Google)
Comment 1 2011-04-27 18:47:55 PDT
Root problem was in the IDL file, we need to ensure that a null DOMString is treated as null (otherwise it gets serialized as the 4-character string "null"). After that, a basic isNull() check works as expected in ::createIndex().
Mark Pilgrim (Google)
Comment 2 2011-04-27 18:52:23 PDT
Created attachment 91404 [details] Patch
David Grogan
Comment 3 2011-04-27 19:25:08 PDT
Comment on attachment 91404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91404&action=review r+ > Source/WebCore/storage/IDBObjectStore.idl:31 > + readonly attribute [ConvertNullStringTo=Null] DOMString name; I wonder why this was left off initially. It seems to be ok though.
Mark Pilgrim (Google)
Comment 4 2011-04-27 20:16:41 PDT
Created attachment 91415 [details] Throw NON_TRANSIENT_ERR instead Updated patch similar to bug 58465 -- Firefox throws NON_TRANSIENT_ERR, not CONSTRAINT_ERR, when name arg is null.
David Grogan
Comment 5 2011-04-27 20:18:43 PDT
Comment on attachment 91415 [details] Throw NON_TRANSIENT_ERR instead r+
WebKit Commit Bot
Comment 6 2011-04-28 10:02:52 PDT
Comment on attachment 91415 [details] Throw NON_TRANSIENT_ERR instead Rejecting attachment 91415 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 2 Last 500 characters of output: eate-index-null-name.html patch unexpectedly ends in middle of line patch: **** malformed patch at line 69: fatal: pathspec 'LayoutTests/storage/indexeddb/mozilla/create-index-null-name.html' did not match any files Failed to git add LayoutTests/storage/indexeddb/mozilla/create-index-null-name.html. at /mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply line 438. Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Tony Chang', u'--force']" exit_code: 2 Full output: http://queues.webkit.org/results/8519429
Mark Pilgrim (Google)
Comment 7 2011-04-28 11:01:27 PDT
What just happened?
Tony Chang
Comment 8 2011-04-28 11:14:06 PDT
Comment on attachment 91415 [details] Throw NON_TRANSIENT_ERR instead Looks like the patch failed to apply, although I'm not sure why (it says it's malformed, but it looks fine to me). Let's try again.
WebKit Commit Bot
Comment 9 2011-04-28 11:15:17 PDT
Comment on attachment 91415 [details] Throw NON_TRANSIENT_ERR instead Rejecting attachment 91415 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 2 Last 500 characters of output: eate-index-null-name.html patch unexpectedly ends in middle of line patch: **** malformed patch at line 69: fatal: pathspec 'LayoutTests/storage/indexeddb/mozilla/create-index-null-name.html' did not match any files Failed to git add LayoutTests/storage/indexeddb/mozilla/create-index-null-name.html. at /mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply line 438. Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Tony Chang', u'--force']" exit_code: 2 Full output: http://queues.webkit.org/results/8514702
Tony Chang
Comment 10 2011-04-28 11:21:37 PDT
Created attachment 91521 [details] Patch for landing
Tony Chang
Comment 11 2011-04-28 11:22:24 PDT
Turns out the diff was missing an newline at the end of the file. This confused patch and prevented the diff from being applied. I've uploaded a fixed diff for landing.
Tony Chang
Comment 12 2011-04-28 11:25:03 PDT
Comment on attachment 91521 [details] Patch for landing Crap, forgot to add the .html file.
Tony Chang
Comment 13 2011-04-28 11:26:26 PDT
Created attachment 91523 [details] Patch for landing
Mark Pilgrim (Google)
Comment 14 2011-04-28 11:27:47 PDT
No idea how that happened. Thanks, Tony.
Tony Chang
Comment 15 2011-04-28 11:28:02 PDT
Comment on attachment 91523 [details] Patch for landing hmm, the expected file is wrong.
Mark Pilgrim (Google)
Comment 16 2011-04-28 11:29:40 PDT
OK, let me start over. I'll upload a new patch ASAP.
Tony Chang
Comment 17 2011-04-28 11:32:42 PDT
(In reply to comment #16) > OK, let me start over. I'll upload a new patch ASAP. Sorry, I just uploaded an incorrect expected file (I ran patch twice so the new file was duplicated). If you have a fixed diff, feel free to upload.
Mark Pilgrim (Google)
Comment 18 2011-04-28 11:37:11 PDT
No, I have nothing yet. I was finishing up another bug's patch. If you'd like to fix it, please do.
Tony Chang
Comment 19 2011-04-28 11:43:24 PDT
Created attachment 91529 [details] Patch for landing
WebKit Commit Bot
Comment 20 2011-04-28 14:11:12 PDT
The commit-queue encountered the following flaky tests while processing attachment 91529 [details]: animations/suspend-resume-animation.html bug 48161 (authors: cmarrin@apple.com and simon.fraser@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 21 2011-04-28 14:12:56 PDT
Comment on attachment 91529 [details] Patch for landing Clearing flags on attachment: 91529 Committed r85238: <http://trac.webkit.org/changeset/85238>
WebKit Commit Bot
Comment 22 2011-04-28 14:13:02 PDT
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 23 2011-04-28 14:22:01 PDT
The commit-queue encountered the following flaky tests while processing attachment 91529 [details]: animations/suspend-resume-animation.html bug 48161 (authors: cmarrin@apple.com and simon.fraser@apple.com) The commit-queue is continuing to process your patch.
Note You need to log in before you can comment on or make changes to this bug.