Summary: | SVG data:uri images are not handled properly | ||
---|---|---|---|
Product: | WebKit | Reporter: | Philip Rogers <pdr> |
Component: | SVG | Assignee: | 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
Philip Rogers
2013-06-26 10:14:31 PDT
Why not simply updating CachedImage::load to enforce data:uri images loading even when network loading is disabled? (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. 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. Created attachment 215923 [details]
Patch
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 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 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 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 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 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 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 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 Created attachment 216279 [details]
Patch
Comment on attachment 216279 [details] Patch Attachment 216279 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/22508163 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 on attachment 216279 [details] Patch Attachment 216279 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/22748151 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 on attachment 216279 [details] Patch Attachment 216279 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/22778136 Comment on attachment 216279 [details] Patch Attachment 216279 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/22368149 Created attachment 216297 [details]
Patch
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 on attachment 216297 [details] Patch Attachment 216297 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/22538193 Created attachment 216366 [details]
Patch
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 on attachment 216366 [details] Patch Attachment 216366 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/22508463 Created attachment 217540 [details]
xcode proj fixed
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 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 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 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
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.
Patch should be simplified once data:// url handling is migrated into WebCore (cf. bug 109199). 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. *** This bug has been marked as a duplicate of bug 99677 *** |