From 53bb069e87174f405d722254449544522fa10199 Mon Sep 17 00:00:00 2001 From: "vsevik@chromium.org" Date: Wed, 18 Jan 2012 20:59:10 +0000 Subject: [PATCH] Web Inspector: Unsafe cross origin access errors should show stack trace in console. https://bugs.webkit.org/show_bug.cgi?id=73099 Reviewed by Pavel Feldman. Source/WebCore: Test: http/tests/inspector/console-cross-origin-iframe-logging.html * bindings/v8/V8Proxy.cpp: (WebCore::V8Proxy::reportUnsafeAccessTo): * dom/ScriptExecutionContext.cpp: (WebCore::ScriptExecutionContext::addConsoleMessage): * dom/ScriptExecutionContext.h: * loader/FrameLoader.cpp: (WebCore::FrameLoader::shouldAllowNavigation): * page/Console.cpp: (WebCore::Console::addMessage): * page/Console.h: * page/DOMWindow.cpp: (WebCore::PostMessageTimer::PostMessageTimer): (WebCore::PostMessageTimer::stackTrace): (WebCore::DOMWindow::postMessage): (WebCore::DOMWindow::postMessageTimerFired): (WebCore::DOMWindow::printErrorMessage): LayoutTests: * http/tests/inspector/console-cross-origin-iframe-logging-expected.txt: Added. * http/tests/inspector/console-cross-origin-iframe-logging.html: Added. * http/tests/inspector/resources/cross-origin-iframe.html: Added. * platform/chromium/http/tests/inspector/console-cross-origin-iframe-logging-expected.txt: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@105310 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 12 ++++++ ...onsole-cross-origin-iframe-logging-expected.txt | 16 +++++++ .../console-cross-origin-iframe-logging.html | 50 ++++++++++++++++++++++ .../inspector/resources/cross-origin-iframe.html | 7 +++ ...onsole-cross-origin-iframe-logging-expected.txt | 16 +++++++ Source/WebCore/ChangeLog | 26 +++++++++++ Source/WebCore/bindings/v8/V8Proxy.cpp | 6 ++- Source/WebCore/dom/ScriptExecutionContext.cpp | 6 +++ Source/WebCore/dom/ScriptExecutionContext.h | 1 + Source/WebCore/loader/FrameLoader.cpp | 21 +++++---- Source/WebCore/page/Console.cpp | 7 +++ Source/WebCore/page/Console.h | 1 + Source/WebCore/page/DOMWindow.cpp | 19 ++++++-- 13 files changed, 172 insertions(+), 16 deletions(-) create mode 100644 LayoutTests/http/tests/inspector/console-cross-origin-iframe-logging-expected.txt create mode 100644 LayoutTests/http/tests/inspector/console-cross-origin-iframe-logging.html create mode 100644 LayoutTests/http/tests/inspector/resources/cross-origin-iframe.html create mode 100644 LayoutTests/platform/chromium/http/tests/inspector/console-cross-origin-iframe-logging-expected.txt diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 076f2f5..4370291 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,15 @@ +2012-01-18 Vsevolod Vlasov + + Web Inspector: Unsafe cross origin access errors should show stack trace in console. + https://bugs.webkit.org/show_bug.cgi?id=73099 + + Reviewed by Pavel Feldman. + + * http/tests/inspector/console-cross-origin-iframe-logging-expected.txt: Added. + * http/tests/inspector/console-cross-origin-iframe-logging.html: Added. + * http/tests/inspector/resources/cross-origin-iframe.html: Added. + * platform/chromium/http/tests/inspector/console-cross-origin-iframe-logging-expected.txt: Added. + 2012-01-18 Alexey Proskuryakov file:// doesn't work as base URL diff --git a/LayoutTests/http/tests/inspector/console-cross-origin-iframe-logging-expected.txt b/LayoutTests/http/tests/inspector/console-cross-origin-iframe-logging-expected.txt new file mode 100644 index 0000000..99896ec --- /dev/null +++ b/LayoutTests/http/tests/inspector/console-cross-origin-iframe-logging-expected.txt @@ -0,0 +1,16 @@ +CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/inspector/resources/cross-origin-iframe.html from frame with URL http://127.0.0.1:8000/inspector/console-cross-origin-iframe-logging.html. Domains, protocols and ports must match. + +CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/inspector/resources/cross-origin-iframe.html from frame with URL http://127.0.0.1:8000/inspector/console-cross-origin-iframe-logging.html. Domains, protocols and ports must match. + +CONSOLE MESSAGE: Unable to post message to http://127.0.0.1:8000. Recipient has origin http://localhost:8000. + +Tests that cross origin errors are logged with source url and line number. + + +Unsafe JavaScript attempt to access frame with URL http://localhost:8000/inspector/resources/cross-origin-iframe.html from frame with URL http://127.0.0.1:8000/inspector/console-cross-origin-iframe-logging.html. Domains, protocols and ports must match. + +Unsafe JavaScript attempt to access frame with URL http://localhost:8000/inspector/resources/cross-origin-iframe.html from frame with URL http://127.0.0.1:8000/inspector/console-cross-origin-iframe-logging.html. Domains, protocols and ports must match. + +Unable to post message to http://127.0.0.1:8000. Recipient has origin http://localhost:8000. + + diff --git a/LayoutTests/http/tests/inspector/console-cross-origin-iframe-logging.html b/LayoutTests/http/tests/inspector/console-cross-origin-iframe-logging.html new file mode 100644 index 0000000..57d21a5 --- /dev/null +++ b/LayoutTests/http/tests/inspector/console-cross-origin-iframe-logging.html @@ -0,0 +1,50 @@ + + + + + + + +

