Skip to content

Commit

Permalink
inspector: fix request path nullptr dereference
Browse files Browse the repository at this point in the history
Fix a nullptr dereference when an invalid path is requested.

Regression introduced in commit 69fc85d ("inspector: generate UUID for
debug targets"), caught by Coverity.

PR-URL: #9184
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
bnoordhuis authored and evanlucas committed Nov 2, 2016
1 parent c231130 commit 2e7b078
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 12 deletions.
9 changes: 6 additions & 3 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -681,17 +681,20 @@ bool AgentImpl::RespondToGet(InspectorSocket* socket, const std::string& path) {

if (match_path_segment(command, "list") || command[0] == '\0') {
SendTargentsListResponse(socket);
return true;
} else if (match_path_segment(command, "protocol")) {
SendProtocolJson(socket);
return true;
} else if (match_path_segment(command, "version")) {
SendVersionResponse(socket);
} else {
const char* pid = match_path_segment(command, "activate");
return true;
} else if (const char* pid = match_path_segment(command, "activate")) {
if (pid != id_)
return false;
SendHttpResponse(socket, "Target activated");
return true;
}
return true;
return false;
}

// static
Expand Down
23 changes: 17 additions & 6 deletions test/inspector/inspector-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,17 @@ function checkHttpResponse(port, path, callback) {
res.setEncoding('utf8');
res
.on('data', (data) => response += data.toString())
.on('end', () => callback(JSON.parse(response)));
.on('end', () => {
let err = null;
let json = undefined;
try {
json = JSON.parse(response);
} catch (e) {
err = e;
err.response = response;
}
callback(err, json);
});
});
}

Expand Down Expand Up @@ -284,8 +294,8 @@ TestSession.prototype.disconnect = function(childDone) {

TestSession.prototype.testHttpResponse = function(path, check) {
return this.enqueue((callback) =>
checkHttpResponse(this.harness_.port, path, (response) => {
check.call(this, response);
checkHttpResponse(this.harness_.port, path, (err, response) => {
check.call(this, err, response);
callback();
}));
};
Expand Down Expand Up @@ -352,8 +362,8 @@ Harness.prototype.enqueue_ = function(task) {

Harness.prototype.testHttpResponse = function(path, check) {
return this.enqueue_((doneCallback) => {
checkHttpResponse(this.port, path, (response) => {
check.call(this, response);
checkHttpResponse(this.port, path, (err, response) => {
check.call(this, err, response);
doneCallback();
});
});
Expand Down Expand Up @@ -393,7 +403,8 @@ Harness.prototype.wsHandshake = function(devtoolsUrl, tests, readyCallback) {

Harness.prototype.runFrontendSession = function(tests) {
return this.enqueue_((callback) => {
checkHttpResponse(this.port, '/json/list', (response) => {
checkHttpResponse(this.port, '/json/list', (err, response) => {
assert.ifError(err);
this.wsHandshake(response[0]['webSocketDebuggerUrl'], tests, callback);
});
});
Expand Down
22 changes: 19 additions & 3 deletions test/inspector/test-inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,26 @@ const helper = require('./inspector-helper.js');

let scopeId;

function checkListResponse(response) {
function checkListResponse(err, response) {
assert.ifError(err);
assert.strictEqual(1, response.length);
assert.ok(response[0]['devtoolsFrontendUrl']);
assert.ok(
response[0]['webSocketDebuggerUrl']
.match(/ws:\/\/localhost:\d+\/[0-9A-Fa-f]{8}-/));
}

function checkVersion(err, response) {
assert.ifError(err);
assert.ok(response);
}

function checkBadPath(err, response) {
assert(err instanceof SyntaxError);
assert(/Unexpected token/.test(err.message));
assert(/WebSockets request was expected/.test(err.response));
}

function expectMainScriptSource(result) {
const expected = helper.mainScriptSource();
const source = result['scriptSource'];
Expand Down Expand Up @@ -153,7 +165,8 @@ function testInspectScope(session) {
}

function testNoUrlsWhenConnected(session) {
session.testHttpResponse('/json/list', (response) => {
session.testHttpResponse('/json/list', (err, response) => {
assert.ifError(err);
assert.strictEqual(1, response.length);
assert.ok(!response[0].hasOwnProperty('devtoolsFrontendUrl'));
assert.ok(!response[0].hasOwnProperty('webSocketDebuggerUrl'));
Expand All @@ -171,7 +184,10 @@ function runTests(harness) {
harness
.testHttpResponse('/json', checkListResponse)
.testHttpResponse('/json/list', checkListResponse)
.testHttpResponse('/json/version', assert.ok)
.testHttpResponse('/json/version', checkVersion)
.testHttpResponse('/json/activate', checkBadPath)
.testHttpResponse('/json/activate/boom', checkBadPath)
.testHttpResponse('/json/badpath', checkBadPath)
.runFrontendSession([
testNoUrlsWhenConnected,
testBreakpointOnStart,
Expand Down

0 comments on commit 2e7b078

Please sign in to comment.