23566 2009-01-26 23:48:22 -0800 base64Decode() patch 2023-05-22 04:08:56 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED INVALID P2 Normal --- 0 ppedriana ppedriana annevk skyul thomas youandmealwaysand4evermylove03 oldest_to_newest 107164 0 ppedriana 2009-01-26 23:48:22 -0800 The base64Decode function doesn't follow the Base64 convention regarding unrecognized encoding characters, as stated in RFC 2045: All line breaks or other characters not found in Table 1 must be ignored by decoding software. Similary, there is corresponding code comments in some WebKit code: // Use the GLib Base64 if available, since WebCore's decoder isn't // general-purpose and fails on Acid3 test 97 (whitespace). This patch fixes the base64Decode function in this respect. 107165 1 27068 ppedriana 2009-01-26 23:50:25 -0800 Created attachment 27068 base64Decode patch 107193 2 27068 darin 2009-01-27 09:08:43 -0800 Comment on attachment 27068 base64Decode patch Looks good! > +2009-01-26 Paul <set EMAIL_ADDRESS environment variable> This should have your full name and your email address. Remember that prepare-ChangeLog helps you write a change log, but doesn't write it for you. You may have to fix things. > + Fixed bug whereby base64Decode() was failing when unrecognized encoding chars You call this a bug in base64Decode, but it's actually the design of base64Decode. For example, this comment is in the header: // this decoder is not general purpose - it returns an error if it encounters a linefeed, as needed for window.atob Changing this behavior might be helpful for some callers and harmful for others. > + were present, whereas the base64 specification requires such chars be ignored. > + This allows some base64 tests, such as that in Acid3, to succeed where they previous failed. I don't understand this. I believe the Acid3 test already passes. Perhaps you're talking specifically about something in the curl or soup handling of data URLs? If so, please be specific. I believe your change may have improved data URLs for those protocols, but incompatibly changed the behavior of window.atob. Please supply some test cases to demonstrate the change in behavior of atob and check the behavior of other browsers, or submit a patch that does not change the behavior of atob. Also, please be more specific in your comments. I don't think "some base64 tests, such as that in Acid3" points clearly enough to the curl and soup implementations, if indeed that's what you're fixing. review- because either: 1) This fixes a bug in atob, but doesn't include a test case demonstrating that bug. or 2) This introduces a bug in atob, so shouldn't be checked in as-is. 107195 3 darin 2009-01-27 09:09:32 -0800 This bug needs a title that makes it clear what the problem is. Perhaps "data URL decoding in curl/soup does not skip illegal base64 characters as required by spec"? 107220 4 ppedriana 2009-01-27 11:08:27 -0800 I posted this as a followup to this email: https://lists.webkit.org/pipermail/webkit-dev/2008-December/006015.html The original bug is that base-64 data URL data decoding in some cases is incorrect, but it seems to me that the existing WebKit window.atob is also incorrect, now that you point it out. Acid3 data URL support might pass for CoreFoundation, but it won't pass for other back-ends present in the WebKit trunk. The window.atob function, by definition, decodes base-64 data (e.g. http://docs.sun.com/source/816-6408-10/window.htm#1201548), and so this patch would fix a bug in the existing window.atob implementation. However, as you point out, this patch should include a test case exercising the effect on window.atob. Perhaps I can augment it to do this and resubmit. I'm pretty sure the patch would be safe for window.atob, as it simply allows something to work that would previously fail (where it shouldn't). Does that seem reasonable, or should the issue of data URLs and window.atob be addressed independently for some reason? 1956913 5 annevk 2023-05-22 04:08:56 -0700 The code probably changed a fair bit here over the years, but we also don't want to follow the RFC directly as stated above. The code should be even clearer about this today and it has better test coverage too. 27068 2009-01-26 23:50:25 -0800 2010-06-10 16:33:13 -0700 base64Decode patch Base64.patch text/plain 1837 ppedriana SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDQwMjg4 KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMDktMDEt MjYgIFBhdWwgIDxzZXQgRU1BSUxfQUREUkVTUyBlbnZpcm9ubWVudCB2YXJpYWJsZT4KKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXQVJOSU5HOiBOTyBU RVNUIENBU0VTIEFEREVEIE9SIENIQU5HRUQKKworICAgICAgICAqIHBsYXRmb3JtL3RleHQvQmFz ZTY0LmNwcDoKKyAgICAgICAgICBGaXhlZCBidWcgd2hlcmVieSBiYXNlNjREZWNvZGUoKSB3YXMg ZmFpbGluZyB3aGVuIHVucmVjb2duaXplZCBlbmNvZGluZyBjaGFycworICAgICAgICAgIHdlcmUg cHJlc2VudCwgd2hlcmVhcyB0aGUgYmFzZTY0IHNwZWNpZmljYXRpb24gcmVxdWlyZXMgc3VjaCBj aGFycyBiZSBpZ25vcmVkLgorICAgICAgICAgIFRoaXMgYWxsb3dzIHNvbWUgYmFzZTY0IHRlc3Rz LCBzdWNoIGFzIHRoYXQgaW4gQWNpZDMsIHRvIHN1Y2NlZWQgd2hlcmUgdGhleSBwcmV2aW91cyBm YWlsZWQuCisKIDIwMDktMDEtMjYgIFNpbW9uIEZyYXNlciAgPHNpbW9uLmZyYXNlckBhcHBsZS5j b20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGF2ZSBIeWF0dApJbmRleDogcGxhdGZvcm0vdGV4 dC9CYXNlNjQuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHBsYXRmb3JtL3RleHQvQmFzZTY0LmNwcAkocmV2 aXNpb24gNDAyODQpCisrKyBwbGF0Zm9ybS90ZXh0L0Jhc2U2NC5jcHAJKHdvcmtpbmcgY29weSkK QEAgLTE0NCwxNyArMTQ0LDE4IEBAIGJvb2wgYmFzZTY0RGVjb2RlKGNvbnN0IGNoYXIqIGRhdGEs IHVuc2kKICAgICAgICAgLS1sZW47CiAKICAgICBvdXQuZ3JvdyhsZW4pOworICAgIHVuc2lnbmVk IG91dFNpemUgPSAwOwogICAgIGZvciAodW5zaWduZWQgaWR4ID0gMDsgaWR4IDwgbGVuOyBpZHgr KykgewogICAgICAgICB1bnNpZ25lZCBjaGFyIGNoID0gZGF0YVtpZHhdOwotICAgICAgICBpZiAo KGNoID4gNDcgJiYgY2ggPCA1OCkgfHwgKGNoID4gNjQgJiYgY2ggPCA5MSkgfHwgKGNoID4gOTYg JiYgY2ggPCAxMjMpIHx8IGNoID09ICcrJyB8fCBjaCA9PSAnLycgfHwgY2ggPT0gJz0nKQotICAg ICAgICAgICAgb3V0W2lkeF0gPSBiYXNlNjREZWNNYXBbY2hdOwotICAgICAgICBlbHNlCi0gICAg ICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgICAgIGlmICgoY2ggPiA0NyAmJiBjaCA8IDU4KSB8 fCAoY2ggPiA2NCAmJiBjaCA8IDkxKSB8fCAoY2ggPiA5NiAmJiBjaCA8IDEyMykgfHwgY2ggPT0g JysnIHx8IGNoID09ICcvJyB8fCBjaCA9PSAnPScpIHsKKyAgICAgICAgICAgIG91dFtvdXRTaXpl XSA9IGJhc2U2NERlY01hcFtjaF07CisgICAgICAgICAgICBvdXRTaXplKys7CisgICAgICAgIH0K ICAgICB9CiAKICAgICAvLyA0LWJ5dGUgdG8gMy1ieXRlIGNvbnZlcnNpb24KLSAgICB1bnNpZ25l ZCBvdXRMZW4gPSBsZW4gLSAoKGxlbiArIDMpIC8gNCk7Ci0gICAgaWYgKCFvdXRMZW4gfHwgKChv dXRMZW4gKyAyKSAvIDMpICogNCA8IGxlbikKKyAgICB1bnNpZ25lZCBvdXRMZW4gPSBvdXRTaXpl IC0gKChvdXRTaXplICsgMykgLyA0KTsKKyAgICBpZiAoIW91dExlbiB8fCAoKG91dExlbiArIDIp IC8gMykgKiA0IDwgb3V0U2l6ZSkKICAgICAgICAgcmV0dXJuIGZhbHNlOwogCiAgICAgdW5zaWdu ZWQgc2lkeCA9IDA7Cg==