REGRESSION: Safari unable to load PDF in <embed> (docs.legalconnect.com)

https://bugs.webkit.org/show_bug.cgi?id=217451
<rdar://problem/69767043>

Reviewed by Alex Christensen.

Source/WebCore:

Test: fast/replaced/pdf-as-embed-with-no-mime-type-is-not-blank.html

If plugins are enabled, we'll always let the request go through, and WebKit will
guess that files with PDFPlugin-handled extensions should instantiate PDFPlugin,
even if no other plugins are available.

However, if plugins are disabled, requestPlugin() will early return if the explicitly
specified MIME type is not handled by an application plugin (even though the downstream
WebKit code would have happily instantiated an application plugin for us).

Application plugins shouldn't depend on the plugin enablement setting.
To fix this, have SubframeLoader guess the MIME type if not explicitly specified
(matching WebKit's behavior), and allow the request if it matches an application plugin.

* loader/SubframeLoader.cpp:
(WebCore::findPluginMIMETypeFromURL):
Improve this previously logging-only function to use the lastPathComponent
of the URL instead of randomly looking at the end of the URL, to ignore
query strings and fragments when looking for the file extension.

(WebCore::FrameLoader::SubframeLoader::requestPlugin):
Make use of findPluginMIMETypeFromURL to guess the MIME type if it's not
explicitly specified. If the guessed MIME type is one that is handled
by application plugins, allow the request to go out to WebKit (which
may then instantiate a PDFPlugin, for example).

(WebCore::logPluginRequest):
(WebCore::FrameLoader::SubframeLoader::requestObject):
(WebCore::FrameLoader::SubframeLoader::createJavaAppletWidget):
Pass the URL instead of stringifying it, so we can lastPathComponent as above.

LayoutTests:

* fast/replaced/pdf-as-embed-with-no-mime-type-is-not-blank-expected-mismatch.html: Added.
* fast/replaced/pdf-as-embed-with-no-mime-type-is-not-blank.html: Added.
Add a test ensuring that <embed> with no specified MIME type still renders the PDF.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@268162 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

LayoutTests/ChangeLog

@@ -1,3 +1,15 @@
+2020-10-07  Tim Horton  <[email protected]>
+
+        REGRESSION: Safari unable to load PDF in <embed> (docs.legalconnect.com)
+        https://bugs.webkit.org/show_bug.cgi?id=217451
+        <rdar://problem/69767043>
+
+        Reviewed by Alex Christensen.
+
+        * fast/replaced/pdf-as-embed-with-no-mime-type-is-not-blank-expected-mismatch.html: Added.
+        * fast/replaced/pdf-as-embed-with-no-mime-type-is-not-blank.html: Added.
+        Add a test ensuring that <embed> with no specified MIME type still renders the PDF.
+
 2020-10-07  Chris Dumez  <[email protected]>
 
         Constructing a AudioWorkletNode should construct an AudioWorkletProcessor on the Worklet thread

LayoutTests/fast/replaced/pdf-as-embed-with-no-mime-type-is-not-blank-expected-mismatch.html

@@ -0,0 +1,2 @@
+<script>testRunner.setPluginsEnabled(false);</script>
+<embed src="resources/image.pdf"></embed>

LayoutTests/fast/replaced/pdf-as-embed-with-no-mime-type-is-not-blank.html

@@ -4278,6 +4278,7 @@ storage/indexeddb/request-leak-private.html [ Skip ]
 storage/indexeddb/request-leak.html [ Skip ]
 
 webkit.org/b/194711 fast/replaced/encrypted-pdf-as-object-and-embed.html [ Failure ]
+webkit.org/b/194711 fast/replaced/pdf-as-embed-with-no-mime-type-is-not-blank.html [ Failure ]
 
 webkit.org/b/195461 http/tests/referrer-policy-iframe/no-referrer/cross-origin-http-http.html [ Failure ]
 webkit.org/b/195461 http/tests/referrer-policy-iframe/no-referrer/same-origin.html [ Failure ]

LayoutTests/platform/win/TestExpectations

@@ -1,3 +1,42 @@
+2020-10-07  Tim Horton  <[email protected]>
+
+        REGRESSION: Safari unable to load PDF in <embed> (docs.legalconnect.com)
+        https://bugs.webkit.org/show_bug.cgi?id=217451
+        <rdar://problem/69767043>
+
+        Reviewed by Alex Christensen.
+
+        Test: fast/replaced/pdf-as-embed-with-no-mime-type-is-not-blank.html
+
+        If plugins are enabled, we'll always let the request go through, and WebKit will
+        guess that files with PDFPlugin-handled extensions should instantiate PDFPlugin,
+        even if no other plugins are available.
+
+        However, if plugins are disabled, requestPlugin() will early return if the explicitly
+        specified MIME type is not handled by an application plugin (even though the downstream
+        WebKit code would have happily instantiated an application plugin for us).
+
+        Application plugins shouldn't depend on the plugin enablement setting.
+        To fix this, have SubframeLoader guess the MIME type if not explicitly specified
+        (matching WebKit's behavior), and allow the request if it matches an application plugin.
+
+        * loader/SubframeLoader.cpp:
+        (WebCore::findPluginMIMETypeFromURL):
+        Improve this previously logging-only function to use the lastPathComponent
+        of the URL instead of randomly looking at the end of the URL, to ignore
+        query strings and fragments when looking for the file extension.
+
+        (WebCore::FrameLoader::SubframeLoader::requestPlugin):
+        Make use of findPluginMIMETypeFromURL to guess the MIME type if it's not
+        explicitly specified. If the guessed MIME type is one that is handled
+        by application plugins, allow the request to go out to WebKit (which
+        may then instantiate a PDFPlugin, for example).
+
+        (WebCore::logPluginRequest):
+        (WebCore::FrameLoader::SubframeLoader::requestObject):
+        (WebCore::FrameLoader::SubframeLoader::createJavaAppletWidget):
+        Pass the URL instead of stringifying it, so we can lastPathComponent as above.
+
 2020-10-07  Chris Dumez  <[email protected]>
 
         Constructing a AudioWorkletNode should construct an AudioWorkletProcessor on the Worklet thread

Source/WebCore/ChangeLog

@@ -148,31 +148,14 @@ bool FrameLoader::SubframeLoader::pluginIsLoadable(const URL& url, const String&
     return true;
 }
 
-bool FrameLoader::SubframeLoader::requestPlugin(HTMLPlugInImageElement& ownerElement, const URL& url, const String& mimeType, const Vector<String>& paramNames, const Vector<String>& paramValues, bool useFallback)
+static String findPluginMIMETypeFromURL(Page& page, const URL& url)
 {
-    // Application plug-ins are plug-ins implemented by the user agent, for example Qt plug-ins,
-    // as opposed to third-party code such as Flash. The user agent decides whether or not they are
-    // permitted, rather than WebKit.
-    if (!(m_frame.settings().arePluginsEnabled() || MIMETypeRegistry::isApplicationPluginMIMEType(mimeType)))
-        return false;
-
-    if (!pluginIsLoadable(url, mimeType))
-        return false;
-
-    ASSERT(ownerElement.hasTagName(objectTag) || ownerElement.hasTagName(embedTag));
-    return loadPlugin(ownerElement, url, mimeType, paramNames, paramValues, useFallback);
-}
-
-static String findPluginMIMETypeFromURL(Page& page, const StringView& url)
-{
-    if (!url)
-        return { };
-
-    size_t dotIndex = url.reverseFind('.');
+    auto lastPathComponent = url.lastPathComponent();
+    size_t dotIndex = lastPathComponent.reverseFind('.');
     if (dotIndex == notFound)
         return { };
 
-    auto extensionFromURL = url.substring(dotIndex + 1);
+    auto extensionFromURL = lastPathComponent.substring(dotIndex + 1);
 
     for (auto& type : page.pluginData().webVisibleMimeTypes()) {
         for (auto& extension : type.extensions) {
@@ -184,7 +167,28 @@ static String findPluginMIMETypeFromURL(Page& page, const StringView& url)
     return { };
 }
 
-static void logPluginRequest(Page* page, const String& mimeType, const String& url, bool success)
+bool FrameLoader::SubframeLoader::requestPlugin(HTMLPlugInImageElement& ownerElement, const URL& url, const String& explicitMIMEType, const Vector<String>& paramNames, const Vector<String>& paramValues, bool useFallback)
+{
+    String mimeType = explicitMIMEType;
+    if (mimeType.isEmpty()) {
+        if (auto page = ownerElement.document().page())
+            mimeType = findPluginMIMETypeFromURL(*page, url);
+    }
+
+    // Application plug-ins are plug-ins implemented by the user agent, for example Qt plug-ins,
+    // as opposed to third-party code such as Flash. The user agent decides whether or not they are
+    // permitted, rather than WebKit.
+    if (!(m_frame.settings().arePluginsEnabled() || MIMETypeRegistry::isApplicationPluginMIMEType(mimeType)))
+        return false;
+
+    if (!pluginIsLoadable(url, explicitMIMEType))
+        return false;
+
+    ASSERT(ownerElement.hasTagName(objectTag) || ownerElement.hasTagName(embedTag));
+    return loadPlugin(ownerElement, url, explicitMIMEType, paramNames, paramValues, useFallback);
+}
+
+static void logPluginRequest(Page* page, const String& mimeType, const URL& url, bool success)
 {
     if (!page)
         return;
@@ -230,7 +234,7 @@ bool FrameLoader::SubframeLoader::requestObject(HTMLPlugInImageElement& ownerEle
     bool useFallback;
     if (shouldUsePlugin(completedURL, mimeType, hasFallbackContent, useFallback)) {
         bool success = requestPlugin(ownerElement, completedURL, mimeType, paramNames, paramValues, useFallback);
-        logPluginRequest(document.page(), mimeType, completedURL.string(), success);
+        logPluginRequest(document.page(), mimeType, completedURL, success);
         return success;
     }
 
@@ -276,7 +280,7 @@ RefPtr<Widget> FrameLoader::SubframeLoader::createJavaAppletWidget(const IntSize
     if (m_frame.settings().arePluginsEnabled())
         widget = m_frame.loader().client().createJavaAppletWidget(size, element, baseURL, paramNames, paramValues);
 
-    logPluginRequest(m_frame.page(), element.serviceType(), String(), widget);
+    logPluginRequest(m_frame.page(), element.serviceType(), { }, widget);
 
     if (!widget) {
         RenderEmbeddedObject* renderer = element.renderEmbeddedObject();