Add validation for dimension values (#131)

* add: throw error when invalid dimensions
* move validation to separate module

Author: Mark Kuhn

package-lock.json

@@ -39,6 +39,7 @@
     "@types/faker": "^4.1.5",
     "@types/jest": "^26.0.22",
     "@types/node": "^12.0.8",
+    "@types/validator": "^13.7.5",
     "@typescript-eslint/eslint-plugin": "^2.23.0",
     "@typescript-eslint/parser": "^2.23.0",
     "aws-sdk": "^2.551.0",
@@ -52,6 +53,7 @@
     "prettier": "^1.19.1",
     "ts-jest": "^26.5.4",
     "typescript": "^3.8.0",
+    "validator": "^13.7.0",
     "y18n": ">=4.0.1"
   },
   "files": [

package.json

@@ -14,6 +14,9 @@
  */
 
 export enum Constants {
+  MAX_DIMENSION_NAME_LENGTH = 250,
+  MAX_DIMENSION_VALUE_LENGTH = 1024,
+
   MAX_DIMENSION_SET_SIZE = 30,
   DEFAULT_NAMESPACE = 'aws-embedded-metrics',
   MAX_METRICS_PER_EVENT = 100,

src/Constants.ts

@@ -0,0 +1,8 @@
+export class InvalidDimensionError extends Error {
+  constructor(msg: string) {
+    super(msg);
+
+    // Set the prototype explicitly.
+    Object.setPrototypeOf(this, InvalidDimensionError.prototype);
+  }
+}

src/exceptions/InvalidDimensionError.ts

@@ -15,10 +15,9 @@
 
 import Configuration from '../config/Configuration';
 import { LOG } from '../utils/Logger';
+import { Validator } from '../utils/Validator';
 import { MetricValues } from './MetricValues';
 import { Unit } from './Unit';
-import { Constants } from '../Constants';
-import { DimensionSetExceededError } from '../exceptions/DimensionSetExceededError';
 
 interface IProperties {
   [s: string]: unknown;
@@ -106,25 +105,14 @@ export class MetricsContext {
     this.defaultDimensions = dimensions;
   }
 
-  /**
-   * Validates dimension set length is not more than Constants.MAX_DIMENSION_SET_SIZE
-   *
-   * @param dimensionSet
-   */
-  public static validateDimensionSet(dimensionSet: Record<string, string>): void {
-    if (Object.keys(dimensionSet).length > Constants.MAX_DIMENSION_SET_SIZE)
-      throw new DimensionSetExceededError(
-        `Maximum number of dimensions per dimension set allowed are ${Constants.MAX_DIMENSION_SET_SIZE}`)
-  }
-
   /**
    * Adds a new set of dimensions. Any time a new dimensions set
    * is added, the set is first prepended by the default dimensions.
    *
    * @param dimensions
    */
   public putDimensions(incomingDimensionSet: Record<string, string>): void {
-    MetricsContext.validateDimensionSet(incomingDimensionSet);
+    Validator.validateDimensionSet(incomingDimensionSet);
 
     // Duplicate dimensions sets are removed before being added to the end of the collection.
     // This ensures the latest dimension key-value is used as a target member on the root EMF node.
@@ -151,7 +139,7 @@ export class MetricsContext {
   public setDimensions(dimensionSets: Array<Record<string, string>>): void {
     this.shouldUseDefaultDimensions = false;
 
-    dimensionSets.forEach(dimensionSet => MetricsContext.validateDimensionSet(dimensionSet))
+    dimensionSets.forEach(dimensionSet => Validator.validateDimensionSet(dimensionSet));
 
     this.dimensions = dimensionSets;
   }

src/logger/MetricsContext.ts

@@ -1,6 +1,7 @@
 import * as faker from 'faker';
 import { MetricsContext } from '../MetricsContext';
 import { DimensionSetExceededError } from '../../exceptions/DimensionSetExceededError';
+import { InvalidDimensionError } from '../../exceptions/InvalidDimensionError';
 
 test('can set property', () => {
   // arrange
@@ -20,7 +21,7 @@ test('can set property', () => {
 test('setDimensions allows 30 dimensions', () => {
   // arrange
   const context = MetricsContext.empty();
-  const numOfDimensions = 30
+  const numOfDimensions = 30;
   const expectedDimensionSet = getDimensionSet(numOfDimensions);
 
   // act
@@ -31,7 +32,6 @@ test('setDimensions allows 30 dimensions', () => {
 });
 
 test('putDimension adds key to dimension and sets the dimension as a property', () => {
-
   // arrange
   const context = MetricsContext.empty();
   const dimension = faker.random.word();
@@ -251,30 +251,65 @@ test('createCopyWithContext copies shouldUseDefaultDimensions', () => {
 test('putDimensions checks the dimension set length', () => {
   // arrange
   const context = MetricsContext.empty();
-  const numOfDimensions = 33
+  const numOfDimensions = 33;
 
   expect(() => {
-    context.putDimensions(getDimensionSet(numOfDimensions))
+    context.putDimensions(getDimensionSet(numOfDimensions));
   }).toThrow(DimensionSetExceededError);
 });
 
 test('setDimensions checks all the dimension sets have less than 30 dimensions', () => {
   // arrange
   const context = MetricsContext.empty();
-  const numOfDimensions = 33
+  const numOfDimensions = 33;
 
   expect(() => {
-    context.setDimensions([getDimensionSet(numOfDimensions)])
+    context.setDimensions([getDimensionSet(numOfDimensions)]);
   }).toThrow(DimensionSetExceededError);
 });
 
+test('adding dimensions validates them', () => {
+  // arrange
+  const context = MetricsContext.empty();
+  const dimensionNameWithInvalidAscii = { '🚀': faker.random.word() };
+  const dimensionValueWithInvalidAscii = { d1: 'مارك' };
+  const dimensionWithLongName = { ['a'.repeat(251)]: faker.random.word() };
+  const dimensionWithLongValue = { d1: 'a'.repeat(1025) };
+  const dimensionWithEmptyName = { ['']: faker.random.word() };
+  const dimensionWithEmptyValue = { d1: '' };
+  const dimensionNameStartWithColon = { ':d1': faker.random.word() };
+
+  // act
+  expect(() => {
+    context.putDimensions(dimensionNameWithInvalidAscii);
+  }).toThrow(InvalidDimensionError);
+  expect(() => {
+    context.setDimensions([dimensionValueWithInvalidAscii]);
+  }).toThrow(InvalidDimensionError);
+  expect(() => {
+    context.putDimensions(dimensionWithLongName);
+  }).toThrow(InvalidDimensionError);
+  expect(() => {
+    context.setDimensions([dimensionWithLongValue]);
+  }).toThrow(InvalidDimensionError);
+  expect(() => {
+    context.putDimensions(dimensionWithEmptyName);
+  }).toThrow(InvalidDimensionError);
+  expect(() => {
+    context.setDimensions([dimensionWithEmptyValue]);
+  }).toThrow(InvalidDimensionError);
+  expect(() => {
+    context.putDimensions(dimensionNameStartWithColon);
+  }).toThrow(InvalidDimensionError);
+});
+
 const getDimensionSet = (numOfDimensions: number) => {
-  const dimensionSet:Record<string, string> = {}
+  const dimensionSet: Record<string, string> = {};
 
   for (let i = 0; i < numOfDimensions; i++) {
     const expectedKey = `${i}`;
     dimensionSet[expectedKey] = faker.random.word();
   }
 
   return dimensionSet;
-}
+};

src/logger/__tests__/MetricsContext.test.ts

@@ -38,21 +38,19 @@ export class LogSerializer implements ISerializer {
     const dimensionKeys: string[][] = [];
     let dimensionProperties = {};
 
-    context.getDimensions().forEach(d => {
-      // we can only take the first 9 defined dimensions
-      // the reason we do this in the serializer is because
-      // it is possible that other sinks or formats can
-      // support more dimensions
-      // in the future it may make sense to introduce a higher-order
-      // representation for sink-specific validations
-      const keys = Object.keys(d);
+    context.getDimensions().forEach(dimensionSet => {
+      const keys = Object.keys(dimensionSet);
+
       if (keys.length > Constants.MAX_DIMENSION_SET_SIZE) {
-        const errMsg = `Maximum number of dimensions allowed are ${Constants.MAX_DIMENSION_SET_SIZE}.` +
-        `Account for default dimensions if not using set_dimensions.`;
-        throw new DimensionSetExceededError(errMsg)
+        const errMsg =
+          `Maximum number of dimensions allowed are ${Constants.MAX_DIMENSION_SET_SIZE}.` +
+          `Account for default dimensions if not using set_dimensions.`;
+        throw new DimensionSetExceededError(errMsg);
       }
+
+
       dimensionKeys.push(keys);
-      dimensionProperties = { ...dimensionProperties, ...d };
+      dimensionProperties = { ...dimensionProperties, ...dimensionSet };
     });
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any

src/serializers/LogSerializer.ts

@@ -180,7 +180,7 @@ test('cannot serialize more than 30 dimensions', () => {
 
   // assert
   expect(() => {
-    serializer.serialize(context)
+    serializer.serialize(context);
   }).toThrow(DimensionSetExceededError);
 });
 

src/serializers/__tests__/LogSerializer.test.ts

@@ -0,0 +1,73 @@
+/*
+ * Copyright 2019 Amazon.com, Inc. or its affiliates.
+ * Licensed under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import validator from 'validator';
+import { Constants } from '../Constants';
+import { DimensionSetExceededError } from '../exceptions/DimensionSetExceededError';
+import { InvalidDimensionError } from '../exceptions/InvalidDimensionError';
+
+export class Validator {
+  /**
+   * Validates dimension set.
+   * @see [CloudWatch Dimensions](https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_Dimension.html)
+   *
+   * @param dimensionSet
+   * @throws {DimensionSetExceededError} Dimension set must not exceed 30 dimensions.
+   * @throws {InvalidDimensionError} Dimension name and value must be valid.
+   */
+  public static validateDimensionSet(dimensionSet: Record<string, string>): void {
+    // Validates dimension set length
+    if (Object.keys(dimensionSet).length > Constants.MAX_DIMENSION_SET_SIZE)
+      throw new DimensionSetExceededError(
+        `Maximum number of dimensions per dimension set allowed are ${Constants.MAX_DIMENSION_SET_SIZE}`,
+      );
+
+    // Validate dimension key and value
+    Object.entries(dimensionSet).forEach(([key, value]) => {
+      dimensionSet[key] = value = String(value);
+
+      if (!validator.isAscii(key)) {
+        throw new InvalidDimensionError(`Dimension key ${key} has invalid characters`);
+      }
+      if (!validator.isAscii(value)) {
+        throw new InvalidDimensionError(`Dimension value ${value} has invalid characters`);
+      }
+
+      if (key.trim().length == 0) {
+        throw new InvalidDimensionError(`Dimension key ${key} must include at least one non-whitespace character`);
+      }
+
+      if (value.trim().length == 0) {
+        throw new InvalidDimensionError(`Dimension value ${value} must include at least one non-whitespace character`);
+      }
+
+      if (key.length > Constants.MAX_DIMENSION_NAME_LENGTH) {
+        throw new InvalidDimensionError(
+          `Dimension key ${key} must not exceed maximum length ${Constants.MAX_DIMENSION_NAME_LENGTH}`,
+        );
+      }
+
+      if (value.length > Constants.MAX_DIMENSION_VALUE_LENGTH) {
+        throw new InvalidDimensionError(
+          `Dimension value ${value} must not exceed maximum length ${Constants.MAX_DIMENSION_VALUE_LENGTH}`,
+        );
+      }
+
+      if (key.startsWith(':')) {
+        throw new InvalidDimensionError(`Dimension key ${key} cannot start with ':'`);
+      }
+    });
+  }
+}