Merge pull request #9322 from electron/debugger-json-crash

Parse debugger protocol messages using v8 JSON parser

atom/browser/api/atom_api_debugger.cc

@@ -9,7 +9,6 @@
 #include "atom/browser/atom_browser_main_parts.h"
 #include "atom/common/native_mate_converters/callback.h"
 #include "atom/common/native_mate_converters/value_converter.h"
-#include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/web_contents.h"
@@ -45,12 +44,23 @@ void Debugger::DispatchProtocolMessage(DevToolsAgentHost* agent_host,
                                        const std::string& message) {
   DCHECK(agent_host == agent_host_.get());
 
-  std::unique_ptr<base::Value> parsed_message(base::JSONReader::Read(message));
-  if (!parsed_message->IsType(base::Value::TYPE_DICTIONARY))
-    return;
+  v8::Locker locker(isolate());
+  v8::HandleScope handle_scope(isolate());
+
+  v8::Local<v8::String> local_message =
+      v8::String::NewFromUtf8(isolate(), message.data());
+  v8::MaybeLocal<v8::Value> parsed_message = v8::JSON::Parse(
+      isolate()->GetCurrentContext(), local_message);
+  if (parsed_message.IsEmpty()) {
+    return;
+  }
+
+  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
+  if (!mate::ConvertFromV8(isolate(), parsed_message.ToLocalChecked(),
+                           dict.get())) {
+    return;
+  }
 
-  base::DictionaryValue* dict =
-      static_cast<base::DictionaryValue*>(parsed_message.get());
   int id;
   if (!dict->GetInteger("id", &id)) {
     std::string method;

spec/api-debugger-spec.js

@@ -1,4 +1,5 @@
 const assert = require('assert')
+const http = require('http')
 const path = require('path')
 const {closeWindow} = require('./window-helpers')
 const BrowserWindow = require('electron').remote.BrowserWindow
@@ -70,6 +71,15 @@ describe('debugger module', function () {
   })
 
   describe('debugger.sendCommand', function () {
+    let server
+
+    afterEach(function () {
+      if (server != null) {
+        server.close()
+        server = null
+      }
+    })
+
     it('retuns response', function (done) {
       w.webContents.loadURL('about:blank')
       try {
@@ -125,5 +135,33 @@ describe('debugger module', function () {
         done()
       })
     })
+
+    it('handles invalid unicode characters in message', function (done) {
+      try {
+        w.webContents.debugger.attach()
+      } catch (err) {
+        done('unexpected error : ' + err)
+      }
+
+      w.webContents.debugger.on('message', (event, method, params) => {
+        if (method === 'Network.loadingFinished') {
+          w.webContents.debugger.sendCommand('Network.getResponseBody', {
+            requestId: params.requestId
+          }, () => {
+            done()
+          })
+        }
+      })
+
+      server = http.createServer((req, res) => {
+        res.setHeader('Content-Type', 'text/plain; charset=utf-8')
+        res.end('\uFFFF')
+      })
+
+      server.listen(0, '127.0.0.1', () => {
+        w.webContents.debugger.sendCommand('Network.enable')
+        w.loadURL(`http://127.0.0.1:${server.address().port}`)
+      })
+    })
   })
 })