Title: [271650] trunk
Revision
271650
Author
katherine_che...@apple.com
Date
2021-01-20 09:44:15 -0800 (Wed, 20 Jan 2021)

Log Message

Safari says "Blocked Plug-in" instead showing a PDF
https://bugs.webkit.org/show_bug.cgi?id=220665
<rdar://problem/64372944>

Reviewed by Darin Adler.

Source/WebCore:

WebKit's PDFPlugin is a browser implementation detail and should
bypass the ContentSecurityPolicy::allowObjectFromSource() check
in order to not be blocked along with other plugins.

* html/HTMLPlugInImageElement.cpp:
(WebCore::HTMLPlugInImageElement::shouldBypassCSPForPDFPlugin const):
The check for whether to use PDFPlugin happens in
WebPage::createPlugin(). Some information there like isUnsupported,
isBlockedPlugin and pluginProcessToken are sent from the UIProcess
after the CSP check so they are unavailable here, but we know that PDFPlugin
will always be used when plugins are disabled and can use that information
to know whether to bypass CSP here in many cases. This will not relax
CSP or change behavior in other cases.

It would be ideal to check for an alternative PDF plugin in the case
where plugins are not disabled, but this information currently lives
in the UIProcess and is not sent to the WebProcess until after this
check. This patch will definitely fix Safari, where plugins are always disabled.

(WebCore::HTMLPlugInImageElement::canLoadPlugInContent const):
* html/HTMLPlugInImageElement.h:
* loader/FrameLoaderClient.h:

Source/WebKit:

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::shouldUsePDFPlugin const):
* WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::createPlugin):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::shouldUsePDFPlugin const):

Tools:

API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/WKPDFView.mm:
(TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (271649 => 271650)


--- trunk/Source/WebCore/ChangeLog	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Source/WebCore/ChangeLog	2021-01-20 17:44:15 UTC (rev 271650)
@@ -1,3 +1,34 @@
+2021-01-20  Kate Cheney  <katherine_che...@apple.com>
+
+        Safari says "Blocked Plug-in" instead showing a PDF
+        https://bugs.webkit.org/show_bug.cgi?id=220665
+        <rdar://problem/64372944>
+
+        Reviewed by Darin Adler.
+
+        WebKit's PDFPlugin is a browser implementation detail and should
+        bypass the ContentSecurityPolicy::allowObjectFromSource() check
+        in order to not be blocked along with other plugins.
+
+        * html/HTMLPlugInImageElement.cpp:
+        (WebCore::HTMLPlugInImageElement::shouldBypassCSPForPDFPlugin const):
+        The check for whether to use PDFPlugin happens in
+        WebPage::createPlugin(). Some information there like isUnsupported,
+        isBlockedPlugin and pluginProcessToken are sent from the UIProcess
+        after the CSP check so they are unavailable here, but we know that PDFPlugin
+        will always be used when plugins are disabled and can use that information
+        to know whether to bypass CSP here in many cases. This will not relax
+        CSP or change behavior in other cases.
+
+        It would be ideal to check for an alternative PDF plugin in the case
+        where plugins are not disabled, but this information currently lives
+        in the UIProcess and is not sent to the WebProcess until after this
+        check. This patch will definitely fix Safari, where plugins are always disabled.
+
+        (WebCore::HTMLPlugInImageElement::canLoadPlugInContent const):
+        * html/HTMLPlugInImageElement.h:
+        * loader/FrameLoaderClient.h:
+
 2021-01-20  Rob Buis  <rb...@igalia.com>
 
         Support transferred min/max block size for aspect-ratio

Modified: trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp (271649 => 271650)


--- trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp	2021-01-20 17:44:15 UTC (rev 271650)
@@ -25,6 +25,7 @@
 #include "ChromeClient.h"
 #include "CommonVM.h"
 #include "ContentSecurityPolicy.h"
+#include "DocumentLoader.h"
 #include "EventNames.h"
 #include "Frame.h"
 #include "FrameLoaderClient.h"
@@ -268,6 +269,23 @@
     HTMLPlugInElement::resumeFromDocumentSuspension();
 }
 
+bool HTMLPlugInImageElement::shouldBypassCSPForPDFPlugin(const String& contentType) const
+{
+#if ENABLE(PDFKIT_PLUGIN)
+    // We only consider bypassing this CSP check if plugins are disabled. In that case we know that
+    // any plugin used is a browser implementation detail. It is not safe to skip this check
+    // if plugins are enabled in case an external plugin is used to load PDF content.
+    // FIXME: Check for alternative PDF plugins here so we can bypass this CSP check for PDFPlugin even when plugins are enabled.
+    if (document().frame()->arePluginsEnabled())
+        return false;
+
+    return document().frame()->loader().client().shouldUsePDFPlugin(contentType, document().url().path());
+#else
+    UNUSED_PARAM(contentType);
+    return false;
+#endif
+}
+
 bool HTMLPlugInImageElement::canLoadPlugInContent(const String& relativeURL, const String& mimeType) const
 {
     // Elements in user agent show tree should load whatever the embedding document policy is.
@@ -283,7 +301,7 @@
 
     contentSecurityPolicy.upgradeInsecureRequestIfNeeded(completedURL, ContentSecurityPolicy::InsecureRequestType::Load);
 
-    if (!contentSecurityPolicy.allowObjectFromSource(completedURL))
+    if (!shouldBypassCSPForPDFPlugin(mimeType) && !contentSecurityPolicy.allowObjectFromSource(completedURL))
         return false;
 
     auto& declaredMimeType = document().isPluginDocument() && document().ownerElement() ?

Modified: trunk/Source/WebCore/html/HTMLPlugInImageElement.h (271649 => 271650)


--- trunk/Source/WebCore/html/HTMLPlugInImageElement.h	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Source/WebCore/html/HTMLPlugInImageElement.h	2021-01-20 17:44:15 UTC (rev 271650)
@@ -68,6 +68,7 @@
 private:
     bool isPlugInImageElement() const final { return true; }
 
+    bool shouldBypassCSPForPDFPlugin(const String&) const;
     bool canLoadPlugInContent(const String& relativeURL, const String& mimeType) const;
     bool canLoadURL(const URL&) const;
 

Modified: trunk/Source/WebCore/loader/FrameLoaderClient.h (271649 => 271650)


--- trunk/Source/WebCore/loader/FrameLoaderClient.h	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Source/WebCore/loader/FrameLoaderClient.h	2021-01-20 17:44:15 UTC (rev 271650)
@@ -380,6 +380,10 @@
     virtual bool shouldEnableInAppBrowserPrivacyProtections() const { return false; }
     virtual void notifyPageOfAppBoundBehavior() { }
 #endif
+
+#if ENABLE(PDFKIT_PLUGIN)
+    virtual bool shouldUsePDFPlugin(const String&, StringView) const { return false; }
+#endif
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebKit/ChangeLog (271649 => 271650)


--- trunk/Source/WebKit/ChangeLog	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Source/WebKit/ChangeLog	2021-01-20 17:44:15 UTC (rev 271650)
@@ -1,3 +1,20 @@
+2021-01-20  Kate Cheney  <katherine_che...@apple.com>
+
+        Safari says "Blocked Plug-in" instead showing a PDF
+        https://bugs.webkit.org/show_bug.cgi?id=220665
+        <rdar://problem/64372944>
+
+        Reviewed by Darin Adler.
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::shouldUsePDFPlugin const):
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::createPlugin):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::shouldUsePDFPlugin const):
+
 2021-01-20  Michael Catanzaro  <mcatanz...@gnome.org>
 
         [WPE][GTK] Ensure URI scheme handler is not registered multiple times

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (271649 => 271650)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp	2021-01-20 17:44:15 UTC (rev 271650)
@@ -1948,6 +1948,14 @@
 }
 #endif
 
+#if ENABLE(PDFKIT_PLUGIN)
+bool WebFrameLoaderClient::shouldUsePDFPlugin(const String& contentType, StringView path) const
+{
+    auto* page = m_frame->page();
+    return page && page->shouldUsePDFPlugin(contentType, path);
+}
+#endif
+
 } // namespace WebKit
 
 #undef PREFIX_PARAMETERS

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h (271649 => 271650)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h	2021-01-20 17:44:15 UTC (rev 271650)
@@ -289,6 +289,11 @@
     bool shouldEnableInAppBrowserPrivacyProtections() const final;
     void notifyPageOfAppBoundBehavior() final;
 #endif
