Title: [258986] trunk/Source
Revision
258986
Author
katherine_che...@apple.com
Date
2020-03-25 09:29:54 -0700 (Wed, 25 Mar 2020)

Log Message

App-bound domain checks should provide more debugging details at script evaluation sites
https://bugs.webkit.org/show_bug.cgi?id=209521
<rdar://problem/60837954>

Reviewed by Chris Dumez.

Source/WebCore:

* bindings/js/ScriptController.cpp:
(WebCore::ScriptController::executeScriptInWorld):
Return makeUnexpected object with an error message instead of null to
provide more details as to why the executeScriptInWorld()
call was not completed. Also add console logging and release logging.

* page/Frame.cpp:
(WebCore::Frame::injectUserScriptImmediately):
There is no option to return an exception here, so this patch adds
console logging and release logging.

Source/WebKit:

Return an exception because that option is available here, and also add
console and release logging for consistency across app-bound domain checks.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::runJavaScript):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (258985 => 258986)


--- trunk/Source/WebCore/ChangeLog	2020-03-25 16:19:00 UTC (rev 258985)
+++ trunk/Source/WebCore/ChangeLog	2020-03-25 16:29:54 UTC (rev 258986)
@@ -1,3 +1,22 @@
+2020-03-25  Kate Cheney  <katherine_che...@apple.com>
+
+        App-bound domain checks should provide more debugging details at script evaluation sites
+        https://bugs.webkit.org/show_bug.cgi?id=209521
+        <rdar://problem/60837954>
+
+        Reviewed by Chris Dumez.
+
+        * bindings/js/ScriptController.cpp:
+        (WebCore::ScriptController::executeScriptInWorld):
+        Return makeUnexpected object with an error message instead of null to
+        provide more details as to why the executeScriptInWorld()
+        call was not completed. Also add console logging and release logging.
+ 
+        * page/Frame.cpp:
+        (WebCore::Frame::injectUserScriptImmediately):
+        There is no option to return an exception here, so this patch adds
+        console logging and release logging.
+
 2020-03-25  Simon Fraser  <simon.fra...@apple.com>
 
         REGRESSION (r251385): box-shadow interferes with backdrop-filter

Modified: trunk/Source/WebCore/bindings/js/ScriptController.cpp (258985 => 258986)


--- trunk/Source/WebCore/bindings/js/ScriptController.cpp	2020-03-25 16:19:00 UTC (rev 258985)
+++ trunk/Source/WebCore/bindings/js/ScriptController.cpp	2020-03-25 16:29:54 UTC (rev 258986)
@@ -40,6 +40,7 @@
 #include "JSDocument.h"
 #include "JSExecState.h"
 #include "LoadableModuleScript.h"
+#include "Logging.h"
 #include "ModuleFetchFailureKind.h"
 #include "ModuleFetchParameters.h"
 #include "NP_jsobject.h"
@@ -77,6 +78,8 @@
 #include <wtf/Threading.h>
 #include <wtf/text/TextPosition.h>
 
+#define RELEASE_LOG_ERROR_IF_ALLOWED(channel, fmt, ...) RELEASE_LOG_ERROR_IF(m_frame.isAlwaysOnLoggingAllowed(), channel, "%p - ScriptController::" fmt, this, ##__VA_ARGS__)
+
 namespace WebCore {
 using namespace JSC;
 
@@ -576,8 +579,12 @@
 
 ValueOrException ScriptController::executeScriptInWorld(DOMWrapperWorld& world, RunJavaScriptParameters&& parameters)
 {
-    if (m_frame.loader().client().hasNavigatedAwayFromAppBoundDomain() && !m_frame.loader().client().needsInAppBrowserPrivacyQuirks())
-        return jsNull();
+    if (m_frame.loader().client().hasNavigatedAwayFromAppBoundDomain() && !m_frame.loader().client().needsInAppBrowserPrivacyQuirks()) {
+        if (auto* document = m_frame.document())
+            document->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "Ignoring user script injection for non-app bound domain.");
+        RELEASE_LOG_ERROR_IF_ALLOWED(Loading, "executeScriptInWorld: Ignoring user script injection for non app-bound domain");
+        return makeUnexpected(ExceptionDetails { "Ignoring user script injection for non-app bound domain"_s });
+    }
 
     UserGestureIndicator gestureIndicator(parameters.forceUserGesture == ForceUserGesture::Yes ? Optional<ProcessingUserGestureState>(ProcessingUserGesture) : WTF::nullopt);
 
@@ -860,3 +867,5 @@
 }
 
 } // namespace WebCore
