FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

inspector: fix request path nullptr dereference · nodejs/node@2e7b078 · GitHub

/ node Public

Commit 2e7b078

Browse files
bnoordhuis
authored and
evanlucas
committed
inspector: fix request path nullptr dereference
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>
1 parent c231130 commit 2e7b078

3 files changed

Lines changed: 42 additions & 12 deletions

File tree

‎src/inspector_agent.cc‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -681,17 +681,20 @@ bool AgentImpl::RespondToGet(InspectorSocket* socket, const std::string& path) {
681681

682682
if (match_path_segment(command, "list") || command[0] == '\0') {
683683
SendTargentsListResponse(socket);
684+
return true;
684685
} else if (match_path_segment(command, "protocol")) {
685686
SendProtocolJson(socket);
687+
return true;
686688
} else if (match_path_segment(command, "version")) {
687689
SendVersionResponse(socket);
688-
} else {
689-
const char* pid = match_path_segment(command, "activate");
690+
return true;
691+
} else if (const char* pid = match_path_segment(command, "activate")) {
690692
if (pid != id_)
691693
return false;
692694
SendHttpResponse(socket, "Target activated");
695+
return true;
693696
}
694-
return true;
697+
return false;
695698
}
696699

697700
// static

‎test/inspector/inspector-helper.js‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,17 @@ function checkHttpResponse(port, path, callback) {
8686
res.setEncoding('utf8');
8787
res
8888
.on('data', (data) => response += data.toString())
89-
.on('end', () => callback(JSON.parse(response)));
89+
.on('end', () => {
90+
let err = null;
91+
let json = undefined;
92+
try {
93+
json = JSON.parse(response);
94+
} catch (e) {
95+
err = e;
96+
err.response = response;
97+
}
98+
callback(err, json);
99+
});
90100
});
91101
}
92102

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

285295
TestSession.prototype.testHttpResponse = function(path, check) {
286296
return this.enqueue((callback) =>
287-
checkHttpResponse(this.harness_.port, path, (response) => {
288-
check.call(this, response);
297+
checkHttpResponse(this.harness_.port, path, (err, response) => {
298+
check.call(this, err, response);
289299
callback();
290300
}));
291301
};
@@ -352,8 +362,8 @@ Harness.prototype.enqueue_ = function(task) {
352362

353363
Harness.prototype.testHttpResponse = function(path, check) {
354364
return this.enqueue_((doneCallback) => {
355-
checkHttpResponse(this.port, path, (response) => {
356-
check.call(this, response);
365+
checkHttpResponse(this.port, path, (err, response) => {
366+
check.call(this, err, response);
357367
doneCallback();
358368
});
359369
});
@@ -393,7 +403,8 @@ Harness.prototype.wsHandshake = function(devtoolsUrl, tests, readyCallback) {
393403

394404
Harness.prototype.runFrontendSession = function(tests) {
395405
return this.enqueue_((callback) => {
396-
checkHttpResponse(this.port, '/json/list', (response) => {
406+
checkHttpResponse(this.port, '/json/list', (err, response) => {
407+
assert.ifError(err);
397408
this.wsHandshake(response[0]['webSocketDebuggerUrl'], tests, callback);
398409
});
399410
});

‎test/inspector/test-inspector.js‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,26 @@ const helper = require('./inspector-helper.js');
55

66
let scopeId;
77

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

17+
function checkVersion(err, response) {
18+
assert.ifError(err);
19+
assert.ok(response);
20+
}
21+
22+
function checkBadPath(err, response) {
23+
assert(err instanceof SyntaxError);
24+
assert(/Unexpected token/.test(err.message));
25+
assert(/WebSockets request was expected/.test(err.response));
26+
}
27+
1628
function expectMainScriptSource(result) {
1729
const expected = helper.mainScriptSource();
1830
const source = result['scriptSource'];
@@ -153,7 +165,8 @@ function testInspectScope(session) {
153165
}
154166

155167
function testNoUrlsWhenConnected(session) {
156-
session.testHttpResponse('/json/list', (response) => {
168+
session.testHttpResponse('/json/list', (err, response) => {
169+
assert.ifError(err);
157170
assert.strictEqual(1, response.length);
158171
assert.ok(!response[0].hasOwnProperty('devtoolsFrontendUrl'));
159172
assert.ok(!response[0].hasOwnProperty('webSocketDebuggerUrl'));
@@ -171,7 +184,10 @@ function runTests(harness) {
171184
harness
172185
.testHttpResponse('/json', checkListResponse)
173186
.testHttpResponse('/json/list', checkListResponse)
174-
.testHttpResponse('/json/version', assert.ok)
187+
.testHttpResponse('/json/version', checkVersion)
188+
.testHttpResponse('/json/activate', checkBadPath)
189+
.testHttpResponse('/json/activate/boom', checkBadPath)
190+
.testHttpResponse('/json/badpath', checkBadPath)
175191
.runFrontendSession([
176192
testNoUrlsWhenConnected,
177193
testBreakpointOnStart,

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL