From 10fa1f5a67e2dc757ee63b2e46d2742a9515942c Mon Sep 17 00:00:00 2001
From: Richard Lau <riclau@uk.ibm.com>
Date: Thu, 24 Nov 2016 14:57:45 -0500
Subject: [PATCH] Fix version reporting in NodeReport section

Report the versions of Node.js and its components based on the runtime
and not compile time constants. Report NodeReport version and the
version of Node.js it was built against.

Extend the tests to validate the versions reported in the report.

Fixes: nodejs/nodereport#29
---
 binding.gyp                          |   3 +
 src/module.cc                        |   1 +
 src/node_report.cc                   | 119 +++++++++++++++++++++++++--
 src/node_report.h                    |   1 +
 test/common.js                       |  54 ++++++++++--
 test/test-api-bad-processobj.js      |  24 ++++++
 test/test-api-bad-processversion.js  |  24 ++++++
 test/test-api-bad-processversions.js |  26 ++++++
 test/test-api-noversioninfo.js       |  28 +++++++
 test/test-api.js                     |   2 +-
 test/test-exception.js               |   2 +-
 test/test-fatal-error.js             |   2 +-
 test/test-signal.js                  |   2 +-
 13 files changed, 269 insertions(+), 19 deletions(-)
 create mode 100644 test/test-api-bad-processobj.js
 create mode 100644 test/test-api-bad-processversion.js
 create mode 100644 test/test-api-bad-processversions.js
 create mode 100644 test/test-api-noversioninfo.js

diff --git a/binding.gyp b/binding.gyp
index 3d0be4e..c665e5b 100644
--- a/binding.gyp
+++ b/binding.gyp
@@ -14,6 +14,9 @@
           "dll_files": [ "dbghelp.dll", "Netapi32.dll" ],
         }],
       ],
+      "defines": [
+        'NODEREPORT_VERSION="<!(node -p \"require(\'./package.json\').version\")"'
+      ],
     },
     {
       "target_name": "install",
diff --git a/src/module.cc b/src/module.cc
index 984a86b..e9de130 100644
--- a/src/module.cc
+++ b/src/module.cc
@@ -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) {
diff --git a/src/node_report.cc b/src/node_report.cc
index a54d8ec..bb20b02 100644
--- a/src/node_report.cc
+++ b/src/node_report.cc
@@ -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>
@@ -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
@@ -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);
@@ -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
@@ -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>();
+
+  // 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);
+    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
  *******************************************************************************/
@@ -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
@@ -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
diff --git a/src/node_report.h b/src/node_report.h
index 624b2b1..210669e 100644
--- a/src/node_report.h
+++ b/src/node_report.h
@@ -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) {
diff --git a/test/common.js b/test/common.js
index e50fd33..bab9cf8 100644
--- a/test/common.js
+++ b/test/common.js
@@ -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$';
@@ -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) {
+            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] : '';
+};
diff --git a/test/test-api-bad-processobj.js b/test/test-api-bad-processobj.js
new file mode 100644
index 0000000..4d32941
--- /dev/null
+++ b/test/test-api-bad-processobj.js
@@ -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);
+  });
+}
diff --git a/test/test-api-bad-processversion.js b/test/test-api-bad-processversion.js
new file mode 100644
index 0000000..a84d9cb
--- /dev/null
+++ b/test/test-api-bad-processversion.js
@@ -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);
+  });
+}
diff --git a/test/test-api-bad-processversions.js b/test/test-api-bad-processversions.js
new file mode 100644
index 0000000..a1099a1
--- /dev/null
+++ b/test/test-api-bad-processversions.js
@@ -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);
+  });
+}
diff --git a/test/test-api-noversioninfo.js b/test/test-api-noversioninfo.js
new file mode 100644
index 0000000..717eb80
--- /dev/null
+++ b/test/test-api-noversioninfo.js
@@ -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);
+  });
+}
diff --git a/test/test-api.js b/test/test-api.js
index f254707..98f365c 100644
--- a/test/test-api.js
+++ b/test/test-api.js
@@ -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});
   });
 }
diff --git a/test/test-exception.js b/test/test-exception.js
index 6966c00..6e0e2f2 100644
--- a/test/test-exception.js
+++ b/test/test-exception.js
@@ -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});
   });
 }
diff --git a/test/test-fatal-error.js b/test/test-fatal-error.js
index 66238eb..038aa83 100644
--- a/test/test-fatal-error.js
+++ b/test/test-fatal-error.js
@@ -28,6 +28,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});
   });
 }
diff --git a/test/test-signal.js b/test/test-signal.js
index 183dbb3..c1fb7fa 100644
--- a/test/test-signal.js
+++ b/test/test-signal.js
@@ -59,6 +59,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});
   });
 }