Skip to content
This repository has been archived by the owner on Jun 18, 2021. It is now read-only.

Fix version reporting in NodeReport section #30

Closed
wants to merge 1 commit into from
Closed
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
3 changes: 3 additions & 0 deletions binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
"dll_files": [ "dbghelp.dll", "Netapi32.dll" ],
}],
],
"defines": [
'NODEREPORT_VERSION="<!(node -p \"require(\'./package.json\').version\")"'
],
},
{
"target_name": "install",
Expand Down
1 change: 1 addition & 0 deletions src/module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ void Initialize(v8::Local<v8::Object> exports) {
node_isolate = isolate;

SetLoadTime();
SetVersionString(isolate);

const char* verbose_switch = secure_getenv("NODEREPORT_VERBOSE");
if (verbose_switch != nullptr) {
Expand Down
119 changes: 110 additions & 9 deletions src/node_report.cc
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
#include "node_report.h"
#include "node_version.h"
#include "v8.h"
#include "time.h"
#include "zlib.h"
#include "ares.h"

#include <fcntl.h>
#include <map>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -35,6 +33,14 @@
#include <sys/utsname.h>
#endif

#if !defined(NODEREPORT_VERSION)
#define NODEREPORT_VERSION "dev"
#endif

#if !defined(UNKNOWN_NODEVERSION_STRING)
#define UNKNOWN_NODEVERSION_STRING "Unable to determine Node.js version\n"
#endif

#ifndef _WIN32
extern char** environ;
#endif
Expand All @@ -52,7 +58,7 @@ using v8::String;
using v8::V8;

// Internal/static function declarations
static void PrintVersionInformation(FILE* fp);
static void PrintVersionInformation(FILE* fp, Isolate* isolate);
static void PrintJavaScriptStack(FILE* fp, Isolate* isolate, DumpEvent event, const char* location);
static void PrintStackFromStackTrace(FILE* fp, Isolate* isolate, DumpEvent event);
static void PrintStackFrame(FILE* fp, Isolate* isolate, Local<StackFrame> frame, int index, void* pc);
Expand All @@ -70,6 +76,7 @@ const char* v8_states[] = {"JS", "GC", "COMPILER", "OTHER", "EXTERNAL", "IDLE"};
static bool report_active = false; // recursion protection
static char report_filename[NR_MAXNAME + 1] = "";
static char report_directory[NR_MAXPATH + 1] = ""; // defaults to current working directory
static std::string version_string = UNKNOWN_NODEVERSION_STRING;
#ifdef _WIN32
static SYSTEMTIME loadtime_tm_struct; // module load time
#else // UNIX, OSX
Expand Down Expand Up @@ -189,6 +196,97 @@ unsigned int ProcessNodeReportVerboseSwitch(const char* args) {
return 0; // Default is verbose mode off
}

void SetVersionString(Isolate* isolate) {
// Catch anything thrown and gracefully return
Nan::TryCatch trycatch;
version_string = UNKNOWN_NODEVERSION_STRING;

// Retrieve the process object
v8::Local<v8::String> process_prop;
if (!Nan::New<v8::String>("process").ToLocal(&process_prop)) return;
v8::Local<v8::Object> global_obj = isolate->GetCurrentContext()->Global();
v8::Local<v8::Value> process_value;
if (!Nan::Get(global_obj, process_prop).ToLocal(&process_value)) return;
if (!process_value->IsObject()) return;
v8::Local<v8::Object> process_obj = process_value.As<v8::Object>();
Copy link
Member

Choose a reason for hiding this comment

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

For the sake of robustness, you should if (!process_value->IsObject()) return; first.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


// Get process.version
v8::Local<v8::String> version_prop;
if (!Nan::New<v8::String>("version").ToLocal(&version_prop)) return;
v8::Local<v8::Value> version;
if (!Nan::Get(process_obj, version_prop).ToLocal(&version)) return;

// e.g. Node.js version: v6.9.1
if (version->IsString()) {
Nan::Utf8String node_version(version);
version_string = "Node.js version: ";
version_string += *node_version;
version_string += "\n";
}

// Get process.versions
v8::Local<v8::String> versions_prop;
if (!Nan::New<v8::String>("versions").ToLocal(&versions_prop)) return;
v8::Local<v8::Value> versions_value;
if (!Nan::Get(process_obj, versions_prop).ToLocal(&versions_value)) return;
if (!versions_value->IsObject()) return;
v8::Local<v8::Object> versions_obj = versions_value.As<v8::Object>();

// Get component names and versions from process.versions
v8::Local<v8::Array> components;
if (!Nan::GetOwnPropertyNames(versions_obj).ToLocal(&components)) return;
v8::Local<v8::Object> components_obj = components.As<v8::Object>();
std::map<std::string, std::string> comp_versions;
uint32_t total_components = (*components)->Length();
for (uint32_t i = 0; i < total_components; i++) {
v8::Local<v8::Value> name_value;
if (!Nan::Get(components_obj, i).ToLocal(&name_value)) continue;
v8::Local<v8::Value> version_value;
if (!Nan::Get(versions_obj, name_value).ToLocal(&version_value)) continue;

Nan::Utf8String component_name(name_value);
Nan::Utf8String component_version(version_value);
Copy link
Member

Choose a reason for hiding this comment

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

Robustness: if (*component_name == nullptr || *component_version == nullptr) continue;

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

if (*component_name == nullptr || *component_version == nullptr) continue;

// Don't duplicate the Node.js version
if (!strcmp("node", *component_name)) {
if (version_string == UNKNOWN_NODEVERSION_STRING) {
version_string = "Node.js version: v";
version_string += *component_version;
version_string += "\n";
}
} else {
comp_versions[*component_name] = *component_version;
}
}

// Format sorted component versions surrounded by (), wrapped
// e.g.
// (ares: 1.10.1-DEV, http_parser: 2.7.0, icu: 57.1, modules: 48,
// openssl: 1.0.2j, uv: 1.9.1, v8: 5.1.281.84, zlib: 1.2.8)
const size_t wrap = 80;
size_t line_length = 1;
version_string += "(";
for (auto it : comp_versions) {
std::string component_name = it.first;
std::string component_version = it.second;
size_t length = component_name.length() + component_version.length() + 2;
if (line_length + length + 1 >= wrap) {
version_string += "\n ";
line_length = 1;
}
version_string += component_name;
version_string += ": ";
version_string += component_version;
line_length += length;
if (it != *std::prev(comp_versions.end())) {
version_string += ", ";
line_length += 2;
}
}
version_string += ")\n";
}

/*******************************************************************************
* Function to save the nodereport module load time value
*******************************************************************************/
Expand Down Expand Up @@ -317,7 +415,7 @@ void TriggerNodeReport(Isolate* isolate, DumpEvent event, const char* message, c
fflush(fp);

// Print Node.js and OS version information
PrintVersionInformation(fp);
PrintVersionInformation(fp, isolate);
fflush(fp);

// Print summary JavaScript stack backtrace
Expand Down Expand Up @@ -369,12 +467,15 @@ void TriggerNodeReport(Isolate* isolate, DumpEvent event, const char* message, c
* Function to print Node.js version, OS version and machine information
*
******************************************************************************/
static void PrintVersionInformation(FILE* fp) {
static void PrintVersionInformation(FILE* fp, Isolate* isolate) {

// Print Node.js and deps component versions
fprintf(fp, "\nNode.js version: %s\n", NODE_VERSION);
fprintf(fp, "(v8: %s, libuv: %s, zlib: %s, ares: %s)\n",
V8::GetVersion(), uv_version_string(), ZLIB_VERSION, ARES_VERSION_STR);
fprintf(fp, "\n%s", version_string.c_str());

// Print NodeReport version
// e.g. NodeReport version: 1.0.6 (built against Node.js v6.9.1)
fprintf(fp, "\nNodeReport version: %s (built against Node.js v%s)\n",
NODEREPORT_VERSION, NODE_VERSION_STRING);

// Print operating system and machine information (Windows)
#ifdef _WIN32
Expand Down
1 change: 1 addition & 0 deletions src/node_report.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ void ProcessNodeReportDirectory(const char* args);
unsigned int ProcessNodeReportVerboseSwitch(const char* args);

void SetLoadTime();
void SetVersionString(Isolate* isolate);

// Local implementation of secure_getenv()
inline const char* secure_getenv(const char* key) {
Expand Down
54 changes: 48 additions & 6 deletions test/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const REPORT_SECTIONS = [
'System Information'
];

const reNewline = '(?:\\r*\\n)';

exports.findReports = (pid) => {
// Default filenames are of the form NodeReport.<date>.<time>.<pid>.<seq>.txt
const format = '^NodeReport\\.\\d+\\.\\d+\\.' + pid + '\\.\\d+\\.txt$';
Expand All @@ -25,20 +27,60 @@ exports.isWindows = () => {
return process.platform === 'win32';
};

exports.validate = (t, report, pid) => {
exports.validate = (t, report, options) => {
t.test('Validating ' + report, (t) => {
fs.readFile(report, (err, data) => {
const pid = options ? options.pid : process.pid;
const reportContents = data.toString();
const plan = REPORT_SECTIONS.length + (pid ? 1 : 0);
const nodeComponents = Object.keys(process.versions);
const expectedVersions = options ?
options.expectedVersions || nodeComponents :
nodeComponents;
const plan = REPORT_SECTIONS.length + nodeComponents.length + 2;
t.plan(plan);
if (pid) {
t.match(reportContents, new RegExp('Process ID: ' + pid),
'Checking report contains expected process ID ' + pid);
}

// Check all sections are present
REPORT_SECTIONS.forEach((section) => {
t.match(reportContents, new RegExp('==== ' + section),
'Checking report contains ' + section + ' section');
});

// Check NodeReport section
const nodeReportSection = getSection(reportContents, 'NodeReport');
t.match(nodeReportSection, new RegExp('Process ID: ' + pid),
'NodeReport section contains expected process ID');
if (options && options.expectNodeVersion === false) {
t.match(nodeReportSection, /Unable to determine Node.js version/,
'NodeReport section contains expected Node.js version');
} else {
t.match(nodeReportSection,
new RegExp('Node.js version: ' + process.version),
'NodeReport section contains expected Node.js version');
}
nodeComponents.forEach((c) => {
if (c !== 'node') {
if (expectedVersions.indexOf(c) === -1) {
Copy link
Member

Choose a reason for hiding this comment

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

Might as well use .includes() here.

Copy link
Member Author

Choose a reason for hiding this comment

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

NodeReport claims to be Node.js v4.x compatible ("node": ">=4.0.0") and the version of V8 in v4.x doesn't have Array.prototype.includes().

t.notMatch(nodeReportSection,
new RegExp(c + ': ' + process.versions[c]),
'NodeReport section does not contain ' + c + ' version');
} else {
t.match(nodeReportSection,
new RegExp(c + ': ' + process.versions[c]),
'NodeReport section contains expected ' + c + ' version');
}
}
});
const nodereportMetadata = require('../package.json');
t.match(nodeReportSection,
new RegExp('NodeReport version: ' + nodereportMetadata.version),
'NodeReport section contains expected NodeReport version');
});
});
};

const getSection = (report, section) => {
const re = new RegExp('==== ' + section + ' =+' + reNewline + '+([\\S\\s]+?)'
+ reNewline + '+={80}' + reNewline);
const match = re.exec(report);
return match ? match[1] : '';
};
24 changes: 24 additions & 0 deletions test/test-api-bad-processobj.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

// Testcase to check NodeReport succeeds if process.versions is damaged
if (process.argv[2] === 'child') {
// Tamper with the process object
Object.defineProperty(process, 'versions', {get() { throw 'boom'; }});
const nodereport = require('../');
nodereport.triggerReport();
} else {
const common = require('./common.js');
const spawn = require('child_process').spawn;
const tap = require('tap');

const child = spawn(process.execPath, [__filename, 'child']);
child.on('exit', (code) => {
tap.plan(3);
tap.equal(code, 0, 'Process exited cleanly');
const reports = common.findReports(child.pid);
tap.equal(reports.length, 1, 'Found reports ' + reports);
const report = reports[0];
const validateOpts = { pid: child.pid, expectedVersions: [] };
common.validate(tap, report, validateOpts);
});
}
24 changes: 24 additions & 0 deletions test/test-api-bad-processversion.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

// Testcase to check NodeReport succeeds if process.version is damaged
if (process.argv[2] === 'child') {
// Tamper with the process object
delete process['version'];
const nodereport = require('../');
nodereport.triggerReport();
} else {
const common = require('./common.js');
const spawn = require('child_process').spawn;
const tap = require('tap');

const child = spawn(process.execPath, [__filename, 'child']);
child.on('exit', (code) => {
tap.plan(3);
tap.equal(code, 0, 'Process exited cleanly');
const reports = common.findReports(child.pid);
tap.equal(reports.length, 1, 'Found reports ' + reports);
const report = reports[0];
const validateOpts = { pid: child.pid, expectNodeVersion: true };
common.validate(tap, report, validateOpts);
});
}
26 changes: 26 additions & 0 deletions test/test-api-bad-processversions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';

// Testcase to check NodeReport succeeds if part of process.versions is damaged
if (process.argv[2] === 'child') {
// Tamper with the process object
Object.defineProperty(process.versions, 'uv', {get() { throw 'boom'; }});
const nodereport = require('../');
nodereport.triggerReport();
} else {
const common = require('./common.js');
const spawn = require('child_process').spawn;
const tap = require('tap');

const child = spawn(process.execPath, [__filename, 'child']);
child.on('exit', (code) => {
tap.plan(3);
tap.equal(code, 0, 'Process exited cleanly');
const reports = common.findReports(child.pid);
tap.equal(reports.length, 1, 'Found reports ' + reports);
const report = reports[0];
const validateOpts = { pid: child.pid,
expectedVersions: Object.keys(process.versions).filter((c) => c !== 'uv')
};
common.validate(tap, report, validateOpts);
});
}
28 changes: 28 additions & 0 deletions test/test-api-noversioninfo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
'use strict';

// Testcase to check NodeReport succeeds if both process.version and
// process.versions are damaged
if (process.argv[2] === 'child') {
// Tamper with the process object
delete process['version'];
delete process['versions'];

const nodereport = require('../');
nodereport.triggerReport();
} else {
const common = require('./common.js');
const spawn = require('child_process').spawn;
const tap = require('tap');

const child = spawn(process.execPath, [__filename, 'child']);
child.on('exit', (code) => {
tap.plan(3);
tap.equal(code, 0, 'Process exited cleanly');
const reports = common.findReports(child.pid);
tap.equal(reports.length, 1, 'Found reports ' + reports);
const report = reports[0];
const validateOpts = { pid: child.pid, expectNodeVersion: false,
expectedVersions: [] };
common.validate(tap, report, validateOpts);
});
}
2 changes: 1 addition & 1 deletion test/test-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ if (process.argv[2] === 'child') {
const reports = common.findReports(child.pid);
tap.equal(reports.length, 1, 'Found reports ' + reports);
const report = reports[0];
common.validate(tap, report, child.pid);
common.validate(tap, report, {pid: child.pid});
});
}
2 changes: 1 addition & 1 deletion test/test-exception.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ if (process.argv[2] === 'child') {
const reports = common.findReports(child.pid);
tap.equal(reports.length, 1, 'Found reports ' + reports);
const report = reports[0];
common.validate(tap, report, child.pid);
common.validate(tap, report, {pid: child.pid});
});
}
Loading