Merge pull request #743 from sasjs/issue-722

fix: improve input validations

.github/dependabot.yml

@@ -4,4 +4,4 @@ updates:
     directory: "/"
     schedule:
       interval: "monthly"
-    open-pull-requests-limit: 2
+    open-pull-requests-limit: 1

src/SASjs.ts

@@ -1,4 +1,4 @@
-import { compareTimestamps, asyncForEach } from './utils'
+import { compareTimestamps, asyncForEach, validateInput } from './utils'
 import {
   SASjsConfig,
   UploadFile,
@@ -686,7 +686,7 @@ export default class SASjs {
       ...config
     }
 
-    const validationResult = this.validateInput(data)
+    const validationResult = validateInput(data)
 
     // status is true if the data passes validation checks above
     if (validationResult.status) {
@@ -748,74 +748,6 @@ export default class SASjs {
     }
   }
 
-  /**
-   * This function validates the input data structure and table naming convention
-   *
-   * @param data A json object that contains one or more tables, it can also be null
-   * @returns An object which contains two attributes: 1) status: boolean, 2) msg: string
-   */
-  private validateInput(data: { [key: string]: any } | null): {
-    status: boolean
-    msg: string
-  } {
-    if (data === null) return { status: true, msg: '' }
-
-    const isSasFormatsTable = (key: string) =>
-      key.match(/^\$.*/) && Object.keys(data).includes(key.replace(/^\$/, ''))
-
-    for (const key in data) {
-      if (!key.match(/^[a-zA-Z_]/) && !isSasFormatsTable(key)) {
-        return {
-          status: false,
-          msg: 'First letter of table should be alphabet or underscore.'
-        }
-      }
-
-      if (!key.match(/^[a-zA-Z_][a-zA-Z0-9_]*$/) && !isSasFormatsTable(key)) {
-        return { status: false, msg: 'Table name should be alphanumeric.' }
-      }
-
-      if (key.length > 32) {
-        return {
-          status: false,
-          msg: 'Maximum length for table name could be 32 characters.'
-        }
-      }
-
-      if (this.getType(data[key]) !== 'Array' && !isSasFormatsTable(key)) {
-        return {
-          status: false,
-          msg: 'Parameter data contains invalid table structure.'
-        }
-      }
-
-      for (let i = 0; i < data[key].length; i++) {
-        if (this.getType(data[key][i]) !== 'object') {
-          return {
-            status: false,
-            msg: `Table ${key} contains invalid structure.`
-          }
-        }
-      }
-    }
-
-    return { status: true, msg: '' }
-  }
-
-  /**
-   * this function returns the type of variable
-   *
-   * @param data it could be anything, like string, array, object etc.
-   * @returns a string which tells the type of input parameter
-   */
-  private getType(data: any): string {
-    if (Array.isArray(data)) {
-      return 'Array'
-    } else {
-      return typeof data
-    }
-  }
-
   /**
    * Creates the folders and services at the given location `appLoc` on the given server `serverUrl`.
    * @param serviceJson - the JSON specifying the folders and services to be created.

src/auth/refreshTokensForSasjs.ts

@@ -28,7 +28,7 @@ export async function refreshTokensForSasjs(
       }
     })
     .catch((err) => {
-      throw prefixMessage(err, 'Error while refreshing tokens')
+      throw prefixMessage(err, 'Error while refreshing tokens: ')
     })
 
   return authResponse

src/auth/refreshTokensForViya.ts

@@ -42,7 +42,7 @@ export async function refreshTokensForViya(
     )
     .then((res) => res.result)
     .catch((err) => {
-      throw prefixMessage(err, 'Error while refreshing tokens')
+      throw prefixMessage(err, 'Error while refreshing tokens: ')
     })
 
   return authResponse

src/auth/spec/refreshTokensForSasjs.spec.ts

@@ -27,17 +27,20 @@ describe('refreshTokensForSasjs', () => {
 
   it('should handle errors while refreshing tokens', async () => {
     setupMocks()
+
     const refresh_token = generateToken(30)
+    const tokenError = 'unable to verify the first certificate'
+
     jest
       .spyOn(requestClient, 'post')
-      .mockImplementation(() => Promise.reject('Token Error'))
+      .mockImplementation(() => Promise.reject(tokenError))
 
     const error = await refreshTokensForSasjs(
       requestClient,
       refresh_token
     ).catch((e: any) => e)
 
-    expect(error).toContain('Error while refreshing tokens')
+    expect(error).toEqual(`Error while refreshing tokens: ${tokenError}`)
   })
 })
 

src/auth/spec/refreshTokensForViya.spec.ts

@@ -46,17 +46,20 @@ describe('refreshTokensForViya', () => {
 
   it('should handle errors while refreshing tokens', async () => {
     setupMocks()
+
     const access_token = generateToken(30)
     const refresh_token = generateToken(30)
+    const tokenError = 'unable to verify the first certificate'
     const authConfig: AuthConfig = {
       access_token,
       refresh_token,
       client: 'cl13nt',
       secret: 's3cr3t'
     }
+
     jest
       .spyOn(requestClient, 'post')
-      .mockImplementation(() => Promise.reject('Token Error'))
+      .mockImplementation(() => Promise.reject(tokenError))
 
     const error = await refreshTokensForViya(
       requestClient,
@@ -65,7 +68,7 @@ describe('refreshTokensForViya', () => {
       authConfig.refresh_token
     ).catch((e: any) => e)
 
-    expect(error).toContain('Error while refreshing tokens')
+    expect(error).toEqual(`Error while refreshing tokens: ${tokenError}`)
   })
 })
 

src/request/RequestClient.ts

@@ -611,7 +611,10 @@ export const throwIfError = (response: AxiosResponse) => {
         throw new LoginRequiredError(response.data)
       }
 
-      if (response.data.toLowerCase() === 'invalid csrf token!') {
+      if (
+        typeof response.data === 'string' &&
+        response.data.toLowerCase() === 'invalid csrf token!'
+      ) {
         throw new InvalidCsrfError()
       }
       break

src/test/utils/formatDataForRequest.spec.ts

@@ -11,13 +11,16 @@ describe('formatDataForRequest', () => {
         { var1: 'string', var2: 232, nullvar: '_' },
         { var1: 'string', var2: 232, nullvar: 0 },
         { var1: 'string', var2: 232, nullvar: 'z' },
-        { var1: 'string', var2: 232, nullvar: null }
+        { var1: 'string', var2: 232, nullvar: null },
+        { var1: 'string', var2: 232, nullvar: '.A' },
+        { var1: 'string', var2: 232, nullvar: '._' },
+        { var1: 'string', var2: 232, nullvar: '.' }
       ],
       [`$${testTable}`]: { formats: { var1: '$char12.', nullvar: 'best.' } }
     }
 
     const expectedOutput = {
-      sasjs1data: `var1:$char12. var2:best. nullvar:best.\r\nstring,232,.a\r\nstring,232,.b\r\nstring,232,._\r\nstring,232,0\r\nstring,232,.z\r\nstring,232,.`,
+      sasjs1data: `var1:$char12. var2:best. nullvar:best.\r\nstring,232,.a\r\nstring,232,.b\r\nstring,232,._\r\nstring,232,0\r\nstring,232,.z\r\nstring,232,.\r\nstring,232,.a\r\nstring,232,._\r\nstring,232,.`,
       sasjs_tables: testTable
     }
 

src/utils/convertToCsv.ts

@@ -141,7 +141,9 @@ export const convertToCSV = (
           )
         }
 
-        return `.${value.toLowerCase()}`
+        const dot = value.includes('.') ? '' : '.'
+
+        return `${dot}${value.toLowerCase()}`
       }
 
       // if there any present, it should have preceding (") for escaping

src/utils/index.ts

@@ -1,20 +1,21 @@
+export * from './appendExtraResponseAttributes'
 export * from './asyncForEach'
 export * from './compareTimestamps'
 export * from './convertToCsv'
 export * from './createAxiosInstance'
 export * from './delay'
+export * from './fetchLogByChunks'
+export * from './getValidJson'
 export * from './isNode'
 export * from './isRelativePath'
 export * from './isUri'
 export * from './isUrl'
 export * from './needsRetry'
 export * from './parseGeneratedCode'
-export * from './parseSourceCode'
 export * from './parseSasViyaLog'
+export * from './parseSourceCode'
+export * from './parseViyaDebugResponse'
+export * from './parseWeboutResponse'
 export * from './serialize'
 export * from './splitChunks'
-export * from './parseWeboutResponse'
-export * from './fetchLogByChunks'
-export * from './getValidJson'
-export * from './parseViyaDebugResponse'
-export * from './appendExtraResponseAttributes'
+export * from './validateInput'

src/utils/spec/convertToCsv.spec.ts

@@ -1,4 +1,4 @@
-import { convertToCSV, isFormatsTable } from './convertToCsv'
+import { convertToCSV, isFormatsTable } from '../convertToCsv'
 
 describe('convertToCsv', () => {
   const tableName = 'testTable'

src/utils/spec/validateInput.spec.ts

@@ -0,0 +1,84 @@
+import {
+  validateInput,
+  INVALID_TABLE_STRUCTURE,
+  MORE_INFO
+} from '../validateInput'
+
+const tableArray = [{ col1: 'first col value' }]
+const stringData: any = { table1: tableArray }
+
+describe('validateInput', () => {
+  it('should not return an error message if input data valid', () => {
+    const validationResult = validateInput(stringData)
+    expect(validationResult).toEqual({
+      status: true,
+      msg: ''
+    })
+  })
+
+  it('should not return an error message if input data is null', () => {
+    const validationResult = validateInput(null)
+    expect(validationResult).toEqual({
+      status: true,
+      msg: ''
+    })
+  })
+
+  it('should return an error message if input data is an array', () => {
+    const validationResult = validateInput(tableArray)
+    expect(validationResult).toEqual({
+      status: false,
+      msg: INVALID_TABLE_STRUCTURE
+    })
+  })
+
+  it('should return an error message if first letter of table is neither alphabet nor underscore', () => {
+    const validationResult = validateInput({ '1stTable': tableArray })
+    expect(validationResult).toEqual({
+      status: false,
+      msg: 'First letter of table should be alphabet or underscore.'
+    })
+  })
+
+  it('should return an error message if table name contains a character other than alphanumeric or underscore', () => {
+    const validationResult = validateInput({ 'table!': tableArray })
+    expect(validationResult).toEqual({
+      status: false,
+      msg: 'Table name should be alphanumeric.'
+    })
+  })
+
+  it('should return an error message if length of table name contains exceeds 32', () => {
+    const validationResult = validateInput({
+      xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: tableArray
+    })
+    expect(validationResult).toEqual({
+      status: false,
+      msg: 'Maximum length for table name could be 32 characters.'
+    })
+  })
+
+  it('should return an error message if table does not have array of objects', () => {
+    const validationResult = validateInput({ table: stringData })
+    expect(validationResult).toEqual({
+      status: false,
+      msg: INVALID_TABLE_STRUCTURE
+    })
+  })
+
+  it('should return an error message if a table array has an item other than object', () => {
+    const validationResult = validateInput({ table1: ['invalid'] })
+    expect(validationResult).toEqual({
+      status: false,
+      msg: `Table table1 contains invalid structure. ${MORE_INFO}`
+    })
+  })
+
+  it('should return an error message if a row in a table contains an column with undefined value', () => {
+    const validationResult = validateInput({ table1: [{ column: undefined }] })
+    expect(validationResult).toEqual({
+      status: false,
+      msg: `A row in table table1 contains invalid value. Can't assign undefined to column.`
+    })
+  })
+})

src/utils/validateInput.ts

@@ -0,0 +1,90 @@
+export const MORE_INFO =
+  'For more info see https://sasjs.io/sasjs-adapter/#request-response'
+export const INVALID_TABLE_STRUCTURE = `Parameter data contains invalid table structure. ${MORE_INFO}`
+
+/**
+ * This function validates the input data structure and table naming convention
+ *
+ * @param data A json object that contains one or more tables, it can also be null
+ * @returns An object which contains two attributes: 1) status: boolean, 2) msg: string
+ */
+export const validateInput = (
+  data: { [key: string]: any } | null
+): {
+  status: boolean
+  msg: string
+} => {
+  if (data === null) return { status: true, msg: '' }
+
+  if (getType(data) !== 'object') {
+    return {
+      status: false,
+      msg: INVALID_TABLE_STRUCTURE
+    }
+  }
+
+  const isSasFormatsTable = (key: string) =>
+    key.match(/^\$.*/) && Object.keys(data).includes(key.replace(/^\$/, ''))
+
+  for (const key in data) {
+    if (!key.match(/^[a-zA-Z_]/) && !isSasFormatsTable(key)) {
+      return {
+        status: false,
+        msg: 'First letter of table should be alphabet or underscore.'
+      }
+    }
+
+    if (!key.match(/^[a-zA-Z_][a-zA-Z0-9_]*$/) && !isSasFormatsTable(key)) {
+      return { status: false, msg: 'Table name should be alphanumeric.' }
+    }
+
+    if (key.length > 32) {
+      return {
+        status: false,
+        msg: 'Maximum length for table name could be 32 characters.'
+      }
+    }
+
+    if (getType(data[key]) !== 'Array' && !isSasFormatsTable(key)) {
+      return {
+        status: false,
+        msg: INVALID_TABLE_STRUCTURE
+      }
+    }
+
+    for (const item of data[key]) {
+      if (getType(item) !== 'object') {
+        return {
+          status: false,
+          msg: `Table ${key} contains invalid structure. ${MORE_INFO}`
+        }
+      } else {
+        const attributes = Object.keys(item)
+        for (const attribute of attributes) {
+          if (item[attribute] === undefined) {
+            return {
+              status: false,
+              msg: `A row in table ${key} contains invalid value. Can't assign undefined to ${attribute}.`
+            }
+          }
+        }
+      }
+    }
+  }
+
+  return { status: true, msg: '' }
+}
+
+/**
+ * this function returns the type of variable
+ *
+ * @param data it could be anything, like string, array, object etc.
+ * @returns a string which tells the type of input parameter
+ */
+const getType = (data: any): string => {
+  if (Array.isArray(data)) {
+    return 'Array'
+  } else {
+    return typeof data
+  }
+}