lib: fix crash if invalid SaaS token

And in general, make the `saas` config option handling more robust and consistent. PR-URL: #235 Reviewed-By: Juan José Arboleda <[email protected]>
nodesource · Jan 9, 2025 · 6b23418 · 6b23418
1 parent c82802a
commit 6b23418
Show file tree

Hide file tree

Showing 2 changed files with 124 additions and 8 deletions.
diff --git a/lib/nsolid.js b/lib/nsolid.js
@@ -684,16 +684,10 @@ function updateConfig(config = {}) {
           throw new ERR_INVALID_ARG_TYPE(`config.${key}`, 'string', value);
 
         nsolidConfig[key] = value;
-        if (key === 'command') {
-          nsolidConfig.saas = undefined;
-        }
       } else if (key === 'statsd' || key === 'grpc') {
         // The statsd variable can now be set to null to stop the statsd agent.
         nsolidConfig[key] = value;
       }
-    } else if (key === 'saas' && !config.command) {
-      nsolidConfig.saas = config.saas;
-      nsolidConfig.command = parseSaasEnvVar(nsolidConfig.saas, 0);
     } else if (key === 'otlp') {
       const otlp = parseOTLPType(config.otlp);
       if (otlp !== nsolidConfig.otlp) {
@@ -713,6 +707,25 @@ function updateConfig(config = {}) {
     nsolidConfig.otlp = null;
   }
 
+  if (nsolidConfig.saas) {
+    if (!config.command) {
+      nsolidConfig.command = undefined;
+    }
+
+    if (nsolidConfig.command) {
+      nsolidConfig.saas = undefined;
+    } else {
+      const url = parseSaasEnvVar(nsolidConfig.saas, 0);
+      if (!url) {
+        nsolidConfig.saas = undefined;
+      } else if (!nsolidConfig.grpc) {
+        nsolidConfig.command = url;
+      } else {
+        nsolidConfig.grpc = '' + config.grpc;
+      }
+    }
+  }
+
   if (nsolidConfig.grpc && !nsolidConfig.saas) {
     // Make sure it's a valid URL otherwise it might crash in the grpc++
     // URLParser implementation
@@ -777,8 +790,13 @@ function initializeConfig(nsolidConfig) {
   if (nsolidConfig.saas) {
     if (nsolidConfig.command) {
       nsolidConfig.saas = undefined;
-    } else if (!nsolidConfig.grpc) {
-      nsolidConfig.command = parseSaasEnvVar(nsolidConfig.saas, 0);
+    } else {
+      const url = parseSaasEnvVar(nsolidConfig.saas, 0);
+      if (!url) {
+        nsolidConfig.saas = undefined;
+      } else if (!nsolidConfig.grpc) {
+        nsolidConfig.command = url;
+      }
     }
   }
 

diff --git a/test/parallel/test-nsolid-config-saas.js b/test/parallel/test-nsolid-config-saas.js
@@ -9,6 +9,7 @@ const PORT = 12345;
 const saasToken =
   'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ.example.org:9001';
 const saasCommand = 'ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ.example.org:9001';
+const invalidSaasToken = 'invalid.example.org:9001';
 
 if (process.config.variables.asan)
   common.skip('incorrect leak reporting in curl.');
@@ -171,6 +172,85 @@ function execProc7() {
   });
 }
 
+function execProc8() {
+  let output = '';
+  const proc = spawn(process.execPath,
+                     [ __filename, 'child1' ],
+                     {
+                       env: {
+                         NSOLID_SAAS: invalidSaasToken
+                       }
+                     });
+  proc.stdout.on('data', (d) => {
+    output += d;
+  });
+
+  proc.on('close', (code) => {
+    assert.strictEqual(code, 0);
+    const config = JSON.parse(output);
+    assert.strictEqual(config.command, undefined);
+    assert.strictEqual(config.saas, undefined);
+  });
+}
+
+function execProc9() {
+  let output = '';
+  const proc = spawn(process.execPath,
+                     [ __filename, 'child1' ],
+                     {
+                       env: {
+                         NSOLID_SAAS: invalidSaasToken,
+                         NSOLID_GRPC: PORT,
+                       }
+                     });
+  proc.stdout.on('data', (d) => {
+    output += d;
+  });
+
+  proc.on('close', (code) => {
+    assert.strictEqual(code, 0);
+    const config = JSON.parse(output);
+    assert.strictEqual(config.command, undefined);
+    assert.strictEqual(config.grpc, `localhost:${PORT}`);
+    assert.strictEqual(config.saas, undefined);
+  });
+}
+
+function execProc10() {
+  let output = '';
+  const proc = spawn(process.execPath,
+                     [ __filename, 'child5' ]);
+  proc.stdout.on('data', (d) => {
+    output += d;
+  });
+
+  proc.on('close', (code) => {
+    assert.strictEqual(code, 0);
+    const config = JSON.parse(output);
+    assert.strictEqual(config.command, undefined);
+    assert.strictEqual(config.grpc, `localhost:${PORT}`);
+    assert.strictEqual(config.saas, undefined);
+  });
+}
+
+function execProc11() {
+  let output = '';
+  const proc = spawn(process.execPath,
+                     [ __filename, 'child6' ]);
+  proc.stdout.on('data', (d) => {
+    output += d;
+  });
+
+  proc.on('close', (code) => {
+    assert.strictEqual(code, 0);
+    const config = JSON.parse(output);
+    assert.strictEqual(config.command, undefined);
+    assert.strictEqual(config.grpc, `${PORT}`);
+    assert.strictEqual(config.saas, saasToken);
+  });
+}
+
+
 switch (process.argv[2]) {
   case 'child1':
     process.stdout.write(JSON.stringify(nsolid.config));
@@ -201,6 +281,20 @@ switch (process.argv[2]) {
       process.stdout.write(JSON.stringify(out));
     }
     break;
+  case 'child5':
+    nsolid.start({
+      grpc: PORT,
+      saas: invalidSaasToken
+    });
+    process.stdout.write(JSON.stringify(nsolid.config));
+    break;
+  case 'child6':
+    nsolid.start({
+      grpc: PORT,
+      saas: saasToken
+    });
+    process.stdout.write(JSON.stringify(nsolid.config));
+    break;
   default:
     execProc1();
     execProc2();
@@ -209,4 +303,8 @@ switch (process.argv[2]) {
     execProc5();
     execProc6();
     execProc7();
+    execProc8();
+    execProc9();
+    execProc10();
+    execProc11();
 }