Tests that cross origin errors are logged with source url and line number.

+ + + diff --git a/LayoutTests/http/tests/inspector/resources/cross-origin-iframe.html b/LayoutTests/http/tests/inspector/resources/cross-origin-iframe.html new file mode 100644 index 0000000..d50b96a --- /dev/null +++ b/LayoutTests/http/tests/inspector/resources/cross-origin-iframe.html @@ -0,0 +1,7 @@ + + + + +

Cross origin frame.

+ + diff --git a/LayoutTests/platform/chromium/http/tests/inspector/console-cross-origin-iframe-logging-expected.txt b/LayoutTests/platform/chromium/http/tests/inspector/console-cross-origin-iframe-logging-expected.txt new file mode 100644 index 0000000..0696ea8 --- /dev/null +++ b/LayoutTests/platform/chromium/http/tests/inspector/console-cross-origin-iframe-logging-expected.txt @@ -0,0 +1,16 @@ +CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/inspector/resources/cross-origin-iframe.html from frame with URL http://127.0.0.1:8000/inspector/console-cross-origin-iframe-logging.html. Domains, protocols and ports must match. + +CONSOLE MESSAGE: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/inspector/resources/cross-origin-iframe.html from frame with URL http://127.0.0.1:8000/inspector/console-cross-origin-iframe-logging.html. Domains, protocols and ports must match. + +CONSOLE MESSAGE: Unable to post message to http://127.0.0.1:8000. Recipient has origin http://localhost:8000. + +Tests that cross origin errors are logged with source url and line number. + + +console-cross-origin-iframe-logging.html:9Unsafe JavaScript attempt to access frame with URL http://localhost:8000/inspector/resources/cross-origin-iframe.html from frame with URL http://127.0.0.1:8000/inspector/console-cross-origin-iframe-logging.html. Domains, protocols and ports must match. + +console-cross-origin-iframe-logging.html:12Unsafe JavaScript attempt to access frame with URL http://localhost:8000/inspector/resources/cross-origin-iframe.html from frame with URL http://127.0.0.1:8000/inspector/console-cross-origin-iframe-logging.html. Domains, protocols and ports must match. + +console-cross-origin-iframe-logging.html:15Unable to post message to http://127.0.0.1:8000. Recipient has origin http://localhost:8000. + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 4c7e76f..594583a 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,29 @@ +2012-01-18 Vsevolod Vlasov + + Web Inspector: Unsafe cross origin access errors should show stack trace in console. + https://bugs.webkit.org/show_bug.cgi?id=73099 + + Reviewed by Pavel Feldman. + + Test: http/tests/inspector/console-cross-origin-iframe-logging.html + + * bindings/v8/V8Proxy.cpp: + (WebCore::V8Proxy::reportUnsafeAccessTo): + * dom/ScriptExecutionContext.cpp: + (WebCore::ScriptExecutionContext::addConsoleMessage): + * dom/ScriptExecutionContext.h: + * loader/FrameLoader.cpp: + (WebCore::FrameLoader::shouldAllowNavigation): + * page/Console.cpp: + (WebCore::Console::addMessage): + * page/Console.h: + * page/DOMWindow.cpp: + (WebCore::PostMessageTimer::PostMessageTimer): + (WebCore::PostMessageTimer::stackTrace): + (WebCore::DOMWindow::postMessage): + (WebCore::DOMWindow::postMessageTimerFired): + (WebCore::DOMWindow::printErrorMessage): + 2012-01-18 Pablo Flouret Add [CallWith] support for attributes in JSC/V8 idl code generators. diff --git a/Source/WebCore/bindings/v8/V8Proxy.cpp b/Source/WebCore/bindings/v8/V8Proxy.cpp index 0d96007..3aaf0b0 100644 --- a/Source/WebCore/bindings/v8/V8Proxy.cpp +++ b/Source/WebCore/bindings/v8/V8Proxy.cpp @@ -43,6 +43,8 @@ #include "IDBFactoryBackendInterface.h" #include "InspectorInstrumentation.h" #include "PlatformSupport.h" +#include "ScriptCallStack.h" +#include "ScriptCallStackFactory.h" #include "ScriptSourceCode.h" #include "SecurityOrigin.h" #include "Settings.h" @@ -137,10 +139,12 @@ void V8Proxy::reportUnsafeAccessTo(Frame* target) String str = "Unsafe JavaScript attempt to access frame with URL " + targetDocument->url().string() + " from frame with URL " + sourceDocument->url().string() + ". Domains, protocols and ports must match.\n"; + RefPtr stackTrace = createScriptCallStack(ScriptCallStack::maxCallStackSizeToCapture, true); + // NOTE: Safari prints the message in the target page, but it seems like // it should be in the source page. Even for delayed messages, we put it in // the source page. - sourceDocument->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, str); + sourceDocument->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, str, stackTrace.release()); } static void handleFatalErrorInV8() diff --git a/Source/WebCore/dom/ScriptExecutionContext.cpp b/Source/WebCore/dom/ScriptExecutionContext.cpp index 11f1a98..ef06c29 100644 --- a/Source/WebCore/dom/ScriptExecutionContext.cpp +++ b/Source/WebCore/dom/ScriptExecutionContext.cpp @@ -332,6 +332,12 @@ void ScriptExecutionContext::addConsoleMessage(MessageSource source, MessageType addMessage(source, type, level, message, sourceURL, lineNumber, callStack); } +void ScriptExecutionContext::addConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, PassRefPtr callStack) +{ + addMessage(source, type, level, message, String(), 0, callStack); +} + + bool ScriptExecutionContext::dispatchErrorEvent(const String& errorMessage, int lineNumber, const String& sourceURL) { EventTarget* target = errorEventTarget(); diff --git a/Source/WebCore/dom/ScriptExecutionContext.h b/Source/WebCore/dom/ScriptExecutionContext.h index 7a89b89..d747203 100644 --- a/Source/WebCore/dom/ScriptExecutionContext.h +++ b/Source/WebCore/dom/ScriptExecutionContext.h @@ -93,6 +93,7 @@ public: bool sanitizeScriptError(String& errorMessage, int& lineNumber, String& sourceURL); void reportException(const String& errorMessage, int lineNumber, const String& sourceURL, PassRefPtr); void addConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, const String& sourceURL = String(), unsigned lineNumber = 0, PassRefPtr = 0); + void addConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, PassRefPtr); // Active objects are not garbage collected even if inaccessible, e.g. because their activity may result in callbacks being invoked. bool canSuspendActiveDOMObjects(); diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp index ec73889..dcf3845 100644 --- a/Source/WebCore/loader/FrameLoader.cpp +++ b/Source/WebCore/loader/FrameLoader.cpp @@ -87,9 +87,11 @@ #include "ResourceHandle.h" #include "ResourceRequest.h" #include "SchemeRegistry.h" -#include "ScrollAnimator.h" +#include "ScriptCallStack.h" +#include "ScriptCallStackFactory.h" #include "ScriptController.h" #include "ScriptSourceCode.h" +#include "ScrollAnimator.h" #include "SecurityOrigin.h" #include "SecurityPolicy.h" #include "SegmentedString.h" @@ -1560,17 +1562,14 @@ bool FrameLoader::shouldAllowNavigation(Frame* targetFrame) const if (canAccessAncestor(activeSecurityOrigin, targetFrame)) return true; - Settings* settings = targetFrame->settings(); - if (settings && !settings->privateBrowsingEnabled()) { - Document* targetDocument = targetFrame->document(); - // FIXME: this error message should contain more specifics of why the navigation change is not allowed. - String message = "Unsafe JavaScript attempt to initiate a navigation change for frame with URL " + - targetDocument->url().string() + " from frame with URL " + activeDocument->url().string() + ".\n"; + Document* targetDocument = targetFrame->document(); + // FIXME: this error message should contain more specifics of why the navigation change is not allowed. + String message = "Unsafe JavaScript attempt to initiate a navigation change for frame with URL " + + targetDocument->url().string() + " from frame with URL " + activeDocument->url().string() + ".\n"; + + // FIXME: should we print to the console of the activeFrame as well? + targetFrame->domWindow()->printErrorMessage(message); - // FIXME: should we print to the console of the activeFrame as well? - targetFrame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, message); - } - return false; } diff --git a/Source/WebCore/page/Console.cpp b/Source/WebCore/page/Console.cpp index 7f6d188..24e13a5 100644 --- a/Source/WebCore/page/Console.cpp +++ b/Source/WebCore/page/Console.cpp @@ -128,6 +128,13 @@ static void printMessageSourceAndLevelPrefix(MessageSource source, MessageLevel printf("%s %s:", sourceString, levelString); } +void addMessage(MessageSource, MessageType, MessageLevel, const String& message, PassRefPtr); + +void Console::addMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, PassRefPtr callStack) +{ + addMessage(source, type, level, message, String(), 0, callStack); +} + void Console::addMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, const String& sourceURL, unsigned lineNumber, PassRefPtr callStack) { Page* page = this->page(); diff --git a/Source/WebCore/page/Console.h b/Source/WebCore/page/Console.h index 4c3a4f8..f30d191 100644 --- a/Source/WebCore/page/Console.h +++ b/Source/WebCore/page/Console.h @@ -56,6 +56,7 @@ public: virtual ~Console(); void addMessage(MessageSource, MessageType, MessageLevel, const String& message, const String& sourceURL = String(), unsigned lineNumber = 0, PassRefPtr = 0); + void addMessage(MessageSource, MessageType, MessageLevel, const String& message, PassRefPtr); void debug(PassRefPtr, PassRefPtr); void error(PassRefPtr, PassRefPtr); diff --git a/Source/WebCore/page/DOMWindow.cpp b/Source/WebCore/page/DOMWindow.cpp index 29ccfb1..31a6da5 100644 --- a/Source/WebCore/page/DOMWindow.cpp +++ b/Source/WebCore/page/DOMWindow.cpp @@ -83,6 +83,8 @@ #include "PlatformScreen.h" #include "ScheduledAction.h" #include "Screen.h" +#include "ScriptCallStack.h" +#include "ScriptCallStackFactory.h" #include "SecurityOrigin.h" #include "SerializedScriptValue.h" #include "Settings.h" @@ -123,13 +125,14 @@ namespace WebCore { class PostMessageTimer : public TimerBase { public: - PostMessageTimer(DOMWindow* window, PassRefPtr message, const String& sourceOrigin, PassRefPtr source, PassOwnPtr channels, SecurityOrigin* targetOrigin) + PostMessageTimer(DOMWindow* window, PassRefPtr message, const String& sourceOrigin, PassRefPtr source, PassOwnPtr channels, SecurityOrigin* targetOrigin, PassRefPtr stackTrace) : m_window(window) , m_message(message) , m_origin(sourceOrigin) , m_source(source) , m_channels(channels) , m_targetOrigin(targetOrigin) + , m_stackTrace(stackTrace) { } @@ -139,6 +142,7 @@ public: return MessageEvent::create(messagePorts.release(), m_message, m_origin, "", m_source); } SecurityOrigin* targetOrigin() const { return m_targetOrigin.get(); } + ScriptCallStack* stackTrace() const { return m_stackTrace.get(); } private: virtual void fired() @@ -153,6 +157,7 @@ private: RefPtr m_source; OwnPtr m_channels; RefPtr m_targetOrigin; + RefPtr m_stackTrace; }; typedef HashCountedSet DOMWindowSet; @@ -858,8 +863,13 @@ void DOMWindow::postMessage(PassRefPtr message, const Mes return; String sourceOrigin = sourceDocument->securityOrigin()->toString(); + // Capture stack trace only when inspector front-end is loaded as it may be time consuming. + RefPtr stackTrace; + if (InspectorInstrumentation::hasFrontends()) + stackTrace = createScriptCallStack(ScriptCallStack::maxCallStackSizeToCapture, true); + // Schedule the message. - PostMessageTimer* timer = new PostMessageTimer(this, message, sourceOrigin, source, channels.release(), target.get()); + PostMessageTimer* timer = new PostMessageTimer(this, message, sourceOrigin, source, channels.release(), target.get(), stackTrace.release()); timer->startOneShot(0); } @@ -883,7 +893,7 @@ void DOMWindow::postMessageTimerFired(PassOwnPtr t) if (!timer->targetOrigin()->isSameSchemeHostPort(document()->securityOrigin())) { String message = "Unable to post message to " + timer->targetOrigin()->toString() + ". Recipient has origin " + document()->securityOrigin()->toString() + ".\n"; - console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, message); + console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, message, timer->stackTrace()); return; } } @@ -1710,7 +1720,8 @@ void DOMWindow::printErrorMessage(const String& message) return; // FIXME: Add arguments so that we can provide a correct source URL and line number. - console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, message); + RefPtr stackTrace = createScriptCallStack(ScriptCallStack::maxCallStackSizeToCapture, true); + console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, message, stackTrace.release()); } String DOMWindow::crossDomainAccessErrorMessage(DOMWindow* activeWindow) -- 2.7.4