fs: fix stack overflow in fs.readdirSync

Previously, fs.readdirSync calls the function returned by env->push_values_to_array_function() in batch and check the returned Maybe right away in C++, which can lead to assertions if the call stack already reaches the maximum size. This patch fixes that by returning early the call fails so the stack overflow error will be properly thrown into JS land. PR-URL: #18647 Fixes: #18645 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Apr 13, 2018 · 2995506 · 2995506
1 parent 973488b
commit 2995506
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 3 deletions.
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -951,14 +951,20 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
       name_v[name_idx++] = filename.ToLocalChecked();
 
       if (name_idx >= arraysize(name_v)) {
-        fn->Call(env->context(), names, name_idx, name_v)
-            .ToLocalChecked();
+        MaybeLocal<Value> ret = fn->Call(env->context(), names, name_idx,
+                                         name_v);
+        if (ret.IsEmpty()) {
+          return;
+        }
         name_idx = 0;
       }
     }
 
     if (name_idx > 0) {
-      fn->Call(env->context(), names, name_idx, name_v).ToLocalChecked();
+      MaybeLocal<Value> ret = fn->Call(env->context(), names, name_idx, name_v);
+      if (ret.IsEmpty()) {
+        return;
+      }
     }
 
     args.GetReturnValue().Set(names);

diff --git a/test/parallel/test-fs-readdir-stack-overflow.js b/test/parallel/test-fs-readdir-stack-overflow.js
@@ -0,0 +1,18 @@
+'use strict';
+
+const common = require('../common');
+
+const fs = require('fs');
+
+function recurse() {
+  fs.readdirSync('.');
+  recurse();
+}
+
+common.expectsError(
+  () => recurse(),
+  {
+    type: RangeError,
+    message: 'Maximum call stack size exceeded'
+  }
+);