src: improve performance of dotenv ToObject

Improves the performance of the dotenv parser
ToObject method. Also, switch to a null prototype
object to avoid potential prototype pollution
issues.

PR-URL: #60038
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>

Author: jasnell

src/node_dotenv.cc

@@ -9,8 +9,10 @@ namespace node {
 using v8::EscapableHandleScope;
 using v8::JustVoid;
 using v8::Local;
+using v8::LocalVector;
 using v8::Maybe;
 using v8::MaybeLocal;
+using v8::Name;
 using v8::Nothing;
 using v8::Object;
 using v8::String;
@@ -86,20 +88,29 @@ Maybe<void> Dotenv::SetEnvironment(node::Environment* env) {
 
 MaybeLocal<Object> Dotenv::ToObject(Environment* env) const {
   EscapableHandleScope scope(env->isolate());
-  Local<Object> result = Object::New(env->isolate());
 
-  Local<Value> name;
-  Local<Value> val;
+  LocalVector<Name> names(env->isolate(), store_.size());
+  LocalVector<Value> values(env->isolate(), store_.size());
   auto context = env->context();
 
+  Local<Value> tmp;
+
+  int n = 0;
   for (const auto& entry : store_) {
-    if (!ToV8Value(context, entry.first).ToLocal(&name) ||
-        !ToV8Value(context, entry.second).ToLocal(&val) ||
-        result->Set(context, name, val).IsNothing()) {
+    if (!ToV8Value(context, entry.first).ToLocal(&tmp)) {
+      return MaybeLocal<Object>();
+    }
+    names[n] = tmp.As<Name>();
+    if (!ToV8Value(context, entry.second).ToLocal(&tmp)) {
       return MaybeLocal<Object>();
     }
+    values[n++] = tmp;
   }
-
+  Local<Object> result = Object::New(env->isolate(),
+                                     Null(env->isolate()),
+                                     names.data(),
+                                     values.data(),
+                                     values.size());
   return scope.Escape(result);
 }
 

test/parallel/test-dotenv-edge-cases.js

@@ -215,6 +215,7 @@ describe('.env supports edge cases', () => {
     ].join('\n'));
 
     assert.deepStrictEqual(result, {
+      __proto__: null,
       baz: 'whatever',
       VALID_AFTER_INVALID: 'test',
       ANOTHER_VALID: 'value',
@@ -236,6 +237,7 @@ describe('.env supports edge cases', () => {
     ].join('\n'));
 
     assert.deepStrictEqual(result, {
+      __proto__: null,
       KEY_WITH_SPACES_BEFORE: 'value_with_spaces_before_and_after',
       KEY_WITH_TABS_BEFORE: 'value_with_tabs_before_and_after',
       KEY_WITH_SPACES_AND_TABS: 'value_with_spaces_and_tabs',
@@ -255,6 +257,7 @@ describe('.env supports edge cases', () => {
     ].join('\n'));
 
     assert.deepStrictEqual(result, {
+      __proto__: null,
       KEY_WITH_COMMENT_IN_VALUE: 'value # this is a comment',
     });
   });

test/parallel/test-util-parse-env.js

@@ -10,7 +10,8 @@ const fs = require('node:fs');
   const validEnvFilePath = fixtures.path('dotenv/valid.env');
   const validContent = fs.readFileSync(validEnvFilePath, 'utf8');
 
-  assert.deepStrictEqual(util.parseEnv(validContent), {
+  const checkObj = {
+    __proto__: null,
     A: 'B=C',
     B: 'C=D',
     AFTER_LINE: 'after_line',
@@ -56,11 +57,14 @@ const fs = require('node:fs');
     SPACED_KEY: 'parsed',
     SPACE_BEFORE_DOUBLE_QUOTES: 'space before double quotes',
     TRIM_SPACE_FROM_UNQUOTED: 'some spaced out string',
-  });
+  };
+
+  assert.deepStrictEqual(util.parseEnv(validContent), checkObj);
 }
 
-assert.deepStrictEqual(util.parseEnv(''), {});
-assert.deepStrictEqual(util.parseEnv('FOO=bar\nFOO=baz\n'), { FOO: 'baz' });
+assert.deepStrictEqual(util.parseEnv(''), { __proto__: null });
+assert.deepStrictEqual(util.parseEnv('FOO=bar\nFOO=baz\n'),
+                       { __proto__: null, FOO: 'baz' });
 
 // Test for invalid input.
 assert.throws(() => {