Kaynağa Gözat

fix: plug fs.readdir refcount leak on error path, assert workflowId in fixture

fszontagh 1 ay önce
ebeveyn
işleme
9ee2f357dd

+ 13 - 4
src/runner/engine/script_engine.cpp

@@ -2578,6 +2578,14 @@ void ScriptEngine::setupBuiltinAPIs() {
         std::string path_str(path);
         JS_FreeCString(ctx, path);
 
+        // Held outside the try so the catch block can free it if an
+        // exception escapes after it was created but before it was handed
+        // to a response object - otherwise a partially built entries array
+        // (and everything already pushed into it) leaks. Freeing
+        // JS_UNDEFINED is a no-op, so this is safe even if the exception
+        // happened before entries was ever created.
+        JSValue entries = JS_UNDEFINED;
+
         try {
             if (!std::filesystem::exists(path_str)) {
                 JSValue response = JS_NewObject(ctx);
@@ -2594,7 +2602,7 @@ void ScriptEngine::setupBuiltinAPIs() {
                 return response;
             }
 
-            JSValue entries = JS_NewArray(ctx);
+            entries = JS_NewArray(ctx);
             uint32_t index = 0;
 
             for (const auto& entry : std::filesystem::directory_iterator(path_str)) {
@@ -2604,15 +2612,14 @@ void ScriptEngine::setupBuiltinAPIs() {
                 JS_SetPropertyStr(ctx, item, "path",
                                   JS_NewString(ctx, entry.path().string().c_str()));
 
-                const bool is_dir = entry.is_directory();
-                JS_SetPropertyStr(ctx, item, "isDirectory", is_dir ? JS_TRUE : JS_FALSE);
-
                 int64_t size = 0;
                 int64_t mtime_ms = 0;
+                bool is_dir = false;
                 // A file removed between listing and stat is ordinary on a
                 // directory being written to, and is reported with zeroes
                 // rather than failing the whole listing.
                 try {
+                    is_dir = entry.is_directory();
                     if (entry.is_regular_file()) {
                         size = static_cast<int64_t>(entry.file_size());
                     }
@@ -2628,6 +2635,7 @@ void ScriptEngine::setupBuiltinAPIs() {
                 } catch (const std::exception&) {
                 }
 
+                JS_SetPropertyStr(ctx, item, "isDirectory", is_dir ? JS_TRUE : JS_FALSE);
                 JS_SetPropertyStr(ctx, item, "size", JS_NewInt64(ctx, size));
                 JS_SetPropertyStr(ctx, item, "modifiedAt", JS_NewInt64(ctx, mtime_ms));
 
@@ -2639,6 +2647,7 @@ void ScriptEngine::setupBuiltinAPIs() {
             JS_SetPropertyStr(ctx, response, "entries", entries);
             return response;
         } catch (const std::exception& e) {
+            JS_FreeValue(ctx, entries);
             JSValue response = JS_NewObject(ctx);
             JS_SetPropertyStr(ctx, response, "success", JS_FALSE);
             JS_SetPropertyStr(ctx, response, "error", JS_NewString(ctx, e.what()));

+ 2 - 1
tests/nodes/fs-readdir.json

@@ -17,7 +17,8 @@
       "subIsDirectory": true,
       "oneIsDirectory": false,
       "missingRejected": true,
-      "notDirRejected": true
+      "notDirRejected": true,
+      "hasWorkflowId": true
     }}}
   }
 }