Skip to content

src: add /json/protocol endpoint to inspector #7491

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions deps/zlib/zlib.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
{
'target_name': 'zlib',
'type': 'static_library',
'defines': [ 'ZLIB_CONST' ],
'sources': [
'adler32.c',
'compress.c',
Expand Down Expand Up @@ -44,6 +45,7 @@
'.',
],
'direct_dependent_settings': {
'defines': [ 'ZLIB_CONST' ],
'include_dirs': [
'.',
],
Expand Down Expand Up @@ -72,10 +74,12 @@
'direct_dependent_settings': {
'defines': [
'USE_SYSTEM_ZLIB',
'ZLIB_CONST',
],
},
'defines': [
'USE_SYSTEM_ZLIB',
'ZLIB_CONST',
],
'link_settings': {
'libraries': [
Expand Down
29 changes: 29 additions & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@
'dependencies': [
'deps/v8_inspector/third_party/v8_inspector/platform/'
'v8_inspector/v8_inspector.gyp:v8_inspector_stl',
'v8_inspector_compress_protocol_json#host',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is there a reason for the inconsistent indent between this and the two lines above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous line is a continuation of the one before it. Note how the first line doesn't have a comma at the end.

],
'include_dirs': [
'deps/v8_inspector/third_party/v8_inspector',
Expand Down Expand Up @@ -651,6 +652,34 @@
} ]
]
},
{
'target_name': 'v8_inspector_compress_protocol_json',
'type': 'none',
'toolsets': ['host'],
'conditions': [
[ 'v8_inspector=="true"', {
'actions': [
{
'action_name': 'v8_inspector_compress_protocol_json',
'process_outputs_as_sources': 1,
'inputs': [
'deps/v8_inspector/third_party/'
'v8_inspector/platform/v8_inspector/js_protocol.json',
],
'outputs': [
'<(SHARED_INTERMEDIATE_DIR)/v8_inspector_protocol_json.h',
],
'action': [
'python',
'tools/compress_json.py',
'<@(_inputs)',
'<@(_outputs)',
],
},
],
}],
],
},
{
'target_name': 'node_js2c',
'type': 'none',
Expand Down
28 changes: 28 additions & 0 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "node_version.h"
#include "v8-platform.h"
#include "util.h"
#include "zlib.h"

#include "platform/v8_inspector/public/InspectorVersion.h"
#include "platform/v8_inspector/public/V8Inspector.h"
Expand Down Expand Up @@ -41,6 +42,10 @@ const char TAG_DISCONNECT[] = "#disconnect";
const char DEVTOOLS_PATH[] = "/node";
const char DEVTOOLS_HASH[] = V8_INSPECTOR_REVISION;

static const uint8_t PROTOCOL_JSON[] = {
#include "v8_inspector_protocol_json.h" // NOLINT(build/include_order)
};

void PrintDebuggerReadyMessage(int port) {
fprintf(stderr, "Debugger listening on port %d.\n"
"Warning: This is an experimental feature and could change at any time.\n"
Expand Down Expand Up @@ -161,6 +166,27 @@ void SendTargentsListResponse(InspectorSocket* socket,
SendHttpResponse(socket, buffer.data(), len);
}

void SendProtocolJson(InspectorSocket* socket) {
z_stream strm;
strm.zalloc = Z_NULL;
strm.zfree = Z_NULL;
strm.opaque = Z_NULL;
CHECK_EQ(Z_OK, inflateInit(&strm));
static const size_t kDecompressedSize =
PROTOCOL_JSON[0] * 0x10000u +
PROTOCOL_JSON[1] * 0x100u +
PROTOCOL_JSON[2];
strm.next_in = PROTOCOL_JSON + 3;
strm.avail_in = sizeof(PROTOCOL_JSON) - 3;
std::vector<char> data(kDecompressedSize);
strm.next_out = reinterpret_cast<Byte*>(&data[0]);
strm.avail_out = data.size();
CHECK_EQ(Z_STREAM_END, inflate(&strm, Z_FINISH));
CHECK_EQ(0, strm.avail_out);
CHECK_EQ(Z_OK, inflateEnd(&strm));
SendHttpResponse(socket, &data[0], data.size());
}

const char* match_path_segment(const char* path, const char* expected) {
size_t len = strlen(expected);
if (StringEqualNoCaseN(path, expected, len)) {
Expand All @@ -179,6 +205,8 @@ bool RespondToGet(InspectorSocket* socket, const std::string& script_name_,

if (match_path_segment(command, "list") || command[0] == '\0') {
SendTargentsListResponse(socket, script_name_, script_path_, port);
} else if (match_path_segment(command, "protocol")) {
SendProtocolJson(socket);
} else if (match_path_segment(command, "version")) {
SendVersionResponse(socket);
} else {
Expand Down
1 change: 1 addition & 0 deletions test/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ exports.testDir = __dirname;
exports.fixturesDir = path.join(exports.testDir, 'fixtures');
exports.libDir = path.join(exports.testDir, '../lib');
exports.tmpDirName = 'tmp';
// PORT should match the definition in test/testpy/__init__.py.
exports.PORT = +process.env.NODE_COMMON_PORT || 12346;
exports.isWindows = process.platform === 'win32';
exports.isWOW64 = exports.isWindows &&
Expand Down
22 changes: 22 additions & 0 deletions test/parallel/test-v8-inspector-json-protocol.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Flags: --inspect={PORT}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had not seen this syntax before... specifically not with the {PORT} syntax (no other examples in test).

Is this documented anywhere? Should it be common.PORT?

I'm assuming I am wrong here, just curious

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm... I am seeing that it is included below. sorry for noise

'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');

const options = {
path: '/json/protocol',
port: common.PORT,
host: common.localhostIPv4,
};

http.get(options, common.mustCall((res) => {
let body = '';
res.setEncoding('utf8');
res.on('data', (data) => body += data);
res.on('end', common.mustCall(() => {
assert(body.length > 0);
assert.deepStrictEqual(JSON.stringify(JSON.parse(body)), body);
}));
}));
5 changes: 4 additions & 1 deletion test/testpy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ def GetCommand(self):
source = open(self.file).read()
flags_match = FLAGS_PATTERN.search(source)
if flags_match:
result += flags_match.group(1).strip().split()
# PORT should match the definition in test/common.js.
env = { 'PORT': int(os.getenv('NODE_COMMON_PORT', '12346')) }
env['PORT'] += self.thread_id * 100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding thread_id * 100 to the port number and requiring the port to match common.PORT seem to be kind of contradictory?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't make it up, exports.PORT += process.env.TEST_THREAD_ID * 100; is what test/common.js does.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, didn’t see that value is changed later in test/common.js.

result += flags_match.group(1).strip().format(**env).split()
files_match = FILES_PATTERN.search(source);
additional_files = []
if files_match:
Expand Down
25 changes: 25 additions & 0 deletions tools/compress_json.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/usr/bin/env python

import json
import struct
import sys
import zlib

if __name__ == '__main__':
fp = open(sys.argv[1])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if important for this kind of script, but fp is not explicitly closed. Since we depend on 2.6-2.7, maybe use a with statement instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script is short-lived so I went for simplicity.

obj = json.load(fp)
text = json.dumps(obj, separators=(',', ':'))
data = zlib.compress(text, zlib.Z_BEST_COMPRESSION)

# To make decompression a little easier, we prepend the compressed data
# with the size of the uncompressed data as a 24 bits BE unsigned integer.
assert len(text) < 1 << 24, 'Uncompressed JSON must be < 16 MB.'
data = struct.pack('>I', len(text))[1:4] + data

step = 20
slices = (data[i:i+step] for i in xrange(0, len(data), step))
slices = map(lambda s: ','.join(str(ord(c)) for c in s), slices)
text = ',\n'.join(slices)

fp = open(sys.argv[2], 'w')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

fp.write(text)