+
+#if ENABLE(PDFKIT_PLUGIN)
+    bool shouldUsePDFPlugin(const String& contentType, StringView path) const final;
+#endif
+
 };
 
 // As long as EmptyFrameLoaderClient exists in WebCore, this can return nullptr.

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (271649 => 271650)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-01-20 17:44:15 UTC (rev 271650)
@@ -1082,8 +1082,7 @@
 
     if (isUnsupported || isBlockedPlugin || !pluginProcessToken) {
 #if ENABLE(PDFKIT_PLUGIN)
-        auto path = parameters.url.path();
-        if (shouldUsePDFPlugin() && (MIMETypeRegistry::isPDFOrPostScriptMIMEType(parameters.mimeType) || (parameters.mimeType.isEmpty() && (path.endsWithIgnoringASCIICase(".pdf") || path.endsWithIgnoringASCIICase(".ps")))))
+        if (shouldUsePDFPlugin(parameters.mimeType, parameters.url.path()))
             return PDFPlugin::create(*frame);
 #endif
     }

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (271649 => 271650)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2021-01-20 17:44:15 UTC (rev 271650)
@@ -1083,7 +1083,6 @@
     void setArtificialPluginInitializationDelayEnabled(bool enabled) { m_artificialPluginInitializationDelayEnabled = enabled; }
 
 #if PLATFORM(COCOA)
-    bool shouldUsePDFPlugin() const;
     bool pdfPluginEnabled() const { return m_pdfPluginEnabled; }
     void setPDFPluginEnabled(bool enabled) { m_pdfPluginEnabled = enabled; }
 
@@ -1379,6 +1378,10 @@
 
     void dispatchWheelEventWithoutScrolling(const WebWheelEvent&, CompletionHandler<void(bool)>&&);
 
+#if ENABLE(PDFKIT_PLUGIN)
+    bool shouldUsePDFPlugin(const String& contentType, StringView path) const;
+#endif
+
 private:
     WebPage(WebCore::PageIdentifier, WebPageCreationParameters&&);
 

Modified: trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm (271649 => 271650)


--- trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm	2021-01-20 17:44:15 UTC (rev 271650)
@@ -196,9 +196,13 @@
     return nil;
 }
 
-bool WebPage::shouldUsePDFPlugin() const
+bool WebPage::shouldUsePDFPlugin(const String& contentType, StringView path) const
 {
-    return pdfPluginEnabled() && classFromPDFKit(@"PDFLayerController");
+    return pdfPluginEnabled()
+        && classFromPDFKit(@"PDFLayerController")
+        && (MIMETypeRegistry::isPDFOrPostScriptMIMEType(contentType)
+            || (contentType.isEmpty()
+                && (path.endsWithIgnoringASCIICase(".pdf") || path.endsWithIgnoringASCIICase(".ps"))));
 }
 
 typedef HashMap<String, String> SelectorNameMap;

Modified: trunk/Tools/ChangeLog (271649 => 271650)


--- trunk/Tools/ChangeLog	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Tools/ChangeLog	2021-01-20 17:44:15 UTC (rev 271650)
@@ -1,3 +1,16 @@
+2021-01-20  Kate Cheney  <katherine_che...@apple.com>
+
+        Safari says "Blocked Plug-in" instead showing a PDF
+        https://bugs.webkit.org/show_bug.cgi?id=220665
+        <rdar://problem/64372944>
+
+        Reviewed by Darin Adler.
+
+        API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKPDFView.mm:
+        (TEST):
+
 2021-01-20  Jonathan Bedard  <jbed...@apple.com>
 
         [webkitcorepy] Support alternative default pypi url on macOS (Follow-up fix)

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKPDFView.mm (271649 => 271650)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKPDFView.mm	2021-01-20 16:02:59 UTC (rev 271649)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKPDFView.mm	2021-01-20 17:44:15 UTC (rev 271650)
@@ -33,6 +33,7 @@
 #import "TestUIDelegate.h"
 #import "TestURLSchemeHandler.h"
 #import "TestWKWebView.h"
+#import <WebKit/WKProcessPoolPrivate.h>
 #import <WebKit/WKWebView.h>
 #import <WebKit/WKWebViewConfigurationPrivate.h>
 #import <WebKit/WKWebViewPrivateForTesting.h>
@@ -359,4 +360,16 @@
     EXPECT_TRUE(hadRightFrame);
 }
 
+TEST(PDFHUD, LoadPDFTypeWithPluginsBlocked)
+{
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [configuration _setOverrideContentSecurityPolicy:@"object-src 'none'"];
+    TestWKWebView *webView = [[[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()] autorelease];
+    [webView loadData:pdfData() MIMEType:@"application/pdf" characterEncodingName:@"" baseURL:[NSURL URLWithString:@"https://www.apple.com/testPath"]];
+    EXPECT_EQ(webView._pdfHUDs.count, 0u);
+    [webView _test_waitForDidFinishNavigation];
+    EXPECT_EQ(webView._pdfHUDs.count, 1u);
+    checkFrame(webView._pdfHUDs.anyObject.frame, 0, 0, 800, 600);
+}
+
 #endif
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to