Changeset 271650 in webkit


Ignore:
Timestamp:
Jan 20, 2021 9:44:15 AM (18 months ago)
Author:
Kate Cheney
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):

Location:
trunk
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r271648 r271650  
     12021-01-20  Kate Cheney  <katherine_cheney@apple.com>
     2
     3        Safari says "Blocked Plug-in" instead showing a PDF
     4        https://bugs.webkit.org/show_bug.cgi?id=220665
     5        <rdar://problem/64372944>
     6
     7        Reviewed by Darin Adler.
     8
     9        WebKit's PDFPlugin is a browser implementation detail and should
     10        bypass the ContentSecurityPolicy::allowObjectFromSource() check
     11        in order to not be blocked along with other plugins.
     12
     13        * html/HTMLPlugInImageElement.cpp:
     14        (WebCore::HTMLPlugInImageElement::shouldBypassCSPForPDFPlugin const):
     15        The check for whether to use PDFPlugin happens in
     16        WebPage::createPlugin(). Some information there like isUnsupported,
     17        isBlockedPlugin and pluginProcessToken are sent from the UIProcess
     18        after the CSP check so they are unavailable here, but we know that PDFPlugin
     19        will always be used when plugins are disabled and can use that information
     20        to know whether to bypass CSP here in many cases. This will not relax
     21        CSP or change behavior in other cases.
     22
     23        It would be ideal to check for an alternative PDF plugin in the case
     24        where plugins are not disabled, but this information currently lives
     25        in the UIProcess and is not sent to the WebProcess until after this
     26        check. This patch will definitely fix Safari, where plugins are always disabled.
     27
     28        (WebCore::HTMLPlugInImageElement::canLoadPlugInContent const):
     29        * html/HTMLPlugInImageElement.h:
     30        * loader/FrameLoaderClient.h:
     31
    1322021-01-20  Rob Buis  <rbuis@igalia.com>
    233

  • trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp

    r269785 r271650  
    2626#include "CommonVM.h"
    2727#include "ContentSecurityPolicy.h"
     28#include "DocumentLoader.h"
    2829#include "EventNames.h"
    2930#include "Frame.h"
    … …  
    269270}
    270271
     272bool HTMLPlugInImageElement::shouldBypassCSPForPDFPlugin(const String& contentType) const
     273{
     274#if ENABLE(PDFKIT_PLUGIN)
     275    // We only consider bypassing this CSP check if plugins are disabled. In that case we know that
     276    // any plugin used is a browser implementation detail. It is not safe to skip this check
     277    // if plugins are enabled in case an external plugin is used to load PDF content.
     278    // FIXME: Check for alternative PDF plugins here so we can bypass this CSP check for PDFPlugin even when plugins are enabled.
     279    if (document().frame()->arePluginsEnabled())
     280        return false;
     281
     282    return document().frame()->loader().client().shouldUsePDFPlugin(contentType, document().url().path());
     283#else
     284    UNUSED_PARAM(contentType);
     285    return false;
     286#endif
     287}
     288
    271289bool HTMLPlugInImageElement::canLoadPlugInContent(const String& relativeURL, const String& mimeType) const
    272290{
    … …  
    284302    contentSecurityPolicy.upgradeInsecureRequestIfNeeded(completedURL, ContentSecurityPolicy::InsecureRequestType::Load);
    285303
    286     if (!contentSecurityPolicy.allowObjectFromSource(completedURL))
     304    if (!shouldBypassCSPForPDFPlugin(mimeType) && !contentSecurityPolicy.allowObjectFromSource(completedURL))
    287305        return false;
    288306

  • trunk/Source/WebCore/html/HTMLPlugInImageElement.h

    r269785 r271650  
    6969    bool isPlugInImageElement() const final { return true; }
    7070
     71    bool shouldBypassCSPForPDFPlugin(const String&) const;
    7172    bool canLoadPlugInContent(const String& relativeURL, const String& mimeType) const;
    7273    bool canLoadURL(const URL&) const;

  • trunk/Source/WebCore/loader/FrameLoaderClient.h

    r271378 r271650  
    381381    virtual void notifyPageOfAppBoundBehavior() { }
    382382#endif
     383
     384#if ENABLE(PDFKIT_PLUGIN)
     385    virtual bool shouldUsePDFPlugin(const String&, StringView) const { return false; }
     386#endif
    383387};
    384388

  • trunk/Source/WebKit/ChangeLog

    r271647 r271650  
     12021-01-20  Kate Cheney  <katherine_cheney@apple.com>
     2
     3        Safari says "Blocked Plug-in" instead showing a PDF
     4        https://bugs.webkit.org/show_bug.cgi?id=220665
     5        <rdar://problem/64372944>
     6
     7        Reviewed by Darin Adler.
     8
     9        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
     10        (WebKit::WebFrameLoaderClient::shouldUsePDFPlugin const):
     11        * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
     12        * WebProcess/WebPage/WebPage.cpp:
     13        (WebKit::WebPage::createPlugin):
     14        * WebProcess/WebPage/WebPage.h:
     15        * WebProcess/WebPage/mac/WebPageMac.mm:
     16        (WebKit::WebPage::shouldUsePDFPlugin const):
     17
    1182021-01-20  Michael Catanzaro  <mcatanzaro@gnome.org>
    219

  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp

    r271378 r271650  
    19491949#endif
    19501950
     1951#if ENABLE(PDFKIT_PLUGIN)
     1952bool WebFrameLoaderClient::shouldUsePDFPlugin(const String& contentType, StringView path) const
     1953{
     1954    auto* page = m_frame->page();
     1955    return page && page->shouldUsePDFPlugin(contentType, path);
     1956}
     1957#endif
     1958
    19511959} // namespace WebKit
    19521960

  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.h

    r271378 r271650  
    290290    void notifyPageOfAppBoundBehavior() final;
    291291#endif
     292
     293#if ENABLE(PDFKIT_PLUGIN)
     294    bool shouldUsePDFPlugin(const String& contentType, StringView path) const final;
     295#endif
     296
    292297};
    293298

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

    r271469 r271650  
    10831083    if (isUnsupported || isBlockedPlugin || !pluginProcessToken) {
    10841084#if ENABLE(PDFKIT_PLUGIN)
    1085         auto path = parameters.url.path();
    1086         if (shouldUsePDFPlugin() && (MIMETypeRegistry::isPDFOrPostScriptMIMEType(parameters.mimeType) || (parameters.mimeType.isEmpty() && (path.endsWithIgnoringASCIICase(".pdf") || path.endsWithIgnoringASCIICase(".ps")))))
     1085        if (shouldUsePDFPlugin(parameters.mimeType, parameters.url.path()))
    10871086            return PDFPlugin::create(*frame);
    10881087#endif

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

    r271469 r271650  
    10841084
    10851085#if PLATFORM(COCOA)
    1086     bool shouldUsePDFPlugin() const;
    10871086    bool pdfPluginEnabled() const { return m_pdfPluginEnabled; }
    10881087    void setPDFPluginEnabled(bool enabled) { m_pdfPluginEnabled = enabled; }
    … …  
    13791378
    13801379    void dispatchWheelEventWithoutScrolling(const WebWheelEvent&, CompletionHandler<void(bool)>&&);
     1380
     1381#if ENABLE(PDFKIT_PLUGIN)
     1382    bool shouldUsePDFPlugin(const String& contentType, StringView path) const;
     1383#endif
    13811384
    13821385private:

  • trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm

    r271459 r271650  
    197197}
    198198
    199 bool WebPage::shouldUsePDFPlugin() const
    200 {
    201     return pdfPluginEnabled() && classFromPDFKit(@"PDFLayerController");
     199bool WebPage::shouldUsePDFPlugin(const String& contentType, StringView path) const
     200{
     201    return pdfPluginEnabled()
     202        && classFromPDFKit(@"PDFLayerController")
     203        && (MIMETypeRegistry::isPDFOrPostScriptMIMEType(contentType)
     204            || (contentType.isEmpty()
     205                && (path.endsWithIgnoringASCIICase(".pdf") || path.endsWithIgnoringASCIICase(".ps"))));
    202206}
    203207

  • trunk/Tools/ChangeLog

    r271649 r271650  
     12021-01-20  Kate Cheney  <katherine_cheney@apple.com>
     2
     3        Safari says "Blocked Plug-in" instead showing a PDF
     4        https://bugs.webkit.org/show_bug.cgi?id=220665
     5        <rdar://problem/64372944>
     6
     7        Reviewed by Darin Adler.
     8
     9        API test coverage.
     10
     11        * TestWebKitAPI/Tests/WebKitCocoa/WKPDFView.mm:
     12        (TEST):
     13
    1142021-01-20  Jonathan Bedard  <jbedard@apple.com>
    215

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKPDFView.mm

    r270592 r271650  
    3434#import "TestURLSchemeHandler.h"
    3535#import "TestWKWebView.h"
     36#import <WebKit/WKProcessPoolPrivate.h>
    3637#import <WebKit/WKWebView.h>
    3738#import <WebKit/WKWebViewConfigurationPrivate.h>
    … …  
    360361}
    361362
    362 #endif
     363TEST(PDFHUD, LoadPDFTypeWithPluginsBlocked)
     364{
     365    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
     366    [configuration _setOverrideContentSecurityPolicy:@"object-src 'none'"];
     367    TestWKWebView *webView = [[[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()] autorelease];
     368    [webView loadData:pdfData() MIMEType:@"application/pdf" characterEncodingName:@"" baseURL:[NSURL URLWithString:@"https://www.apple.com/testPath"]];
     369    EXPECT_EQ(webView._pdfHUDs.count, 0u);
     370    [webView _test_waitForDidFinishNavigation];
     371    EXPECT_EQ(webView._pdfHUDs.count, 1u);
     372    checkFrame(webView._pdfHUDs.anyObject.frame, 0, 0, 800, 600);
     373}
     374
     375#endif
Note: See TracChangeset for help on using the changeset viewer.