+
+#undef RELEASE_LOG_ERROR_IF_ALLOWED

Modified: trunk/Source/WebCore/page/Frame.cpp (258985 => 258986)


--- trunk/Source/WebCore/page/Frame.cpp	2020-03-25 16:19:00 UTC (rev 258985)
+++ trunk/Source/WebCore/page/Frame.cpp	2020-03-25 16:29:54 UTC (rev 258986)
@@ -107,6 +107,8 @@
 #include <wtf/StdLibExtras.h>
 #include <wtf/text/StringBuilder.h>
 
+#define RELEASE_LOG_ERROR_IF_ALLOWED(channel, fmt, ...) RELEASE_LOG_ERROR_IF(isAlwaysOnLoggingAllowed(), channel, "%p - Frame::" fmt, this, ##__VA_ARGS__)
+
 namespace WebCore {
 
 using namespace HTMLNames;
@@ -624,8 +626,12 @@
 
 void Frame::injectUserScriptImmediately(DOMWrapperWorld& world, const UserScript& script)
 {
-    if (loader().client().hasNavigatedAwayFromAppBoundDomain() && !loader().client().needsInAppBrowserPrivacyQuirks())
+    if (loader().client().hasNavigatedAwayFromAppBoundDomain() && !loader().client().needsInAppBrowserPrivacyQuirks()) {
+        if (auto* document = this->document())
+            document->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "Ignoring user script injection for non-app bound domain."_s);
+        RELEASE_LOG_ERROR_IF_ALLOWED(Loading, "injectUserScriptImmediately: Ignoring user script injection for non app-bound domain");
         return;
+    }
 
     auto* document = this->document();
     if (!document)
@@ -1047,3 +1053,5 @@
 }
 
 } // namespace WebCore
+
+#undef RELEASE_LOG_ERROR_IF_ALLOWED

Modified: trunk/Source/WebKit/ChangeLog (258985 => 258986)


--- trunk/Source/WebKit/ChangeLog	2020-03-25 16:19:00 UTC (rev 258985)
+++ trunk/Source/WebKit/ChangeLog	2020-03-25 16:29:54 UTC (rev 258986)
@@ -1,3 +1,17 @@
+2020-03-25  Kate Cheney  <katherine_che...@apple.com>
+
+        App-bound domain checks should provide more debugging details at script evaluation sites
+        https://bugs.webkit.org/show_bug.cgi?id=209521
+        <rdar://problem/60837954>
+
+        Reviewed by Chris Dumez.
+
+        Return an exception because that option is available here, and also add
+        console and release logging for consistency across app-bound domain checks.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::runJavaScript):
+
 2020-03-25  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Avoid querying pasteboard strings while dragging content over a potential drop target

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (258985 => 258986)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2020-03-25 16:19:00 UTC (rev 258985)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2020-03-25 16:29:54 UTC (rev 258986)
@@ -3448,6 +3448,9 @@
     
     if (hasNavigatedAwayFromAppBoundDomain() == NavigatedAwayFromAppBoundDomain::Yes && !m_needsInAppBrowserPrivacyQuirks) {
         send(Messages::WebPageProxy::ScriptValueCallback({ }, ExceptionDetails { "Unable to execute _javascript_"_s }, callbackID));
+        if (auto* document = m_page->mainFrame().document())
+            document->addConsoleMessage(MessageSource::Security, MessageLevel::Warning, "Ignoring user script injection for non-app bound domain.");
+        RELEASE_LOG_ERROR_IF_ALLOWED(Loading, "runJavaScript: Ignoring user script injection for non app-bound domain");
         return;
     }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to