47661 2010-10-14 04:11:43 -0700 Invalid base64 data in orphaned-selection-crash-bug32823-2.html 2022-07-31 12:15:52 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) PC OS X 10.5 RESOLVED LATER P2 Normal --- 0 svillar webkit-unassigned ahmad.saleem792 ap bfulgham darin mrobinson rniwa oldest_to_newest 294041 0 svillar 2010-10-14 04:11:43 -0700 That test includes this base64 encoded data iVBORw0KGrkJggg== which has 1 extra padding '=' 294043 1 70724 svillar 2010-10-14 04:18:32 -0700 Created attachment 70724 Fix for the bug Removes an invalid extra "=" padding character in the data URL 294067 2 svillar 2010-10-14 05:35:26 -0700 Adding Ryosuke 294124 3 svillar 2010-10-14 08:37:49 -0700 Adding Darin also as he reviewed the patch 294134 4 mrobinson 2010-10-14 08:43:29 -0700 Is it possible to instead make the libsoup Data URI implementation resilient against extra data in the URL? 294141 5 mrobinson 2010-10-14 08:55:03 -0700 Looks like you've done that here: https://bugs.webkit.org/show_bug.cgi?id=47666 I suppose we should wait to see if that change fixes this failure. 294145 6 svillar 2010-10-14 09:00:27 -0700 (In reply to comment #4) > Is it possible to instead make the libsoup Data URI implementation resilient against extra data in the URL? Indeed it's possible, but if the test is wrong I guess we should fix that IMHO 294152 7 rniwa 2010-10-14 09:37:21 -0700 Are you sure the original crash reproduces with the extra =? It's quite possible that missing = was intentional because it's a crash test. The test just needs to produce a crash, it doesn't need to be 'correct'. 294175 8 svillar 2010-10-14 10:20:48 -0700 (In reply to comment #7) > Are you sure the original crash reproduces with the extra =? It's quite possible that missing = was intentional because it's a crash test. The test just needs to produce a crash, it doesn't need to be 'correct'. Thx for the answer Ryosuke. I don't know the code that was causing the crash in detail but it seems that the extra "=" does not have to do with the original problem. Just pinging you to know if that was intentional or not, but I guess I can double-check that. 294179 9 rniwa 2010-10-14 10:25:41 -0700 (In reply to comment #8) > Thx for the answer Ryosuke. I don't know the code that was causing the crash in detail but it seems that the extra "=" does not have to do with the original problem. Just pinging you to know if that was intentional or not, but I guess I can double-check that. Could you download the code BEFORE r66032 (http://trac.webkit.org/changeset/66032) and make sure WebKit crashes with your fix? 294289 10 ap 2010-10-14 12:38:59 -0700 Note that Safari (or more precisely, CFNetwork) is strict in this respect - "data:image/png;base64,iVBORw0KGrkJggg==" fails to load, while "data:image/png;base64,iVBORw0KGrkJggg=" loads fine (but then fails to render, since the data itself is of course broken). This matches Firefox. So, I suspect that both this change and the change in bug 47666 are wrong. There is probably some difference in how failed data: load is reported by libsoup, which was making the layout test fail. 294299 11 rniwa 2010-10-14 12:54:08 -0700 (In reply to comment #10) > Note that Safari (or more precisely, CFNetwork) is strict in this respect - "data:image/png;base64,iVBORw0KGrkJggg==" fails to load, while "data:image/png;base64,iVBORw0KGrkJggg=" loads fine (but then fails to render, since the data itself is of course broken). This matches Firefox. > > So, I suspect that both this change and the change in bug 47666 are wrong. There is probably some difference in how failed data: load is reported by libsoup, which was making the layout test fail. Yeah, only if they cc-ed me in the bug 47666, I could have stopped them from making that change :( Or that I should have been more careful after I checked in the test. Nonetheless, we should verify that this change won't "fix" the crash. 294619 12 svillar 2010-10-15 00:50:52 -0700 (In reply to comment #11) > (In reply to comment #10) > > Note that Safari (or more precisely, CFNetwork) is strict in this respect - "data:image/png;base64,iVBORw0KGrkJggg==" fails to load, while "data:image/png;base64,iVBORw0KGrkJggg=" loads fine (but then fails to render, since the data itself is of course broken). This matches Firefox. > > > > So, I suspect that both this change and the change in bug 47666 are wrong. There is probably some difference in how failed data: load is reported by libsoup, which was making the layout test fail. > > Yeah, only if they cc-ed me in the bug 47666, I could have stopped them from making that change :( Or that I should have been more careful after I checked in the test. Nonetheless, we should verify that this change won't "fix" the crash. Thx Ryosuke and Alexey for your comments. I'd most likely reopen 47666 in order to fix it properly. I just would like to know if there is any spec that mentions how to deal with invalid base64 data. I guess that the right behaviour is to fail the load as Safari does. 1887711 13 ahmad.saleem792 2022-07-29 11:20:41 -0700 I think this was workaround by following commit - https://github.com/WebKit/WebKit/commit/6a237ea157f017c19e1e12273af6bb4c1823050a Do we need this fix now? Thanks! 1887968 14 rniwa 2022-07-31 12:15:52 -0700 Looks like this issue sorted itself out. 70724 2010-10-14 04:18:32 -0700 2010-10-14 12:39:16 -0700 Fix for the bug 0001-Fix-for-the-bug.patch text/plain 1878 svillar RnJvbSBhZDA3NGI2NzYxMjY3NzQ5NGFiMmY3ZTZlYmU0NDgxZjRmODNjZTgwIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBTZXJnaW8gVmlsbGFyIFNlbmluIDxzdmlsbGFyQGlnYWxpYS5j b20+CkRhdGU6IFRodSwgMTQgT2N0IDIwMTAgMTM6MTY6NTUgKzAyMDAKU3ViamVjdDogW1BBVENI XSBGaXggZm9yIHRoZSBidWcKCi0tLQogTGF5b3V0VGVzdHMvQ2hhbmdlTG9nICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgfCAgIDExICsrKysrKysrKysrCiAuLi4vdW5kby9vcnBoYW5lZC1z ZWxlY3Rpb24tY3Jhc2gtYnVnMzI4MjMtMi5odG1sICB8ICAgIDIgKy0KIDIgZmlsZXMgY2hhbmdl ZCwgMTIgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9MYXlvdXRU ZXN0cy9DaGFuZ2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggMzI0NzY1OC4uMzYw Mjc5MCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDEwLTEwLTE0ICBTZXJnaW8gVmlsbGFyIFNl bmluICA8c3ZpbGxhckBpZ2FsaWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIEludmFsaWQgYmFzZTY0IGRhdGEgaW4gb3JwaGFuZWQtc2VsZWN0 aW9uLWNyYXNoLWJ1ZzMyODIzLTIuaHRtbAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9NDc2NjEKKworICAgICAgICBSZW1vdmVkIGFuIGludmFsaWQgZXh0 cmEgcGFkZGluZyBjaGFyYWN0ZXIgIj0iIGluIGEgZGF0YSBVUkwKKworICAgICAgICAqIGVkaXRp bmcvdW5kby9vcnBoYW5lZC1zZWxlY3Rpb24tY3Jhc2gtYnVnMzI4MjMtMi5odG1sOgorCiAyMDEw LTEwLTEzICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3 ZWQgYnkgRGFyaW4gQWRsZXIuCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9lZGl0aW5nL3VuZG8v b3JwaGFuZWQtc2VsZWN0aW9uLWNyYXNoLWJ1ZzMyODIzLTIuaHRtbCBiL0xheW91dFRlc3RzL2Vk aXRpbmcvdW5kby9vcnBoYW5lZC1zZWxlY3Rpb24tY3Jhc2gtYnVnMzI4MjMtMi5odG1sCmluZGV4 IDY5MDc5NWYuLjkzNTAzZjEgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL2VkaXRpbmcvdW5kby9v cnBoYW5lZC1zZWxlY3Rpb24tY3Jhc2gtYnVnMzI4MjMtMi5odG1sCisrKyBiL0xheW91dFRlc3Rz L2VkaXRpbmcvdW5kby9vcnBoYW5lZC1zZWxlY3Rpb24tY3Jhc2gtYnVnMzI4MjMtMi5odG1sCkBA IC01LDcgKzUsNyBAQCBpZiAod2luZG93LmxheW91dFRlc3RDb250cm9sbGVyKQogZG9jdW1lbnQu ZXhlY0NvbW1hbmQoIk91dGRlbnQiLCBmYWxzZSwgMjYyMTQwKTsNCiBkb2N1bWVudC5kZXNpZ25N b2RlID0gIm9uIjsNCiBkb2N1bWVudC5leGVjQ29tbWFuZCgiU2VsZWN0QWxsIiwgZmFsc2UsICJo dHRwczovL3d3dy5leGFtcGxlLmNvbSIpOw0KLWRvY3VtZW50LmV4ZWNDb21tYW5kKCJpbnNlcnRp bWFnZSIsIGZhbHNlLCAiZGF0YTppbWFnZS9wbmc7YmFzZTY0LGlWQk9SdzBLR3JrSmdnZz09Iik7 DQorZG9jdW1lbnQuZXhlY0NvbW1hbmQoImluc2VydGltYWdlIiwgZmFsc2UsICJkYXRhOmltYWdl L3BuZztiYXNlNjQsaVZCT1J3MEtHcmtKZ2dnPSIpOw0KIGRvY3VtZW50LmV4ZWNDb21tYW5kKCJz ZWxlY3RhbGwiLCBmYWxzZSwgTmFOKTsNCiBkb2N1bWVudC5leGVjQ29tbWFuZCgic3RyaWtldGhy b3VnaCIsIGZhbHNlLCAiZ29waGVyOi8vd3d3LmV4YW1wbGUuY29tIik7DQogZG9jdW1lbnQuZXhl Y0NvbW1hbmQoIk1vdmVXb3JkRm9yd2FyZCIsIGZhbHNlLCA0MTAwKTsNCi0tIAoxLjcuMQoK