Merge pull request #87 from adobe/fix/validate-key-characters

fix: validate key characters, throw actionable error

Author: MichaelGoberling

README.md

@@ -70,7 +70,7 @@ Apply when init with OW credentials (and not own cloud DB credentials):
 - Max state key size: `1024 bytes`
 - Max total state size: `10 GB`
 - Token expiry (need to re-init after expiry): `1 hour`
-- Non supported characters for state keys are: `'/', '\\', '?', '#'`
+- Non supported characters for state keys are: `'/', '\', '?', '#'`
 
 ## Adobe I/O State Store Consistency Guarantees
 

lib/StateStore.js

@@ -55,7 +55,9 @@ function validateInput (input, schema, details) {
 
 // eslint-disable-next-line jsdoc/require-jsdoc
 function validateKey (key, details, label = 'key') {
-  validateInput(key, joi.string().label(label).required(), details)
+  validateInput(key, joi.string().label(label).required().regex(/[?#/\\]/, { invert: true }).messages({
+    'string.pattern.invert.base': 'Key cannot contain ?, #, /, or \\'
+  }), details)
 }
 
 // eslint-disable-next-line jsdoc/require-jsdoc

lib/StateStoreError.js

@@ -27,7 +27,7 @@ const logger = require('@adobe/aio-lib-core-logging')('@adobe/aio-lib-state', {
  *
  * @typedef StateLibErrors
  * @type {object}
- * @property {StateLibError} ERROR_BAD_ARGUMENT this error is thrown when an argument is missing or has invalid type
+ * @property {StateLibError} ERROR_BAD_ARGUMENT this error is thrown when an argument is missing, has invalid type, or includes invalid characters.
  * @property {StateLibError} ERROR_BAD_REQUEST this error is thrown when an argument has an illegal value.
  * @property {StateLibError} ERROR_NOT_IMPLEMENTED this error is thrown when a method is not implemented or when calling
  * methods directly on the abstract class (StateStore).

test/StateStore.test.js

@@ -38,6 +38,14 @@ describe('get', () => {
     const state = new StateStore(true)
     await global.expectToThrowBadArg(state.get.bind(state, 123), ['string', 'key'], { key: 123 })
   })
+  // eslint-disable-next-line jest/expect-expect
+  test('bad key characters', async () => {
+    const state = new StateStore(true)
+    await global.expectToThrowBadArg(state.put.bind(state, '?test', 'value', {}), ['Key', 'cannot', 'contain'], { key: '?test', value: 'value', options: {} })
+    await global.expectToThrowBadArg(state.put.bind(state, 't#est', 'value', {}), ['Key', 'cannot', 'contain'], { key: 't#est', value: 'value', options: {} })
+    await global.expectToThrowBadArg(state.put.bind(state, 't\\est', 'value', {}), ['Key', 'cannot', 'contain'], { key: 't\\est', value: 'value', options: {} })
+    await global.expectToThrowBadArg(state.put.bind(state, 'test/', 'value', {}), ['Key', 'cannot', 'contain'], { key: 'test/', value: 'value', options: {} })
+  })
   test('calls _get (part of interface)', async () => {
     const state = new StateStore(true)
     state._get = jest.fn()
@@ -60,6 +68,14 @@ describe('put', () => {
     await global.expectToThrowBadArg(state.put.bind(state, 123, 'value', {}), ['string', 'key'], { key: 123, value: 'value', options: {} })
   })
   // eslint-disable-next-line jest/expect-expect
+  test('bad key characters', async () => {
+    const state = new StateStore(true)
+    await global.expectToThrowBadArg(state.put.bind(state, '?test', 'value', {}), ['Key', 'cannot', 'contain'], { key: '?test', value: 'value', options: {} })
+    await global.expectToThrowBadArg(state.put.bind(state, 't#est', 'value', {}), ['Key', 'cannot', 'contain'], { key: 't#est', value: 'value', options: {} })
+    await global.expectToThrowBadArg(state.put.bind(state, 't\\est', 'value', {}), ['Key', 'cannot', 'contain'], { key: 't\\est', value: 'value', options: {} })
+    await global.expectToThrowBadArg(state.put.bind(state, 'test/', 'value', {}), ['Key', 'cannot', 'contain'], { key: 'test/', value: 'value', options: {} })
+  })
+  // eslint-disable-next-line jest/expect-expect
   test('bad options', async () => {
     const state = new StateStore(true)
     const expectedDetails = { key: 'key', value: 'value' }
@@ -102,6 +118,14 @@ describe('delete', () => {
     const state = new StateStore(true)
     await global.expectToThrowBadArg(state.delete.bind(state, 123), ['string', 'key'], { key: 123 })
   })
+  // eslint-disable-next-line jest/expect-expect
+  test('bad key characters', async () => {
+    const state = new StateStore(true)
+    await global.expectToThrowBadArg(state.put.bind(state, '?test', 'value', {}), ['Key', 'cannot', 'contain'], { key: '?test', value: 'value', options: {} })
+    await global.expectToThrowBadArg(state.put.bind(state, 't#est', 'value', {}), ['Key', 'cannot', 'contain'], { key: 't#est', value: 'value', options: {} })
+    await global.expectToThrowBadArg(state.put.bind(state, 't\\est', 'value', {}), ['Key', 'cannot', 'contain'], { key: 't\\est', value: 'value', options: {} })
+    await global.expectToThrowBadArg(state.put.bind(state, 'test/', 'value', {}), ['Key', 'cannot', 'contain'], { key: 'test/', value: 'value', options: {} })
+  })
   test('calls _delete (part of interface)', async () => {
     const state = new StateStore(true)
     state._delete = jest.fn()