more validations on parsing client configuration

snowflakedb · Oct 6, 2023 · 04f549d · 04f549d
1 parent d818058
commit 04f549d
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 7 deletions.
diff --git a/lib/configuration/client_configuration.js b/lib/configuration/client_configuration.js
@@ -4,6 +4,7 @@
 const os = require('os');
 const path = require('path');
 const fs = require('fs');
+const {isString} = require('../util');
 const clientConfigFileName = 'sf_client_config.json';
 
 const Levels = Object.freeze({
@@ -95,8 +96,8 @@ function ConfigurationUtil(fsPromisesModule, processModule) {
       validate(parsedConfiguration);
       return new ClientConfig(
         new ClientLoggingConfig(
-          logLevel(parsedConfiguration),
-          logPath(parsedConfiguration)
+          getLogLevel(parsedConfiguration),
+          getLogPath(parsedConfiguration)
         )
       );
     } catch (err) {
@@ -105,17 +106,36 @@ function ConfigurationUtil(fsPromisesModule, processModule) {
   }
 
   function validate (configuration) {
-    const level = logLevel(configuration);
-    if (level != null) {
-      levelFromString(level);
+    validateLogLevel(configuration);
+    validateLogPath(configuration);
+  }
+
+  function validateLogLevel(configuration) {
+    const logLevel = getLogLevel(configuration);
+    if (logLevel == null) {
+      return;
+    }
+    if (!isString(logLevel)) {
+      throw new Error('Log level is not a string');
+    }
+    levelFromString(logLevel);
+  }
+
+  function validateLogPath(configuration) {
+    const logPath = getLogPath(configuration);
+    if (logPath == null) {
+      return;
+    }
+    if (!isString(logPath)) {
+      throw new Error('Log path is not a string');
     }
   }
 
-  function logLevel (configuration) {
+  function getLogLevel (configuration) {
     return configuration.common.log_level;
   }
 
-  function logPath (configuration) {
+  function getLogPath (configuration) {
     return configuration.common.log_path;
   }
 

diff --git a/test/unit/configuration/configuration_parsing_test.js b/test/unit/configuration/configuration_parsing_test.js
@@ -132,6 +132,24 @@ describe('Configuration parsing tests', function () {
       testCaseName: 'no common in config',
       fileContent: '{}'
     },
+    {
+      testCaseName: 'log level is not a string',
+      fileContent: `{
+            "common": {
+                "log_level": 5,
+                "log_path": "/some-path/some-directory"
+            } 
+        }`
+    },
+    {
+      testCaseName: 'log path is not a string',
+      fileContent: `{
+            "common": {
+                "log_level": "${Levels.Info}",
+                "log_path": true
+            } 
+        }`
+    },
   ].forEach(({testCaseName, fileContent}) => {
     it('should fail for wrong config content ' + testCaseName, async function () {
       // given