Bug 118068

Summary: SVG data:uri images are not handled properly
Product: WebKit Reporter: Philip Rogers <pdr>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: benjamin, buildbot, cmarcelo, commit-queue, contact, dino, eflews.bot, esprehn+autocc, gtk-ews, gyuyoung.kim, japhet, kangil.han, krit, rego+ews, rniwa, syoichi, thorton, webkit-bug-importer, xan.lopez, youennf, zimmermann, zourtney
Priority: P2 Keywords: BlinkMergeCandidate, InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
none
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
none
Patch
none
Patch
none
Patch
none
xcode proj fixed
none
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion
none
updated mac baselines none

Description Philip Rogers 2013-06-26 10:14:31 PDT
At the moment SVG data uri images (bitmap or vector) are not allowed in SVG images. This is a huge bummer and prevents WebKit from showing many Adobe Illustrator SVG files where Illustrator filters are flattened into images.

For example:
<svg width="100" height="100" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
<!-- base64 encoded image: <svg width='100' height='100' xmlns='http://www.w3.org/2000/svg'><rect fill='green' width='100' height='100'></rect></svg> -->
<image width="100" height="100" xlink:href="data:image/svg+xml;base64,PHN2ZyB3aWR0aD0nMTAwJyBoZWlnaHQ9JzEwMCcgeG1sbnM9J2h0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnJz48cmVjdCBmaWxsPSdncmVlbicgd2lkdGg9JzEwMCcgaGVpZ2h0PScxMDAnPjwvcmVjdD48L3N2Zz4="/>
</svg>

A full testcase is available at crbug.com/224317

This can be fixed by merging the following Blink patches:
1) https://codereview.chromium.org/16433002 - Prepopulate the memoryCache with data:uri images
(This is not a straight merge and requires plumbing the data uri parsing from somewhere, possible DataURI.h)
2) https://codereview.chromium.org/16922002 - Do not load deferred/disabled data uri images
3) https://codereview.chromium.org/17682003 - Do not fire load events from frames with scripting disabled
Comment 1 Radar WebKit Bug Importer 2013-06-26 14:26:53 PDT
<rdar://problem/14281148>
Comment 2 youenn fablet 2013-09-17 06:31:20 PDT
Why not simply updating CachedImage::load to enforce data:uri images loading even when network loading is disabled?
Comment 3 Philip Rogers 2013-09-20 14:34:32 PDT
(In reply to comment #2)
> Why not simply updating CachedImage::load to enforce data:uri images loading even when network loading is disabled?

I'm afraid the loader code (in both Blink and WebKit) has diverged significantly since I wrote this patch. My best recollection is that doing this would result in a request being sent to the platform layer with a null networking context which could escape the SVG Image "sandbox" that prevents resource requests. This could allow an SVG image embedded in an email to work as a tracker, for example.

This may no longer be true (or may have only been true for Chromium) and I would encourage you to trace this code further to see what happens today.
Comment 4 youenn fablet 2013-11-04 07:44:40 PST
Moving data uri parsing into WebCore allows removing memory copy between WebCore and the network stack.
I did a test with run-perf-test and this makes a difference in terms of loading time for large data uri images.

I will shortly submit a patch along the lines of Blink patches.
The data URL parsing routine code originates from Source/WebCore/platform/network/DataURL.cpp.
Comment 5 youenn fablet 2013-11-04 08:39:50 PST
Created attachment 215923 [details]
Patch
Comment 6 WebKit Commit Bot 2013-11-04 08:42:14 PST
Attachment 215923 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/loader/data-uri-images-load-synchronously-expected.txt', u'LayoutTests/loader/data-uri-images-load-synchronously.html', u'LayoutTests/loader/data-uri-images-reload-synchronously-expected.txt', u'LayoutTests/loader/data-uri-images-reload-synchronously.html', u'LayoutTests/svg/as-image/resources/svg-with-data-uri.svg', u'LayoutTests/svg/as-image/svg-as-image-with-data-uri-expected.txt', u'LayoutTests/svg/as-image/svg-as-image-with-data-uri.html', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/text/Base64.cpp', u'Source/WTF/wtf/text/Base64.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/Document.cpp', u'Source/WebCore/loader/ImageLoader.cpp', u'Source/WebCore/loader/cache/CachedRawResource.cpp', u'Source/WebCore/loader/cache/CachedRawResource.h', u'Source/WebCore/loader/cache/CachedResourceLoader.cpp']" exit_code: 1
Source/WebCore/loader/cache/CachedResourceLoader.cpp:40:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Build Bot 2013-11-04 09:29:50 PST
Comment on attachment 215923 [details]
Patch

Attachment 215923 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/19668774

New failing tests:
fast/forms/basic-buttons.html
compositing/tiling/huge-layer-img.html
Comment 8 Build Bot 2013-11-04 09:29:53 PST
Created attachment 215925 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 9 Build Bot 2013-11-04 10:58:21 PST
Comment on attachment 215923 [details]
Patch

