Title: [226272] trunk
Revision
226272
Author
[email protected]
Date
2017-12-22 12:41:48 -0800 (Fri, 22 Dec 2017)

Log Message

REGRESSION(r223678): Cannot copy & paste a web page content into Yahoo! Mail
https://bugs.webkit.org/show_bug.cgi?id=181114

Reviewed by Geoffrey Garen.

Source/WebCore:

Turns out converting all URLs to blob isn't Web compatible. Don't do this conversion on HTTP, HTTP, and data URLs
since websites tend to have access to contents accessible via those protocols, and blob URL conversion would break
Yahoo! Mail, Gmail, and other major online email services.

We've also considered using data URLs instead of blob URLs for conversion but pasting a large image converted into
a data URL seems to break WordPress (it stores an empty post instead of the one with the image) so it's not likely
to be Web compatible either.

This patch therefore disables the blob conversion in sanitizeMarkupWithArchive for HTTP, HTTPS, data URLs for
cross-origin content, restoring the behavior prior to r223678. For contents converted from attributed strings,
we continue to convert to blob URL since there is no other way for websites to read local files or images references
by an in-memory web archive.

Tests: http/tests/security/clipboard/copy-paste-html-cross-in-origin-iframe-across-origin.html

* editing/cocoa/WebContentReaderCocoa.mm:
(WebCore::shouldConvertToBlob): Added.
(WebCore::sanitizeMarkupWithArchive):

LayoutTests:

Updated http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html to test the new behavior
whereby which HTTP/HTTPs and data URLs are not converted to blob URLs.

* http/tests/security/clipboard/copy-paste-html-across-origin-sanitizes-html.html:
* http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin-expected.txt: Renamed.
* http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html: Added more test cases for data URLs.
* http/tests/security/clipboard/resources/content-to-copy.html: Notify the parent that the page had finished loading.
* http/tests/security/clipboard/resources/data-url-content-to-copy.html: Added.
* http/tests/security/clipboard/resources/subdirectory/paste-html.html: Since we can no longer access contents
in the pasted frames but scripts DO run in the pasted cross-origin iframes, rely on those frames to postMessage
this frame when the image had finished loading.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (226271 => 226272)


--- trunk/LayoutTests/ChangeLog	2017-12-22 20:40:23 UTC (rev 226271)
+++ trunk/LayoutTests/ChangeLog	2017-12-22 20:41:48 UTC (rev 226272)
@@ -1,3 +1,22 @@
+2017-12-21  Ryosuke Niwa  <[email protected]>
+
+        REGRESSION(r223678): Cannot copy & paste a web page content into Yahoo! Mail
+        https://bugs.webkit.org/show_bug.cgi?id=181114
+
+        Reviewed by Geoffrey Garen.
+
+        Updated http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html to test the new behavior
+        whereby which HTTP/HTTPs and data URLs are not converted to blob URLs.
+
+        * http/tests/security/clipboard/copy-paste-html-across-origin-sanitizes-html.html:
+        * http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin-expected.txt: Renamed.
+        * http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html: Added more test cases for data URLs.
+        * http/tests/security/clipboard/resources/content-to-copy.html: Notify the parent that the page had finished loading.
+        * http/tests/security/clipboard/resources/data-url-content-to-copy.html: Added.
+        * http/tests/security/clipboard/resources/subdirectory/paste-html.html: Since we can no longer access contents
+        in the pasted frames but scripts DO run in the pasted cross-origin iframes, rely on those frames to postMessage
+        this frame when the image had finished loading.
+
 2017-12-21  John Wilander  <[email protected]>
 
         Skip http/tests/storageAccess/request-and-grant-storage-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-non-recent-user-interaction-and-try-access-from-right-frame.html

Modified: trunk/LayoutTests/http/tests/misc/copy-resolves-urls-expected.txt (226271 => 226272)


