That test includes this base64 encoded data iVBORw0KGrkJggg== which has 1 extra padding '='
Created attachment 70724 [details] Fix for the bug Removes an invalid extra "=" padding character in the data URL
Adding Ryosuke
Adding Darin also as he reviewed the patch
Is it possible to instead make the libsoup Data URI implementation resilient against extra data in the URL?
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.
(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
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'.
(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.
(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?
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.
(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.
(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.
I think this was workaround by following commit - https://github.com/WebKit/WebKit/commit/6a237ea157f017c19e1e12273af6bb4c1823050a Do we need this fix now? Thanks!
Looks like this issue sorted itself out.