116223 2013-05-16 07:40:55 -0700 Remove the incorrectly implemented and unused SQLiteStatement::getColumnBlob(). 2022-03-14 16:36:02 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) Unspecified Unspecified RESOLVED DUPLICATE 127868 P2 Normal --- 1 liachen liachen beidson darin joenotcharles oldest_to_newest 889815 0 liachen 2013-05-16 07:40:55 -0700 In SQLiteStatement::getColumnBlob(), it calls finalize(), prepare(), and step() unconditionally, which makes user can never get blob data out through this method. It should be changed to match method like getColumnBlobAsString() and getColumnBlobAsVector(), which assumes when m_statement is there then prepare() and step() have been called before this being called. And this will match what bindBlob() does, saving any binary data with defined length. 889817 1 liachen 2013-05-16 07:43:22 -0700 cc: Joe Mason, Darin, Beidson. 889825 2 201957 liachen 2013-05-16 07:55:45 -0700 Created attachment 201957 Patch for 116223. 889838 3 201957 joenotcharles 2013-05-16 08:31:18 -0700 Comment on attachment 201957 Patch for 116223. View in context: https://bugs.webkit.org/attachment.cgi?id=201957&action=review > Source/WebCore/ChangeLog:15 > + No new tests needed as no one use SQLiteStatement::getColumnBlob() yet. Still needs a test, to make sure it's actually working when someone wants to use it. 889841 4 liachen 2013-05-16 08:39:02 -0700 (In reply to comment #3) > (From update of attachment 201957 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=201957&action=review > > > Source/WebCore/ChangeLog:15 > > + No new tests needed as no one use SQLiteStatement::getColumnBlob() yet. > > Still needs a test, to make sure it's actually working when someone wants to use it. So it will be a unit test, instead of layout test? 889867 5 ap 2013-05-16 09:42:38 -0700 I'd say that if SQLiteStatement::getColumnBlob() is never called, we should strongly consider removing it completely. 889868 6 liachen 2013-05-16 09:43:51 -0700 (In reply to comment #5) > I'd say that if SQLiteStatement::getColumnBlob() is never called, we should strongly consider removing it completely. But I am planning to use it, and then found the issue. 889898 7 201969 liachen 2013-05-16 10:44:28 -0700 Created attachment 201969 Remove it instead of fixing it. 889934 8 joenotcharles 2013-05-16 11:52:44 -0700 (In reply to comment #7) > Created an attachment (id=201969) [details] > Remove it instead of fixing it. Are you planning to re-add it along with our platform specific patch once we're finished the code that uses it? 889935 9 joenotcharles 2013-05-16 11:53:37 -0700 (In reply to comment #8) > (In reply to comment #7) > > Created an attachment (id=201969) [details] [details] > > Remove it instead of fixing it. > > Are you planning to re-add it along with our platform specific patch once we're finished the code that uses it? Never mind, I actually looked at the patch and I see your comment now. :) 889943 10 liachen 2013-05-16 12:01:38 -0700 (In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > Created an attachment (id=201969) [details] [details] [details] > > > Remove it instead of fixing it. > > > > Are you planning to re-add it along with our platform specific patch once we're finished the code that uses it? > > Never mind, I actually looked at the patch and I see your comment now. :) Yeah, I thought getColumnBlob name more correct than getColumnBlobAsVector, and can avoid unnecessary full copy. But then I think maybe this is not that terrible. At least IconDatabase::getImageDataForIconURLFromSQLDatabase() is doing it this way. (I think it should use SharedBuffer::adoptVector() instead of SharedBuffer::create(), but it is another story), and our password is short. 890033 11 201969 benjamin 2013-05-16 16:27:54 -0700 Comment on attachment 201969 Remove it instead of fixing it. View in context: https://bugs.webkit.org/attachment.cgi?id=201969&action=review > Source/WebCore/ChangeLog:12 > + No new tests needed. Whaaaaat? ;) 890035 12 201969 benjamin 2013-05-16 16:30:00 -0700 Comment on attachment 201969 Remove it instead of fixing it. Let's cq- actually: -Please change the title to reflect what is in the patch. 890090 13 201969 liachen 2013-05-16 18:40:15 -0700 Comment on attachment 201969 Remove it instead of fixing it. Re-set the commit-queue flag after changing the bug title to reflect the patch. Ben: do you feel the changed title is OK now? 1851073 14 201969 ntim 2022-03-14 16:35:44 -0700 Comment on attachment 201969 Remove it instead of fixing it. Bug 127868 made the same change. 1851074 15 ntim 2022-03-14 16:36:02 -0700 *** This bug has been marked as a duplicate of bug 127868 *** 201957 2013-05-16 07:55:45 -0700 2013-05-16 10:44:28 -0700 Patch for 116223. 116223.patch text/plain 2006 liachen ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAyODYxMDNhLi5hNWFmY2FjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjIg QEAKKzIwMTMtMDUtMTYgIExpYW5naHVpIENoZW4gIDxsaWFjaGVuQHJpbS5jb20+CisKKyAgICAg ICAgU1FMaXRlU3RhdGVtZW50OjpnZXRDb2x1bW5CbG9iKCkgY2FsbHMgZmluYWxpemUoKSwgcHJl cGFyZSgpLCBhbmQgc3RlcCgpIHVubmVjZXNzYXJ5LgorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTE2MjIzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgUmlnaHQgbm93IFNRTGl0ZVN0YXRlbWVudDo6Z2V0Q29s dW1uQmxvYigpIGNhbGxzIGZpbmFsaXplKCksIHByZXBhcmUoKSwgYW5kIHN0ZXAoKQorICAgICAg ICB1bmNvbmRpdGlvbmFsbHksIHdoaWNoIG1lYW5zIGl0IG1lYW5zIHRvIGZpbmFsaXplIHRoZSBs YXN0IHN0ZXAgYW5kIGV4ZWN1dGUgYSBuZXcKKyAgICAgICAgc3RlcCgpLiBUaGlzIG1ha2UgaXQg aW1wb3NzaWJsZSB0byB1c2UgaXQgdG8gcmVhZCBibG9iIGRhdGEgYXMgcHVyZSBiaW5hcnkgZGF0 YQorICAgICAgICBhbG9uZyB3aXRoIG90aGVyIGdldENvbHVtblhYWCgpIG1ldGhvZHMsIHNvIGl0 IHdpbGwgYmUgdXNlZnVsIG9ubHkgaWYgdGhlIGJsb2IgZGF0YQorICAgICAgICBpcyB0aGUgb25s eSBjb2x1bW4uIFRoaXMgcGF0Y2ggd2lsbCBtYWtlIGl0IHNpbWlsYXIgdG8gYWxsIG90aGVyIGdl dENvbHVtblhYWCgpCisgICAgICAgIG1ldGhvZCBhbmQgY2FuIGJlIHVzZWQgYWxvbmcgd2l0aCB0 aGVtIGFmdGVyIHRoZSBzYW1lIHN0ZXAoKSBleGVjdXRlZC4KKworICAgICAgICBObyBuZXcgdGVz dHMgbmVlZGVkIGFzIG5vIG9uZSB1c2UgU1FMaXRlU3RhdGVtZW50OjpnZXRDb2x1bW5CbG9iKCkg eWV0LgorCisgICAgICAgICogcGxhdGZvcm0vc3FsL1NRTGl0ZVN0YXRlbWVudC5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpTUUxpdGVTdGF0ZW1lbnQ6OmdldENvbHVtbkJsb2IpOgorCiAyMDEzLTA1 LTE1ICBBbmRlcnMgQ2FybHNzb24gIDxhbmRlcnNjYUBhcHBsZS5jb20+CiAKICAgICAgICAgRml4 IGEgdGhpbmtvLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vc3FsL1NRTGl0 ZVN0YXRlbWVudC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9zcWwvU1FMaXRlU3RhdGVt ZW50LmNwcAppbmRleCAzYzdlOTQ5Li4wOTM4NDg3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9wbGF0Zm9ybS9zcWwvU1FMaXRlU3RhdGVtZW50LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9zcWwvU1FMaXRlU3RhdGVtZW50LmNwcApAQCAtNDM4LDE3ICs0MzgsOSBAQCBjb25z dCB2b2lkKiBTUUxpdGVTdGF0ZW1lbnQ6OmdldENvbHVtbkJsb2IoaW50IGNvbCwgaW50JiBzaXpl KQogCiAgICAgc2l6ZSA9IDA7CiAKLSAgICBpZiAoZmluYWxpemUoKSAhPSBTUUxJVEVfT0spCi0g ICAgICAgIExPRyhTUUxEYXRhYmFzZSwgIkZpbmFsaXplIGZhaWxlZCIpOwotICAgIGlmIChwcmVw YXJlKCkgIT0gU1FMSVRFX09LKSB7Ci0gICAgICAgIExPRyhTUUxEYXRhYmFzZSwgIlByZXBhcmUg ZmFpbGVkIik7Ci0gICAgICAgIHJldHVybiAwOwotICAgIH0KLSAgICBpZiAoc3RlcCgpICE9IFNR TElURV9ST1cpIHsKLSAgICAgICAgTE9HKFNRTERhdGFiYXNlLCAiU3RlcCB3YXNuJ3QgYSByb3ci KTsKLSAgICAgICAgcmV0dXJuIDA7Ci0gICAgfQotCisgICAgaWYgKCFtX3N0YXRlbWVudCkKKyAg ICAgICAgaWYgKHByZXBhcmVBbmRTdGVwKCkgIT0gU1FMSVRFX1JPVykKKyAgICAgICAgICAgIHJl dHVybiAwOwogICAgIGlmIChjb2x1bW5Db3VudCgpIDw9IGNvbCkKICAgICAgICAgcmV0dXJuIDA7 CiAgICAgICAgIAo= 201969 2013-05-16 10:44:28 -0700 2022-03-14 16:35:44 -0700 Remove it instead of fixing it. 116223-2.patch text/plain 2554 liachen ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAyODYxMDNhLi44ZjhjNWJlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjAg QEAKKzIwMTMtMDUtMTYgIExpYW5naHVpIENoZW4gIDxsaWFjaGVuQHJpbS5jb20+CisKKyAgICAg ICAgU1FMaXRlU3RhdGVtZW50OjpnZXRDb2x1bW5CbG9iKCkgY2FsbHMgZmluYWxpemUoKSwgcHJl cGFyZSgpLCBhbmQgc3RlcCgpIHVubmVjZXNzYXJ5LgorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTE2MjIzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgSW5zdGVhZCBvZiBmaXhpbmcgU1FMaXRlU3RhdGVtZW50 OjpnZXRDb2x1bW5CbG9iKCksIHJlbW92ZSBpdCBhcyBhcCBzdWdnZXN0ZWQKKyAgICAgICAgaW4g aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExNjIyMyNjNSwgYXMgaXQg aXMgbm90IHVzZWQgYW55d2hlcmUKKyAgICAgICAgYW55d2F5LiBVc2UgZ2V0Q29sdW1uQmxvYkFz VmVjdG9yKCkgaW5zdGVhZC4KKworICAgICAgICBObyBuZXcgdGVzdHMgbmVlZGVkLgorCisgICAg ICAgICogcGxhdGZvcm0vc3FsL1NRTGl0ZVN0YXRlbWVudC5jcHA6CisgICAgICAgICogcGxhdGZv cm0vc3FsL1NRTGl0ZVN0YXRlbWVudC5oOgorICAgICAgICAoU1FMaXRlU3RhdGVtZW50KToKKwog MjAxMy0wNS0xNSAgQW5kZXJzIENhcmxzc29uICA8YW5kZXJzY2FAYXBwbGUuY29tPgogCiAgICAg ICAgIEZpeCBhIHRoaW5rby4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3Nx bC9TUUxpdGVTdGF0ZW1lbnQuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vc3FsL1NRTGl0 ZVN0YXRlbWVudC5jcHAKaW5kZXggM2M3ZTk0OS4uNjU0NGI1NCAxMDA2NDQKLS0tIGEvU291cmNl L1dlYkNvcmUvcGxhdGZvcm0vc3FsL1NRTGl0ZVN0YXRlbWVudC5jcHAKKysrIGIvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vc3FsL1NRTGl0ZVN0YXRlbWVudC5jcHAKQEAgLTQzMiwzNCArNDMyLDYg QEAgdm9pZCBTUUxpdGVTdGF0ZW1lbnQ6OmdldENvbHVtbkJsb2JBc1ZlY3RvcihpbnQgY29sLCBW ZWN0b3I8Y2hhcj4mIHJlc3VsdCkKICAgICAgICAgcmVzdWx0W2ldID0gKHN0YXRpY19jYXN0PGNv bnN0IHVuc2lnbmVkIGNoYXIqPihibG9iKSlbaV07CiB9CiAKLWNvbnN0IHZvaWQqIFNRTGl0ZVN0 YXRlbWVudDo6Z2V0Q29sdW1uQmxvYihpbnQgY29sLCBpbnQmIHNpemUpCi17Ci0gICAgQVNTRVJU KGNvbCA+PSAwKTsKLQotICAgIHNpemUgPSAwOwotCi0gICAgaWYgKGZpbmFsaXplKCkgIT0gU1FM SVRFX09LKQotICAgICAgICBMT0coU1FMRGF0YWJhc2UsICJGaW5hbGl6ZSBmYWlsZWQiKTsKLSAg ICBpZiAocHJlcGFyZSgpICE9IFNRTElURV9PSykgewotICAgICAgICBMT0coU1FMRGF0YWJhc2Us ICJQcmVwYXJlIGZhaWxlZCIpOwotICAgICAgICByZXR1cm4gMDsKLSAgICB9Ci0gICAgaWYgKHN0 ZXAoKSAhPSBTUUxJVEVfUk9XKSB7Ci0gICAgICAgIExPRyhTUUxEYXRhYmFzZSwgIlN0ZXAgd2Fz bid0IGEgcm93Iik7Ci0gICAgICAgIHJldHVybiAwOwotICAgIH0KLQotICAgIGlmIChjb2x1bW5D b3VudCgpIDw9IGNvbCkKLSAgICAgICAgcmV0dXJuIDA7Ci0gICAgICAgIAotICAgIGNvbnN0IHZv aWQqIGJsb2IgPSBzcWxpdGUzX2NvbHVtbl9ibG9iKG1fc3RhdGVtZW50LCBjb2wpOwotICAgIGlm ICghYmxvYikKLSAgICAgICAgcmV0dXJuIDA7Ci0KLSAgICBzaXplID0gc3FsaXRlM19jb2x1bW5f Ynl0ZXMobV9zdGF0ZW1lbnQsIGNvbCk7Ci0gICAgcmV0dXJuIGJsb2I7Ci19Ci0KIGJvb2wgU1FM aXRlU3RhdGVtZW50OjpyZXR1cm5UZXh0UmVzdWx0cyhpbnQgY29sLCBWZWN0b3I8U3RyaW5nPiYg dikKIHsKICAgICBBU1NFUlQoY29sID49IDApOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vc3FsL1NRTGl0ZVN0YXRlbWVudC5oIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v c3FsL1NRTGl0ZVN0YXRlbWVudC5oCmluZGV4IDhhOWU0YWMuLjBmZWI1N2MgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL3NxbC9TUUxpdGVTdGF0ZW1lbnQuaAorKysgYi9Tb3Vy Y2UvV2ViQ29yZS9wbGF0Zm9ybS9zcWwvU1FMaXRlU3RhdGVtZW50LmgKQEAgLTgxLDcgKzgxLDYg QEAgcHVibGljOgogICAgIGRvdWJsZSBnZXRDb2x1bW5Eb3VibGUoaW50IGNvbCk7CiAgICAgaW50 IGdldENvbHVtbkludChpbnQgY29sKTsKICAgICBpbnQ2NF90IGdldENvbHVtbkludDY0KGludCBj b2wpOwotICAgIGNvbnN0IHZvaWQqIGdldENvbHVtbkJsb2IoaW50IGNvbCwgaW50JiBzaXplKTsK ICAgICBTdHJpbmcgZ2V0Q29sdW1uQmxvYkFzU3RyaW5nKGludCBjb2wpOwogICAgIHZvaWQgZ2V0 Q29sdW1uQmxvYkFzVmVjdG9yKGludCBjb2wsIFZlY3RvcjxjaGFyPiYpOwogCg==