--- trunk/LayoutTests/http/tests/misc/copy-resolves-urls-expected.txt	2017-12-22 20:40:23 UTC (rev 226271)
+++ trunk/LayoutTests/http/tests/misc/copy-resolves-urls-expected.txt	2017-12-22 20:41:48 UTC (rev 226272)
@@ -2,4 +2,4 @@
 
 link
 link
-<a href="" src=""
+<a href="" src=""

Modified: trunk/LayoutTests/http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin-expected.txt (226271 => 226272)


--- trunk/LayoutTests/http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin-expected.txt	2017-12-22 20:40:23 UTC (rev 226271)
+++ trunk/LayoutTests/http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin-expected.txt	2017-12-22 20:41:48 UTC (rev 226272)
@@ -1,3 +1,7 @@
+CONSOLE MESSAGE: line 1: Blocked a frame with origin "http://localhost:8000" from accessing a frame with origin "http://127.0.0.1:8000". Protocols, domains, and ports must match.
+CONSOLE MESSAGE: line 1: Blocked a frame with origin "http://localhost:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
+CONSOLE MESSAGE: line 1: Blocked a frame with origin "http://localhost:8000" from accessing a frame with origin "http://127.0.0.1:8000". Protocols, domains, and ports must match.
+CONSOLE MESSAGE: line 1: Blocked a frame with origin "http://localhost:8000" from accessing a frame with origin "http://localhost:8080". Protocols, domains, and ports must match.
 This tests copying and pasting HTML by the default action. WebKit should sanitize the HTML across origin.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
@@ -6,18 +10,28 @@
 html in DataTransfer
 PASS html.includes("hello") is true
 PASS fragment = (new DOMParser).parseFromString(html, "text/html"); img = fragment.querySelector("img"); !!img is true
-PASS new URL(img.src).protocol is "blob:"
-PASS new URL(fragment.querySelector(".same-origin-frame").src).protocol is "blob:"
-PASS new URL(fragment.querySelector(".cross-origin-frame").src).protocol is "blob:"
-PASS frames.length is 2
-PASS new URL(frames[0].src).protocol is "blob:"
-PASS frames[0].canAccessContentDocument is true
-PASS frames[0].hasImage is true
+PASS new URL(img.src).protocol is "http:"
+PASS new URL(fragment.querySelector(".same-origin-frame").src).protocol is "http:"
+PASS new URL(fragment.querySelector(".cross-origin-frame").src).protocol is "http:"
+PASS new URL(fragment.querySelector(".same-origin-frame-with-data-url").src).protocol is "http:"
+PASS new URL(fragment.querySelector(".cross-origin-frame-with-data-url").src).protocol is "http:"
+PASS frames.length is 4
+PASS new URL(frames[0].src).protocol is "http:"
+PASS frames[0].canAccessContentDocument is false
+PASS new URL(frames[0].imageSrc).protocol is "http:"
 PASS frames[0].imageWidth is 80
-PASS new URL(frames[1].src).protocol is "blob:"
-PASS frames[1].canAccessContentDocument is true
-PASS frames[1].hasImage is true
+PASS new URL(frames[1].src).protocol is "http:"
+PASS frames[1].canAccessContentDocument is false
+PASS new URL(frames[1].imageSrc).protocol is "http:"
 PASS frames[1].imageWidth is 80
+PASS new URL(frames[2].src).protocol is "http:"
+PASS frames[2].canAccessContentDocument is false
+PASS new URL(frames[2].imageSrc).protocol is "data:"
+PASS frames[2].imageWidth is 10
+PASS new URL(frames[3].src).protocol is "http:"
+PASS frames[3].canAccessContentDocument is false
+PASS new URL(frames[3].imageSrc).protocol is "data:"
+PASS frames[3].imageWidth is 10
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html (226271 => 226272)


--- trunk/LayoutTests/http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html	2017-12-22 20:40:23 UTC (rev 226271)
+++ trunk/LayoutTests/http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html	2017-12-22 20:41:48 UTC (rev 226272)
@@ -12,6 +12,8 @@
     <img _onclick_="dangerousCode()" src=""
     <iframe class="same-origin-frame" src="" width=80 height=80></iframe>
     <iframe class="cross-origin-frame" src="" width="100" height="100"></iframe>