Attachment 215923 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/19658832

New failing tests:
fast/forms/basic-buttons.html
compositing/tiling/huge-layer-img.html
Comment 10 Build Bot 2013-11-04 10:58:25 PST
Created attachment 215931 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 11 Build Bot 2013-11-04 12:09:02 PST
Comment on attachment 215923 [details]
Patch

Attachment 215923 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/19618811

New failing tests:
fast/forms/basic-buttons.html
compositing/tiling/huge-layer-img.html
Comment 12 Build Bot 2013-11-04 12:09:07 PST
Created attachment 215938 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 13 Philip Rogers 2013-11-04 12:51:09 PST
Comment on attachment 215923 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215923&action=review

> Source/WebCore/ChangeLog:5
> +

Please add more information about what this patch does. It may be useful to list the specific patches being merged as well.

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:164
> +static PassRefPtr<ResourceBuffer> parseDataURI(const URL &url, ResourceResponse &response)

This seems to be an exact copy of WebCore/platform/network/DataURL.cpp's handleDataURL. Can these be unified?

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:246
> +            memoryCache()->add(resource);

I think this may cause a security bug in the xml parser, see https://chromiumcodereview.appspot.com/18226005
Comment 14 youenn fablet 2013-11-07 02:12:28 PST
Created attachment 216279 [details]
Patch
Comment 15 EFL EWS Bot 2013-11-07 02:24:34 PST
Comment on attachment 216279 [details]
Patch

Attachment 216279 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/22508163
Comment 16 EFL EWS Bot 2013-11-07 02:34:03 PST
Comment on attachment 216279 [details]
Patch

Attachment 216279 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/22698186
Comment 17 kov's GTK+ EWS bot 2013-11-07 02:35:23 PST
Comment on attachment 216279 [details]
Patch

Attachment 216279 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/22748151
Comment 18 Build Bot 2013-11-07 02:41:55 PST
Comment on attachment 216279 [details]
Patch

Attachment 216279 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/22738141
Comment 19 Build Bot 2013-11-07 02:49:06 PST
Comment on attachment 216279 [details]
Patch

Attachment 216279 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/22778136
Comment 20 Build Bot 2013-11-07 02:52:43 PST
Comment on attachment 216279 [details]
Patch

Attachment 216279 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/22368149
Comment 21 youenn fablet 2013-11-07 05:47:45 PST
Created attachment 216297 [details]
Patch
Comment 22 Build Bot 2013-11-07 06:23:02 PST
Comment on attachment 216297 [details]
Patch

Attachment 216297 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/22638205
Comment 23 Build Bot 2013-11-07 06:45:04 PST
Comment on attachment 216297 [details]
Patch

Attachment 216297 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/22538193
Comment 24 youenn fablet 2013-11-08 01:37:39 PST
Created attachment 216366 [details]
Patch
Comment 25 Build Bot 2013-11-08 02:06:42 PST
Comment on attachment 216366 [details]
Patch

Attachment 216366 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/22638447
Comment 26 Build Bot 2013-11-08 02:11:57 PST
Comment on attachment 216366 [details]
Patch

Attachment 216366 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/22508463
Comment 27 youenn fablet 2013-11-21 02:47:42 PST
Created attachment 217540 [details]
xcode proj fixed
Comment 28 Build Bot 2013-11-21 03:40:41 PST
Comment on attachment 217540 [details]
xcode proj fixed

Attachment 217540 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/29928217

New failing tests:
fast/forms/basic-buttons.html
compositing/tiling/huge-layer-img.html
Comment 29 Build Bot 2013-11-21 03:40:44 PST
Created attachment 217545 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 30 Build Bot 2013-11-21 05:07:26 PST
Comment on attachment 217540 [details]
xcode proj fixed

Attachment 217540 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/29858088

New failing tests:
fast/forms/basic-buttons.html
compositing/tiling/huge-layer-img.html
Comment 31 Build Bot 2013-11-21 05:07:34 PST
Created attachment 217548 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 32 youenn fablet 2013-11-26 05:27:22 PST
Created attachment 217874 [details]
updated mac baselines

failing mac tests results are slightly changing due to data urls loading being synchronous with the patch, hence updating the expected tests results.
Comment 33 youenn fablet 2014-02-10 03:10:09 PST
Patch should be simplified once data:// url handling is migrated into WebCore (cf. bug 109199).
Comment 34 Steven Vachon 2014-07-21 03:47:30 PDT
http://www.svachon.com/webframes/examples.html

The non-vector examples do not work in WebKit/Safari, but they work everywhere else. Please fix this in time for Safari 8's release.
Comment 35 Alexey Proskuryakov 2014-10-22 20:03:31 PDT

*** This bug has been marked as a duplicate of bug 99677 ***