v8: make writeHeapSnapshot throw if fopen fails

Fixes: #41346
nodejs · Jan 1, 2022 · 1a3db37 · 1a3db37
1 parent a4795ad
commit 1a3db37
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 9 deletions.
diff --git a/src/env.cc b/src/env.cc
@@ -1643,7 +1643,7 @@ size_t Environment::NearHeapLimitCallback(void* data,
   env->isolate()->RemoveNearHeapLimitCallback(NearHeapLimitCallback,
                                               initial_heap_limit);
 
-  heap::WriteSnapshot(env->isolate(), filename.c_str());
+  heap::WriteSnapshot(env, filename.c_str());
   env->heap_limit_snapshot_taken_ += 1;
 
   // Don't take more snapshots than the number specified by

diff --git a/src/heap_utils.cc b/src/heap_utils.cc
@@ -308,20 +308,22 @@ class HeapSnapshotStream : public AsyncWrap,
   HeapSnapshotPointer snapshot_;
 };
 
-inline void TakeSnapshot(Isolate* isolate, v8::OutputStream* out) {
+inline void TakeSnapshot(Environment* env, v8::OutputStream* out) {
   HeapSnapshotPointer snapshot {
-      isolate->GetHeapProfiler()->TakeHeapSnapshot() };
+      env->isolate()->GetHeapProfiler()->TakeHeapSnapshot() };
   snapshot->Serialize(out, HeapSnapshot::kJSON);
 }
 
 }  // namespace
 
-bool WriteSnapshot(Isolate* isolate, const char* filename) {
+bool WriteSnapshot(Environment* env, const char* filename) {
   FILE* fp = fopen(filename, "w");
-  if (fp == nullptr)
+  if (fp == nullptr) {
+    env->ThrowErrnoException(errno, "fopen");
     return false;
+  }
   FileOutputStream stream(fp);
-  TakeSnapshot(isolate, &stream);
+  TakeSnapshot(env, &stream);
   fclose(fp);
   return true;
 }
@@ -374,7 +376,7 @@ void TriggerHeapSnapshot(const FunctionCallbackInfo<Value>& args) {
 
   if (filename_v->IsUndefined()) {
     DiagnosticFilename name(env, "Heap", "heapsnapshot");
-    if (!WriteSnapshot(isolate, *name))
+    if (!WriteSnapshot(env, *name))
       return;
     if (String::NewFromUtf8(isolate, *name).ToLocal(&filename_v)) {
       args.GetReturnValue().Set(filename_v);
@@ -384,7 +386,7 @@ void TriggerHeapSnapshot(const FunctionCallbackInfo<Value>& args) {
 
   BufferValue path(isolate, filename_v);
   CHECK_NOT_NULL(*path);
-  if (!WriteSnapshot(isolate, *path))
+  if (!WriteSnapshot(env, *path))
     return;
   return args.GetReturnValue().Set(filename_v);
 }

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -379,7 +379,7 @@ class DiagnosticFilename {
 };
 
 namespace heap {
-bool WriteSnapshot(v8::Isolate* isolate, const char* filename);
+bool WriteSnapshot(Environment* env, const char* filename);
 }
 
 class TraceEventScope {

diff --git a/test/sequential/test-heapdump.js b/test/sequential/test-heapdump.js
@@ -7,6 +7,7 @@ if (!common.isMainThread)
 
 const { writeHeapSnapshot, getHeapSnapshot } = require('v8');
 const assert = require('assert');
+const path = require('path');
 const fs = require('fs');
 const tmpdir = require('../common/tmpdir');
 
@@ -24,6 +25,18 @@ process.chdir(tmpdir.path);
   fs.accessSync(heapdump);
 }
 
+{
+  const readonlyFile = path.join(tmpdir.path, 'ro');
+  fs.writeFileSync(readonlyFile, Buffer.alloc(0), { mode: 0o444 });
+  assert.throws(() => {
+    writeHeapSnapshot(readonlyFile);
+  }, (e) => {
+    assert.ok(e, 'writeHeapSnapshot should error');
+    assert.strictEqual(e.code, 'EACCES');
+    return true;
+  });
+}
+
 [1, true, {}, [], null, Infinity, NaN].forEach((i) => {
   assert.throws(() => writeHeapSnapshot(i), {
     code: 'ERR_INVALID_ARG_TYPE',