+    <iframe class="same-origin-frame-with-data-url" src="" width=80 height=80></iframe>
+    <iframe class="cross-origin-frame-with-data-url" src="" width=80 height=80></iframe>
 </div>
 <iframe id="destinationFrame" src=""
 </div>
@@ -23,6 +25,8 @@
 if (window.internals)
     internals.settings.setCustomPasteboardDataEnabled(true);
 
+setTimeout(finishJSTest, 3000);
+
 function runTest() {
     document.getElementById('source').focus();
     document.execCommand('selectAll');
@@ -39,20 +43,30 @@
         debug('html in DataTransfer');
         shouldBeTrue('html.includes("hello")');
         shouldBeTrue('fragment = (new DOMParser).parseFromString(html, "text/html"); img = fragment.querySelector("img"); !!img');
-        shouldBeEqualToString('new URL(img.src).protocol', 'blob:');
-        shouldBeEqualToString('new URL(fragment.querySelector(".same-origin-frame").src).protocol', 'blob:');
-        shouldBeEqualToString('new URL(fragment.querySelector(".cross-origin-frame").src).protocol', 'blob:');
+        shouldBeEqualToString('new URL(img.src).protocol', 'http:');
+        shouldBeEqualToString('new URL(fragment.querySelector(".same-origin-frame").src).protocol', 'http:');
+        shouldBeEqualToString('new URL(fragment.querySelector(".cross-origin-frame").src).protocol', 'http:');
+        shouldBeEqualToString('new URL(fragment.querySelector(".same-origin-frame-with-data-url").src).protocol', 'http:');
+        shouldBeEqualToString('new URL(fragment.querySelector(".cross-origin-frame-with-data-url").src).protocol', 'http:');
     } else if (event.data.type == 'checkedState') {
         frames = event.data.frames;
-        shouldBe('frames.length', '2');
-        shouldBeEqualToString('new URL(frames[0].src).protocol', 'blob:');
-        shouldBeTrue('frames[0].canAccessContentDocument');
-        shouldBeTrue('frames[0].hasImage');
+        shouldBe('frames.length', '4');
+        shouldBeEqualToString('new URL(frames[0].src).protocol', 'http:');
+        shouldBeFalse('frames[0].canAccessContentDocument');
+        shouldBeEqualToString('new URL(frames[0].imageSrc).protocol', 'http:');
         shouldBe('frames[0].imageWidth', '80');
-        shouldBeEqualToString('new URL(frames[1].src).protocol', 'blob:');
-        shouldBeTrue('frames[1].canAccessContentDocument');
-        shouldBeTrue('frames[1].hasImage');
+        shouldBeEqualToString('new URL(frames[1].src).protocol', 'http:');
+        shouldBeFalse('frames[1].canAccessContentDocument');
+        shouldBeEqualToString('new URL(frames[1].imageSrc).protocol', 'http:');
         shouldBe('frames[1].imageWidth', '80');
+        shouldBeEqualToString('new URL(frames[2].src).protocol', 'http:');
+        shouldBeFalse('frames[2].canAccessContentDocument');
+        shouldBeEqualToString('new URL(frames[2].imageSrc).protocol', 'data:');
+        shouldBe('frames[2].imageWidth', '10');
+        shouldBeEqualToString('new URL(frames[3].src).protocol', 'http:');
+        shouldBeFalse('frames[3].canAccessContentDocument');
+        shouldBeEqualToString('new URL(frames[3].imageSrc).protocol', 'data:');
+        shouldBe('frames[3].imageWidth', '10');
         if (window.testRunner)
             container.remove();
         finishJSTest();

Modified: trunk/LayoutTests/http/tests/security/clipboard/resources/content-to-copy.html (226271 => 226272)


--- trunk/LayoutTests/http/tests/security/clipboard/resources/content-to-copy.html	2017-12-22 20:40:23 UTC (rev 226271)
+++ trunk/LayoutTests/http/tests/security/clipboard/resources/content-to-copy.html	2017-12-22 20:41:48 UTC (rev 226272)
@@ -3,7 +3,10 @@
 <body>
 <img src=""
 <script>
-window._onload_ = parent.postMessage({type: 'contentLoaded'}, '*');
+window._onload_ = () => {
+    const image = document.querySelector('img');
+    parent.postMessage({type: 'contentLoaded', imageSrc: image.src, imageWidth: image.width}, '*');
+}
 </script>
 </body>
 </html>

Added: trunk/LayoutTests/http/tests/security/clipboard/resources/data-url-content-to-copy.html (0 => 226272)


--- trunk/LayoutTests/http/tests/security/clipboard/resources/data-url-content-to-copy.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/clipboard/resources/data-url-content-to-copy.html	2017-12-22 20:41:48 UTC (rev 226272)
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<body>
+<img src=""
+<script>
+window._onload_ = () => {
+    const image = document.querySelector('img');
+    parent.postMessage({type: 'contentLoaded', imageSrc: image.src, imageWidth: image.width}, '*');
+}
+</script>
+</body>
+</html>
+

Modified: trunk/LayoutTests/http/tests/security/clipboard/resources/subdirectory/paste-html.html (226271 => 226272)


--- trunk/LayoutTests/http/tests/security/clipboard/resources/subdirectory/paste-html.html	2017-12-22 20:40:23 UTC (rev 226271)
+++ trunk/LayoutTests/http/tests/security/clipboard/resources/subdirectory/paste-html.html	2017-12-22 20:41:48 UTC (rev 226272)
@@ -2,6 +2,7 @@
 <html>
 <head>
 <script>
+let loadedContent = [];
 _onmessage_ = function (event) {
     if (event.data.type == 'paste') {
         document.body.focus();
@@ -8,6 +9,9 @@
         document.execCommand('selectAll');
         if (window.testRunner)
             document.execCommand('paste');
+    } else if (event.data.type == 'contentLoaded') {
+        loadedContent.push(event.data);
+        checkIfLoadCompleted();
     }
 }
 
@@ -14,41 +18,23 @@
 let frames = [];
 function doPaste(event) {
     top.postMessage({type: 'pasted', html: event.clipboardData.getData('text/html')}, '*');
-    setTimeout(() => {
-        frames = Array.from(document.body.querySelectorAll('iframe'));
+}
 
-        Promise.all(frames.map((frame) => {
-            return new Promise((resolve) => {
-                const waitForImage = () => {
-                    const img = frame.contentDocument.querySelector('img');
-                    if (img && !img.complete)
-                        img._onload_ = resolve;
-                    else
-                        resolve();
-                }
+function checkIfLoadCompleted() {
+    const frames = Array.from(document.body.querySelectorAll('iframe'));
+    if (loadedContent.length < frames.length)
+        return;
 
-                if (frame.contentDocument && (!frame.contentDocument.body || frame.contentDocument.body.innerHTML == ''))
-                    frame._onload_ = waitForImage;
-                else
-                    waitForImage();
-
-            });
-        })).then(checkState);
-    }, 0);
-}
-
-function checkState() {
     top.postMessage({
         type: 'checkedState',
         html: document.body.innerHTML,
-        frames: frames.map((frame) => {
-            const img = frame.contentDocument ? frame.contentDocument.querySelector('img') : null;
+        frames: frames.map((frame, i) => {
             return {
                 src: frame.src,
                 class: frame.className,
                 canAccessContentDocument: !!frame.contentDocument,
-                hasImage: !!img,
-                imageWidth: img ? img.width : null,
+                imageSrc: loadedContent[i].imageSrc,
+                imageWidth: loadedContent[i].imageWidth,
             }
         })}, '*');
 }

Modified: trunk/Source/WebCore/ChangeLog (226271 => 226272)


--- trunk/Source/WebCore/ChangeLog	2017-12-22 20:40:23 UTC (rev 226271)
+++ trunk/Source/WebCore/ChangeLog	2017-12-22 20:41:48 UTC (rev 226272)
@@ -1,3 +1,29 @@
+2017-12-21  Ryosuke Niwa  <[email protected]>
+
+        REGRESSION(r223678): Cannot copy & paste a web page content into Yahoo! Mail
+        https://bugs.webkit.org/show_bug.cgi?id=181114
+
+        Reviewed by Geoffrey Garen.
+
+        Turns out converting all URLs to blob isn't Web compatible. Don't do this conversion on HTTP, HTTP, and data URLs
+        since websites tend to have access to contents accessible via those protocols, and blob URL conversion would break
+        Yahoo! Mail, Gmail, and other major online email services.
+
+        We've also considered using data URLs instead of blob URLs for conversion but pasting a large image converted into
+        a data URL seems to break WordPress (it stores an empty post instead of the one with the image) so it's not likely
+        to be Web compatible either.
+
+        This patch therefore disables the blob conversion in sanitizeMarkupWithArchive for HTTP, HTTPS, data URLs for
+        cross-origin content, restoring the behavior prior to r223678. For contents converted from attributed strings,
+        we continue to convert to blob URL since there is no other way for websites to read local files or images references
+        by an in-memory web archive.
+
+        Tests: http/tests/security/clipboard/copy-paste-html-cross-in-origin-iframe-across-origin.html
+
+        * editing/cocoa/WebContentReaderCocoa.mm:
+        (WebCore::shouldConvertToBlob): Added.
+        (WebCore::sanitizeMarkupWithArchive):
+
 2017-12-22  Michael Catanzaro  <[email protected]>
 
         Credentials warning spam when running CodeGenerator.pm

Modified: trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm (226271 => 226272)


--- trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm	2017-12-22 20:40:23 UTC (rev 226271)
+++ trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm	2017-12-22 20:41:48 UTC (rev 226272)
@@ -353,6 +353,11 @@
     return createMarkup(range.get(), nullptr, AnnotateForInterchange, false, ResolveNonLocalURLs);
 }
 
+static bool shouldConvertToBlob(const URL& url)
+{
+    return !(url.protocolIsInHTTPFamily() || url.protocolIsData());
+}
+
 static String sanitizeMarkupWithArchive(Document& destinationDocument, MarkupAndArchive& markupAndArchive, const std::function<bool(const String)>& canShowMIMETypeAsHTML)
 {
     auto page = createPageForSanitizingWebContent();
@@ -367,9 +372,12 @@
 
     HashMap<AtomicString, AtomicString> blobURLMap;
     for (const Ref<ArchiveResource>& subresource : markupAndArchive.archive->subresources()) {
+        auto& subresourceURL = subresource->url();
+        if (!shouldConvertToBlob(subresourceURL))
+            continue;
         auto blob = Blob::create(subresource->data(), subresource->mimeType());
         String blobURL = DOMURL::createObjectURL(destinationDocument, blob);
-        blobURLMap.set(subresource->url().string(), blobURL);
+        blobURLMap.set(subresourceURL.string(), blobURL);
     }
 
     auto contentOrigin = SecurityOrigin::create(markupAndArchive.mainResource->url());
@@ -383,6 +391,9 @@
             continue;
 
         auto subframeURL = subframeMainResource->url();
+        if (!shouldConvertToBlob(subframeURL))
+            continue;
+
         MarkupAndArchive subframeContent = { String::fromUTF8(subframeMainResource->data().data(), subframeMainResource->data().size()),
             subframeMainResource.releaseNonNull(), subframeArchive.copyRef() };
         auto subframeMarkup = sanitizeMarkupWithArchive(destinationDocument, subframeContent, canShowMIMETypeAsHTML);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to