Report Javascript Runtime Errors to the Error Console

extensions/browser/extension_error.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/browser/extension_error.h"
 
-#include "base/json/json_reader.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "extensions/common/constants.h"
+#include "third_party/re2/re2/re2.h"
 #include "url/gurl.h"
 
 using base::string16;
 
 namespace extensions {
 
 namespace {
 
 const char kLineNumberKey[] = "lineNumber";
 const char kColumnNumberKey[] = "columnNumber";
 const char kURLKey[] = "url";
 const char kFunctionNameKey[] = "functionName";
 const char kExecutionContextURLKey[] = "executionContextURL";
 const char kStackTraceKey[] = "stackTrace";
+const char kStackFrameDelimiter[] = "\n    at ";
+const char kAnonymousFunction[] = "(anonymous function)";
 
 // Try to retrieve an extension ID from a |url|. On success, returns true and
 // populates |extension_id| with the ID. On failure, returns false and leaves
 // extension_id untouched.
 bool GetExtensionIDFromGURL(const GURL& url, std::string* extension_id) {
   if (url.SchemeIs(kExtensionScheme)) {
     *extension_id = url.host();
     return true;
   }
   return false;
@@ skipping 34 matching lines @@
 }
 
 ManifestParsingError::~ManifestParsingError() {
 }
 
 std::string ManifestParsingError::PrintForTest() const {
   return ExtensionError::PrintForTest() +
          "\n  Type:    ManifestParsingError";
 }
 
-JavascriptRuntimeError::StackFrame::StackFrame() : line_number(-1),
-                                                   column_number(-1) {
-}
-
 JavascriptRuntimeError::StackFrame::StackFrame(size_t frame_line,
                                                size_t frame_column,
                                                const string16& frame_url,
                                                const string16& frame_function)
     : line_number(frame_line),
       column_number(frame_column),
       url(frame_url),
       function(frame_function) {
 }
 
 JavascriptRuntimeError::StackFrame::~StackFrame() {
 }
 
+// Create a stack frame from the passed text. The text must follow one of two
+// formats:
+//   - "function_name (source:line_number:column_number)"
+//   - "source:line_number:column_number"
+// (We have to recognize two formats because V8 will report stack traces in
+// both ways. If we reconcile this, we can clean this up.)
+scoped_ptr<JavascriptRuntimeError::StackFrame>
+JavascriptRuntimeError::StackFrame::CreateFromText(const string16& utf16_text) {
+  // We need to use utf8 for re2 matching.
+  std::string text = base::UTF16ToUTF8(utf16_text);
+
+  size_t line = -1;
+  size_t column = -1;
+  std::string url;
+  std::string function;
+  if (!re2::RE2::FullMatch(text,

[Cris Neckar, 2013/08/27 19:10:47]

I don't much like that we are exposing the entire attack surface of re2 directly
to attack here. Why aren't you simply passing the StackFrame object across IPC?

[Devlin, 2013/08/27 19:52:48]

So, to confirm, you would prefer to do this work on the renderer side, and
introduce a content/public/common/stack_frame object? (If so, I can certainly do
that - just wanna make sure I understand fully).

[Devlin, 2013/08/27 23:51:33]

Done.

[jam, 2013/08/29 16:10:32]

I'm not familiar with re2, but I hear that Justin is.

Justin: can we get a second opinion on how much of a threat this is? i.e. to
avoid the extra complexity of doing this in the renderer.

+                           "(.+) \\(([^\\(\\)]+):(\\d+):(\\d+)\\)",
+                           &function, &url, &line, &column) &&
+      !re2::RE2::FullMatch(text,
+                           "([^\\(\\)]+):(\\d+):(\\d+)",
+                           &url, &line, &column)) {
+    return scoped_ptr<StackFrame>();
+  }
+
+  if (function.empty())
+    function = kAnonymousFunction;
+
+  return scoped_ptr<StackFrame>(
+      new StackFrame(
+          line, column, base::UTF8ToUTF16(url), base::UTF8ToUTF16(function)));
+}
+
 JavascriptRuntimeError::JavascriptRuntimeError(bool from_incognito,
                                                const string16& source,
                                                const string16& message,
-                                               logging::LogSeverity level,
-                                               const string16& details)
+                                               const string16& stack_trace,
+                                               int32 line_number,
+                                               const GURL& context_url,
+                                               logging::LogSeverity level)
     : ExtensionError(ExtensionError::JAVASCRIPT_RUNTIME_ERROR,
-                     std::string(),  // We don't know the id yet.
+                     GURL(source).host(),
                      from_incognito,
                      source,
                      message),
+      context_url_(context_url),
       level_(level) {
-  ParseDetails(details);
-  DetermineExtensionID();
+  GetStackTrace(message, stack_trace, line_number);
+  CleanUpInit();
 }
 
 JavascriptRuntimeError::~JavascriptRuntimeError() {
 }
 
 std::string JavascriptRuntimeError::PrintForTest() const {
   std::string result = ExtensionError::PrintForTest() +
          "\n  Type:    JavascriptRuntimeError"
-         "\n  Context: " + base::UTF16ToUTF8(execution_context_url_) +
+         "\n  Context: " + context_url_.spec() +
          "\n  Stack Trace: ";
   for (StackTrace::const_iterator iter = stack_trace_.begin();
        iter != stack_trace_.end(); ++iter) {
     result += "\n    {"
-              "\n      Line:     " + base::IntToString(iter->line_number) +
-              "\n      Column:   " + base::IntToString(iter->column_number) +
-              "\n      URL:      " + base::UTF16ToUTF8(iter->url) +
-              "\n      Function: " + base::UTF16ToUTF8(iter->function) +
+              "\n      Line:     " + base::IntToString((*iter)->line_number) +
+              "\n      Column:   " + base::IntToString((*iter)->column_number) +
+              "\n      URL:      " + base::UTF16ToUTF8((*iter)->url) +
+              "\n      Function: " + base::UTF16ToUTF8((*iter)->function) +
               "\n    }";
   }
   return result;
 }
 
-void JavascriptRuntimeError::ParseDetails(const string16& details) {
-  scoped_ptr<base::Value> value(
-      base::JSONReader::Read(base::UTF16ToUTF8(details)));
-  const base::DictionaryValue* details_value;
-  const base::ListValue* trace_value = NULL;
+void JavascriptRuntimeError::GetStackTrace(const string16& message,
+                                           const string16& stack_trace,
+                                           int32 line_number) {
+  std::vector<string16> pieces;
+  size_t index = 0;
 
-  // The |details| value should contain an execution context url and a stack
-  // trace.
-  if (!value.get() ||
-      !value->GetAsDictionary(&details_value) ||
-      !details_value->GetString(kExecutionContextURLKey,
-                                &execution_context_url_) ||
-      !details_value->GetList(kStackTraceKey, &trace_value)) {
-    NOTREACHED();
-    return;
+  // There are two possible scenarios:
+  // 1. WebKit gives us a stack trace in |stack_trace|.
+  // 2. The stack trace is embedded in the error |message| by the extension
+  //    system (such as in the evaluation of a browser action). This will be
+  //    more useful than |stack_trace|, since |stack_trace| will include the
+  //    internal bindings trace, instead of a developer's code.
+  if (message.find(base::UTF8ToUTF16(kStackFrameDelimiter)) != string16::npos) {
+    base::SplitStringUsingSubstr(message,
+                                 base::UTF8ToUTF16(kStackFrameDelimiter),
+                                 &pieces);
+    message_ = pieces[0];
+    index = 1;
+  } else if (!stack_trace.empty()) {
+    base::SplitStringUsingSubstr(stack_trace,
+                                 base::UTF8ToUTF16(kStackFrameDelimiter),
+                                 &pieces);
   }
 
-  int line = 0;
-  int column = 0;
-  string16 url;
-
-  for (size_t i = 0; i < trace_value->GetSize(); ++i) {
-    const base::DictionaryValue* frame_value = NULL;
-    CHECK(trace_value->GetDictionary(i, &frame_value));
-
-    frame_value->GetInteger(kLineNumberKey, &line);
-    frame_value->GetInteger(kColumnNumberKey, &column);
-    frame_value->GetString(kURLKey, &url);
-
-    string16 function;
-    frame_value->GetString(kFunctionNameKey, &function);  // This can be empty.
-    stack_trace_.push_back(StackFrame(line, column, url, function));
+  // If we got a stack trace, parse each frame from the text.
+  if (index < pieces.size()) {
+    for (; index < pieces.size(); ++index) {
+      scoped_ptr<StackFrame> frame = StackFrame::CreateFromText(pieces[index]);
+      if (frame.get())
+        stack_trace_.push_back(frame.release());
+    }
+  } else {  // Otherwise, mock one up from the given line/url.
+    stack_trace_.push_back(new StackFrame(line_number,

[Cris Neckar, 2013/08/27 19:10:47]

ditto for this vector.. Having this complexity on the browser side seems wrong.

[Devlin, 2013/08/27 23:51:33]

Done.

+                                          1,  // column number
+                                          source_,
+                                          UTF8ToUTF16(kAnonymousFunction)));
   }
 }
 
-void JavascriptRuntimeError::DetermineExtensionID() {
-  if (!GetExtensionIDFromGURL(GURL(source_), &extension_id_))
-    GetExtensionIDFromGURL(GURL(execution_context_url_), &extension_id_);
+void JavascriptRuntimeError::CleanUpInit() {
+  // If the error came from a generated background page, the "context" is empty
+  // because there's no visible URL. We should set context to be the generated
+  // background page in this case.
+  GURL source_url = GURL(source_);
+  if (context_url_.is_empty() &&
+      source_url.path() ==
+          std::string("/") + kGeneratedBackgroundPageFilename) {
+    context_url_ = source_url;
+  }
+
+  // In some instances (due to the fact that we're reusing error reporting from
+  // other systems), the source won't match up with the final entry in the stack
+  // trace. (For instance, in a browser action error, the source is the page -
+  // sometimes the background page - but the error is thrown from the script.)
+  // Make the source match the stack trace, since that is more likely the cause
+  // of the error.
+  if (!stack_trace_.empty() && source_ != stack_trace_[0]->url)
+    source_ = stack_trace_[0]->url;
 }
 
 }  // namespace extensions