From 807c9462b1906ffe8a562a77716068b0f36ebbad Mon Sep 17 00:00:00 2001 From: vignesh Date: Sun, 13 Jan 2019 12:18:43 +0530 Subject: [PATCH 1/6] Add validation for tagkey & tagvalue. Fix issue #268 --- packages/opencensus-core/src/stats/view.ts | 24 +++++++++++++++++++++- packages/opencensus-core/test/test-view.ts | 22 ++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/packages/opencensus-core/src/stats/view.ts b/packages/opencensus-core/src/stats/view.ts index 4720221e5..0790ef4a9 100644 --- a/packages/opencensus-core/src/stats/view.ts +++ b/packages/opencensus-core/src/stats/view.ts @@ -74,6 +74,11 @@ export class BaseView implements View { // @ts-ignore private logger: loggerTypes.Logger; + + /** + * A list of tag keys that represents the possible column labels + */ + private readonly MAX_LENGTH: number = 256; /** * Creates a new View instance. This constructor is used by Stats. User should * prefer using Stats.createView() instead. @@ -151,15 +156,32 @@ export class BaseView implements View { } /** - * Checks if tag keys and values have only printable characters. + * Checks if tag keys and values are valid. * @param tags The tags to be checked */ private invalidTags(tags: Tags): boolean { + return this.invalidPrintableCharacters(tags) || this.invalidLength(tags); + } + + /** + * Checks if tag keys and values have only printable characters. + * @param tags The tags to be checked + */ + private invalidPrintableCharacters(tags: Tags): boolean { return Object.keys(tags).some(tagKey => { return invalidString.test(tagKey) || invalidString.test(tags[tagKey]); }); } + /** + * Checks if length of tagkey is greater than 0 & less than 256. + * @param tags The tags to be checked + */ + private invalidLength(tags: Tags): boolean { + return Object.keys(tags).some(tagKey => { + return tagKey.length <= 0 || tagKey.length >= this.MAX_LENGTH; + }); + } /** * Creates an empty aggregation data for a given tags. * @param tags The tags for that aggregation data diff --git a/packages/opencensus-core/test/test-view.ts b/packages/opencensus-core/test/test-view.ts index f557f25b0..7f6ce878d 100644 --- a/packages/opencensus-core/test/test-view.ts +++ b/packages/opencensus-core/test/test-view.ts @@ -192,6 +192,28 @@ describe('BaseView', () => { view.recordMeasurement(measurement); assert.ok(!view.getSnapshot(measurement.tags)); }); + + it('should not record a measurement when a tag key is longer than 255 characters', () => { + const tagkey = 'NHHcUMQtGf7kFjSvy86aZZMIp5zx1aoRH4FFjOJnU4AvwVmAD5GPcmQmwnP6NuWx5NmHdts3xgqyAjn57i9Yc5mak22duPlf7JehY3bMcjbacocAEC1TepDCEt3ihzSOuI9mRvKL4vop7AZ3Uahge6OL3ogIJOhulRlIkK2qeT2avh8FeoGcnNV3O6yKHNovvUoUf01vxnwhG3ruPTh6j2E8G51q1tGAbSUd0UE0Sf2KceRQTr28GOp8zGlIj6lpqJXg'; + const measurement = { + measure, + tags: {[tagkey]: 'testValue'}, + value: 10 + }; + view.recordMeasurement(measurement); + assert.ok(!view.getSnapshot(measurement.tags)); + }); + + it('should not record a measurement when tag key is 0 character long', () => { + const tagkey = ''; + const measurement = { + measure, + tags: {[tagkey]: 'testValue'}, + value: 10 + }; + view.recordMeasurement(measurement); + assert.ok(!view.getSnapshot(measurement.tags)); + }); }); describe('getMetric()', () => { From 21f9890c4ec746eb82c475659b7d8df80a921c82 Mon Sep 17 00:00:00 2001 From: vignesh Date: Sun, 13 Jan 2019 12:42:05 +0530 Subject: [PATCH 2/6] Fix formatting errors --- packages/opencensus-core/src/stats/view.ts | 6 +-- packages/opencensus-core/test/test-view.ts | 43 ++++++++++++---------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/packages/opencensus-core/src/stats/view.ts b/packages/opencensus-core/src/stats/view.ts index 0790ef4a9..916e6d900 100644 --- a/packages/opencensus-core/src/stats/view.ts +++ b/packages/opencensus-core/src/stats/view.ts @@ -73,12 +73,10 @@ export class BaseView implements View { /** An object to log information to */ // @ts-ignore private logger: loggerTypes.Logger; - - /** - * A list of tag keys that represents the possible column labels + * Max Length of a TagKey */ - private readonly MAX_LENGTH: number = 256; + private readonly MAX_LENGTH: number = 256; /** * Creates a new View instance. This constructor is used by Stats. User should * prefer using Stats.createView() instead. diff --git a/packages/opencensus-core/test/test-view.ts b/packages/opencensus-core/test/test-view.ts index 7f6ce878d..b08e3ce5a 100644 --- a/packages/opencensus-core/test/test-view.ts +++ b/packages/opencensus-core/test/test-view.ts @@ -193,27 +193,30 @@ describe('BaseView', () => { assert.ok(!view.getSnapshot(measurement.tags)); }); - it('should not record a measurement when a tag key is longer than 255 characters', () => { - const tagkey = 'NHHcUMQtGf7kFjSvy86aZZMIp5zx1aoRH4FFjOJnU4AvwVmAD5GPcmQmwnP6NuWx5NmHdts3xgqyAjn57i9Yc5mak22duPlf7JehY3bMcjbacocAEC1TepDCEt3ihzSOuI9mRvKL4vop7AZ3Uahge6OL3ogIJOhulRlIkK2qeT2avh8FeoGcnNV3O6yKHNovvUoUf01vxnwhG3ruPTh6j2E8G51q1tGAbSUd0UE0Sf2KceRQTr28GOp8zGlIj6lpqJXg'; - const measurement = { - measure, - tags: {[tagkey]: 'testValue'}, - value: 10 - }; - view.recordMeasurement(measurement); - assert.ok(!view.getSnapshot(measurement.tags)); - }); + it('should not record a measurement when a tag key is longer than 255 characters', + () => { + const tagkey = + 'NHHcUMQtGf7kFjSvy86aZZMIp5zx1aoRH4FFjOJnU4AvwVmAD5GPcmQmwnP6NuWx5NmHdts3xgqyAjn57i9Yc5mak22duPlf7JehY3bMcjbacocAEC1TepDCEt3ihzSOuI9mRvKL4vop7AZ3Uahge6OL3ogIJOhulRlIkK2qeT2avh8FeoGcnNV3O6yKHNovvUoUf01vxnwhG3ruPTh6j2E8G51q1tGAbSUd0UE0Sf2KceRQTr28GOp8zGlIj6lpqJXg'; + const measurement = { + measure, + tags: {[tagkey]: 'testValue'}, + value: 10 + }; + view.recordMeasurement(measurement); + assert.ok(!view.getSnapshot(measurement.tags)); + }); - it('should not record a measurement when tag key is 0 character long', () => { - const tagkey = ''; - const measurement = { - measure, - tags: {[tagkey]: 'testValue'}, - value: 10 - }; - view.recordMeasurement(measurement); - assert.ok(!view.getSnapshot(measurement.tags)); - }); + it('should not record a measurement when tag key is 0 character long', + () => { + const tagkey = ''; + const measurement = { + measure, + tags: {[tagkey]: 'testValue'}, + value: 10 + }; + view.recordMeasurement(measurement); + assert.ok(!view.getSnapshot(measurement.tags)); + }); }); describe('getMetric()', () => { From 9ef27705ec097a1ee3a73d494c96c7efb290f609 Mon Sep 17 00:00:00 2001 From: vignesh Date: Sun, 13 Jan 2019 15:23:26 +0530 Subject: [PATCH 3/6] Log warning to screen when invalid tag has been used --- packages/opencensus-core/src/stats/view.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/opencensus-core/src/stats/view.ts b/packages/opencensus-core/src/stats/view.ts index 916e6d900..db52ebba2 100644 --- a/packages/opencensus-core/src/stats/view.ts +++ b/packages/opencensus-core/src/stats/view.ts @@ -22,6 +22,7 @@ import {BucketBoundaries} from './bucket-boundaries'; import {MetricUtils} from './metric-utils'; import {Recorder} from './recorder'; import {AggregationData, AggregationType, Measure, Measurement, Tags, View} from './types'; +import { logger } from '..'; const RECORD_SEPARATOR = String.fromCharCode(30); const UNIT_SEPARATOR = String.fromCharCode(31); @@ -158,7 +159,11 @@ export class BaseView implements View { * @param tags The tags to be checked */ private invalidTags(tags: Tags): boolean { - return this.invalidPrintableCharacters(tags) || this.invalidLength(tags); + const result: boolean = this.invalidPrintableCharacters(tags) || this.invalidLength(tags); + if(result) { + this.logger.warn('Unable to create tagkey/tagvalue with the specified tags.'); + } + return result; } /** From 9128e13f8e3223c7b930b0b895f3bdf61ddbddb5 Mon Sep 17 00:00:00 2001 From: vignesh Date: Sun, 13 Jan 2019 15:26:01 +0530 Subject: [PATCH 4/6] Remove unnecessary import --- packages/opencensus-core/src/stats/view.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/opencensus-core/src/stats/view.ts b/packages/opencensus-core/src/stats/view.ts index db52ebba2..b58edf6cd 100644 --- a/packages/opencensus-core/src/stats/view.ts +++ b/packages/opencensus-core/src/stats/view.ts @@ -22,7 +22,6 @@ import {BucketBoundaries} from './bucket-boundaries'; import {MetricUtils} from './metric-utils'; import {Recorder} from './recorder'; import {AggregationData, AggregationType, Measure, Measurement, Tags, View} from './types'; -import { logger } from '..'; const RECORD_SEPARATOR = String.fromCharCode(30); const UNIT_SEPARATOR = String.fromCharCode(31); From 06116b5c2e78882278355e5bd133e4f6737bacbf Mon Sep 17 00:00:00 2001 From: vignesh Date: Sun, 13 Jan 2019 15:28:26 +0530 Subject: [PATCH 5/6] Fix formatting error --- packages/opencensus-core/src/stats/view.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/opencensus-core/src/stats/view.ts b/packages/opencensus-core/src/stats/view.ts index b58edf6cd..9d0bc6f6f 100644 --- a/packages/opencensus-core/src/stats/view.ts +++ b/packages/opencensus-core/src/stats/view.ts @@ -158,9 +158,11 @@ export class BaseView implements View { * @param tags The tags to be checked */ private invalidTags(tags: Tags): boolean { - const result: boolean = this.invalidPrintableCharacters(tags) || this.invalidLength(tags); - if(result) { - this.logger.warn('Unable to create tagkey/tagvalue with the specified tags.'); + const result: boolean = + this.invalidPrintableCharacters(tags) || this.invalidLength(tags); + if (result) { + this.logger.warn( + 'Unable to create tagkey/tagvalue with the specified tags.'); } return result; } From 0bc2a1d6ffade365a6df46ad266b62b4c7aef254 Mon Sep 17 00:00:00 2001 From: vignesh Date: Mon, 14 Jan 2019 20:46:13 +0530 Subject: [PATCH 6/6] Replace long string in test case --- packages/opencensus-core/test/test-view.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/opencensus-core/test/test-view.ts b/packages/opencensus-core/test/test-view.ts index b08e3ce5a..82570203e 100644 --- a/packages/opencensus-core/test/test-view.ts +++ b/packages/opencensus-core/test/test-view.ts @@ -195,8 +195,7 @@ describe('BaseView', () => { it('should not record a measurement when a tag key is longer than 255 characters', () => { - const tagkey = - 'NHHcUMQtGf7kFjSvy86aZZMIp5zx1aoRH4FFjOJnU4AvwVmAD5GPcmQmwnP6NuWx5NmHdts3xgqyAjn57i9Yc5mak22duPlf7JehY3bMcjbacocAEC1TepDCEt3ihzSOuI9mRvKL4vop7AZ3Uahge6OL3ogIJOhulRlIkK2qeT2avh8FeoGcnNV3O6yKHNovvUoUf01vxnwhG3ruPTh6j2E8G51q1tGAbSUd0UE0Sf2KceRQTr28GOp8zGlIj6lpqJXg'; + const tagkey = 'a'.repeat(256); const measurement = { measure, tags: {[tagkey]: 'testValue'},