The following does not render on Safari (macOS, iOS), but does on Chrome and Firefox: <img src="data:image/png; base64, iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg=="/> The following renders on Safari: <img src="data:image/png;base64, iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg=="/> <img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg=="/> Codepen: https://codepen.io/JeffreyCA/pen/BaKMNPR
<rdar://problem/69659841>
Created attachment 409919 [details] Patch
Comment on attachment 409919 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409919&action=review > Source/WebCore/platform/network/DataURLDecoder.cpp:72 > + const char base64String[] = "base64"; This doesn’t need to be a named array constant any more. > Source/WebCore/platform/network/DataURLDecoder.cpp:83 > + size_t mediaTypeEnd = header.find(';', 0); No need for ", 0" here. > Source/WebCore/platform/network/DataURLDecoder.cpp:86 > mediaType = mediaType.stripWhiteSpace(); Seems likely this does the wrong thing; this function uses the Unicode definition of white space and also strips internal spaces. Is that really what we want? > Source/WebCore/platform/network/DataURLDecoder.cpp:95 > + isBase64 = equalIgnoringASCIICase(formatType, base64String); Can use equalLettersIgnoringASCIICase(formatType, "base64") here.
Created attachment 409935 [details] Patch
Comment on attachment 409919 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409919&action=review >> Source/WebCore/platform/network/DataURLDecoder.cpp:72 >> + const char base64String[] = "base64"; > > This doesn’t need to be a named array constant any more. Variable was removed. >> Source/WebCore/platform/network/DataURLDecoder.cpp:83 >> + size_t mediaTypeEnd = header.find(';', 0); > > No need for ", 0" here. ", 0" was removed but I had to use reverseFind() because we should look for the last semicolon in the header to find the end of the media type. >> Source/WebCore/platform/network/DataURLDecoder.cpp:86 >> mediaType = mediaType.stripWhiteSpace(); > > Seems likely this does the wrong thing; this function uses the Unicode definition of white space and also strips internal spaces. Is that really what we want? I think all we need to do is to remove the leading and the trailing whitespaces. My understanding is stripWhiteSpace() does that. Please advice if my understanding is incorrect. >> Source/WebCore/platform/network/DataURLDecoder.cpp:95 >> + isBase64 = equalIgnoringASCIICase(formatType, base64String); > > Can use equalLettersIgnoringASCIICase(formatType, "base64") here. Done.
Comment on attachment 409919 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409919&action=review >>> Source/WebCore/platform/network/DataURLDecoder.cpp:86 >>> mediaType = mediaType.stripWhiteSpace(); >> >> Seems likely this does the wrong thing; this function uses the Unicode definition of white space and also strips internal spaces. Is that really what we want? > > I think all we need to do is to remove the leading and the trailing whitespaces. My understanding is stripWhiteSpace() does that. Please advice if my understanding is incorrect. It does that, but uses the Unicode definition of whitespace, which includes characters we don’t want to strip. I think we want one of these: stripLeadingAndTrailingHTMLSpaces stripLeadingAndTrailingHTTPSpaces I was wrong to say it "strips internal spaces". I was thinking of "simplifyWhiteSpace". But I was right to highlight that this unnecessarily involves the Unicode definition of whitespace.
Comment on attachment 409935 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409935&action=review > Source/WebCore/platform/network/DataURLDecoder.cpp:94 > + formatType = formatType.stripWhiteSpace(); To show this is wrong, we need tests that involve non-ASCII white space characters such as U+2000, U+0085, or U+00A0. They should not get stripped, but they will get stripped.
Created attachment 409960 [details] Patch
Comment on attachment 409919 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409919&action=review >>>> Source/WebCore/platform/network/DataURLDecoder.cpp:86 >>>> mediaType = mediaType.stripWhiteSpace(); >>> >>> Seems likely this does the wrong thing; this function uses the Unicode definition of white space and also strips internal spaces. Is that really what we want? >> >> I think all we need to do is to remove the leading and the trailing whitespaces. My understanding is stripWhiteSpace() does that. Please advice if my understanding is incorrect. > > It does that, but uses the Unicode definition of whitespace, which includes characters we don’t want to strip. I think we want one of these: > > stripLeadingAndTrailingHTMLSpaces > stripLeadingAndTrailingHTTPSpaces > > I was wrong to say it "strips internal spaces". I was thinking of "simplifyWhiteSpace". But I was right to highlight that this unnecessarily involves the Unicode definition of whitespace. I used stripLeadingAndTrailingHTTPSpaces instead of stripWhiteSpace() because I found it is used in ParsedContentType::create().
Committed r267730: <https://trac.webkit.org/changeset/267730> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409960 [details].