Skip to content
23 changes: 22 additions & 1 deletion packages/performance/src/resources/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,14 @@ describe('Firebase Performance > trace', () => {

expect(trace.getMetric('cacheHits')).to.eql(600);
});

it('throws error if metric doesnt exist and has invalid name', () => {
expect(() => trace.incrementMetric('_invalidMetric', 1)).to.throw();
});
});

describe('#putMetric', () => {
it('creates new metric if one doesnt exist.', () => {
it('creates new metric if one doesnt exist and has valid name.', () => {
trace.putMetric('cacheHits', 200);

expect(trace.getMetric('cacheHits')).to.eql(200);
Expand All @@ -131,6 +135,10 @@ describe('Firebase Performance > trace', () => {

expect(trace.getMetric('cacheHits')).to.eql(400);
});

it('throws error if metric doesnt exist and has invalid name', () => {
expect(() => trace.putMetric('_invalidMetric', 1)).to.throw();
});
});

describe('#getMetric', () => {
Expand Down Expand Up @@ -172,6 +180,19 @@ describe('Firebase Performance > trace', () => {

expect(trace.getAttributes()).to.eql({ level: '7' });
});

it('throws error if attribute name is invalid', () => {
expect(() => trace.putAttribute('_invalidAttribute', '1')).to.throw();
});

it('throws error if attribute value is invalid', () => {
const longAttributeValue =
'too-long-attribute-value-over-one-hundred-characters-too-long-attribute-value-over-one-' +
'hundred-charac';
expect(() =>
trace.putAttribute('validName', longAttributeValue)
).to.throw();
});
});

describe('#getAttribute', () => {
Expand Down
33 changes: 30 additions & 3 deletions packages/performance/src/resources/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ import {
import { Api } from '../services/api_service';
import { logTrace } from '../services/perf_logger';
import { ERROR_FACTORY, ErrorCode } from '../utils/errors';
import {
isValidCustomAttributeName,
isValidCustomAttributeValue
} from '../utils/attributes_utils';
import { isValidCustomMetricName } from '../utils/metric_utils';
import { PerformanceTrace } from '@firebase/performance-types';

const enum TraceState {
Expand Down Expand Up @@ -146,7 +151,7 @@ export class Trace implements PerformanceTrace {
*/
incrementMetric(counter: string, num = 1): void {
if (this.counters[counter] === undefined) {
this.counters[counter] = 0;
this.putMetric(counter, 0);
}
this.counters[counter] += num;
}
Expand All @@ -158,7 +163,13 @@ export class Trace implements PerformanceTrace {
* @param num Set custom metric to this value
*/
putMetric(counter: string, num: number): void {
this.counters[counter] = num;
if (isValidCustomMetricName(counter)) {
this.counters[counter] = num;
} else {
throw ERROR_FACTORY.create(ErrorCode.INVALID_CUSTOM_METRIC_NAME, {
customMetricName: counter
});
}
}

/**
Expand All @@ -176,7 +187,23 @@ export class Trace implements PerformanceTrace {
* @param value
*/
putAttribute(attr: string, value: string): void {
this.customAttributes[attr] = value;
const isValidName = isValidCustomAttributeName(attr);
const isValidValue = isValidCustomAttributeValue(value);
if (isValidName && isValidValue) {
this.customAttributes[attr] = value;
return;
}
// Throw appropriate error when the attribute name or value is invalid.
if (!isValidName) {
throw ERROR_FACTORY.create(ErrorCode.INVALID_ATTRIBUTE_NAME, {
attributeName: attr
});
}
if (!isValidValue) {
throw ERROR_FACTORY.create(ErrorCode.INVALID_ATTRIBUTE_VALUE, {
attributeValue: value
});
}
}

/**
Expand Down
51 changes: 50 additions & 1 deletion packages/performance/src/utils/attribute_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ import {
getVisibilityState,
VisibilityState,
getServiceWorkerStatus,
getEffectiveConnectionType
getEffectiveConnectionType,
isValidCustomAttributeName,
isValidCustomAttributeValue
} from './attributes_utils';

import '../../test/setup';
Expand Down Expand Up @@ -172,4 +174,51 @@ describe('Firebase Performance > attribute_utils', () => {
expect(getEffectiveConnectionType()).to.be.eql(0);
});
});

describe('#isValidCustomAttributeName', () => {
it('returns true when name is valid', () => {
expect(isValidCustomAttributeName('validCustom_Attribute_Name')).to.be
.true;
});

it('returns false when name is blank', () => {
expect(isValidCustomAttributeName('')).to.be.false;
});

it('returns false when name is too long', () => {
expect(
isValidCustomAttributeName('invalid_custom_name_over_forty_characters')
).to.be.false;
});

it('returns false when name starts with a reserved prefix', () => {
expect(isValidCustomAttributeName('firebase_invalidCustomName')).to.be
.false;
});

it('returns false when name does not begin with a letter', () => {
expect(isValidCustomAttributeName('_invalidCustomName')).to.be.false;
});

it('returns false when name contains prohibited characters', () => {
expect(isValidCustomAttributeName('invalidCustomName&')).to.be.false;
});
});

describe('#isValidCustomAttributeValue', () => {
it('returns true when value is valid', () => {
expect(isValidCustomAttributeValue('valid_attribute_value')).to.be.true;
});

it('returns false when value is blank', () => {
expect(isValidCustomAttributeValue('')).to.be.false;
});

it('returns false when value is too long', () => {
const longAttributeValue =
'too_long_attribute_value_over_one_hundred_characters_too_long_attribute_value_over_one_' +
'hundred_charac';
expect(isValidCustomAttributeValue(longAttributeValue)).to.be.false;
});
});
});
19 changes: 19 additions & 0 deletions packages/performance/src/utils/attributes_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ const enum EffectiveConnectionType {
CONNECTION_4G = 4
}

const RESERVED_ATTRIBUTE_PREFIXES = ['firebase_', 'google_', 'ga_'];
const ATTRIBUTE_FORMAT_REGEX = new RegExp('^[a-zA-Z]\\w*$');
const MAX_ATTRIBUTE_NAME_LENGTH = 40;
const MAX_ATTRIBUTE_VALUE_LENGTH = 100;

export function getServiceWorkerStatus(): ServiceWorkerStatus {
const navigator = Api.getInstance().navigator;
if ('serviceWorker' in navigator) {
Expand Down Expand Up @@ -88,3 +93,17 @@ export function getEffectiveConnectionType(): EffectiveConnectionType {
return EffectiveConnectionType.UNKNOWN;
}
}

export function isValidCustomAttributeName(name: string): boolean {
if (name.length === 0 || name.length > MAX_ATTRIBUTE_NAME_LENGTH) {
return false;
}
const matchesReservedPrefix = RESERVED_ATTRIBUTE_PREFIXES.some(prefix =>
name.startsWith(prefix)
);
return !matchesReservedPrefix && !!name.match(ATTRIBUTE_FORMAT_REGEX);
}

export function isValidCustomAttributeValue(value: string): boolean {
return value.length !== 0 && value.length <= MAX_ATTRIBUTE_VALUE_LENGTH;
}
16 changes: 14 additions & 2 deletions packages/performance/src/utils/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ export const enum ErrorCode {
NO_API_KEY = 'no api key',
INVALID_CC_LOG = 'invalid cc log',
FB_NOT_DEFAULT = 'FB not default',
RC_NOT_OK = 'RC response not ok'
RC_NOT_OK = 'RC response not ok',
INVALID_ATTRIBUTE_NAME = 'invalid attribute name',
INVALID_ATTRIBUTE_VALUE = 'invalid attribute value',
INVALID_CUSTOM_METRIC_NAME = 'invalide custom metric name'
}

const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
Expand All @@ -40,12 +43,21 @@ const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
[ErrorCode.INVALID_CC_LOG]: 'Attempted to queue invalid cc event',
[ErrorCode.FB_NOT_DEFAULT]:
'Performance can only start when Firebase app instance is the default one.',
[ErrorCode.RC_NOT_OK]: 'RC response is not ok'
[ErrorCode.RC_NOT_OK]: 'RC response is not ok',
[ErrorCode.INVALID_ATTRIBUTE_NAME]:
'Attribute name {$attributeName} is invalid.',
[ErrorCode.INVALID_ATTRIBUTE_VALUE]:
'Attribute value {$attributeValue} is invalid.',
[ErrorCode.INVALID_CUSTOM_METRIC_NAME]:
'Custom metric name {$customMetricName} is invalid'
};

interface ErrorParams {
[ErrorCode.TRACE_STARTED_BEFORE]: { traceName: string };
[ErrorCode.TRACE_STOPPED_BEFORE]: { traceName: string };
[ErrorCode.INVALID_ATTRIBUTE_NAME]: { attributeName: string };
[ErrorCode.INVALID_ATTRIBUTE_VALUE]: { attributeValue: string };
[ErrorCode.INVALID_CUSTOM_METRIC_NAME]: { customMetricName: string };
}

export const ERROR_FACTORY = new ErrorFactory<ErrorCode, ErrorParams>(
Expand Down
45 changes: 45 additions & 0 deletions packages/performance/src/utils/metric_utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* @license
* Copyright 2019 Google Inc.
*
* 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 unknown KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { expect } from 'chai';

import { isValidCustomMetricName } from './metric_utils';

import '../../test/setup';

describe('Firebase Performance > metric_utils', () => {
describe('#isValidCustomMetricName', () => {
it('returns true when name is valid', () => {
expect(isValidCustomMetricName('validCustom_Metric_Name')).to.be.true;
});

it('returns false when name is blank', () => {
expect(isValidCustomMetricName('')).to.be.false;
});

it('returns false when name is too long', () => {
const longMetricName =
'too_long_metric_name_over_one_hundred_characters_too_long_metric_name_over_one_' +
'hundred_characters_too';
expect(isValidCustomMetricName(longMetricName)).to.be.false;
});

it('returns false when name starts with a reserved prefix', () => {
expect(isValidCustomMetricName('_invalidMetricName')).to.be.false;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kim-f Hello, I have noticed that i'm getting an uncaught exception for _fp as defined in

export const FIRST_PAINT_COUNTER_NAME = '_fp';

can this be updated to ignore the already defined constants?

});
});
});
26 changes: 26 additions & 0 deletions packages/performance/src/utils/metric_utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* @license
* Copyright 2019 Google Inc.
*
* 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.
*/

const MAX_METRIC_NAME_LENGTH = 100;
const RESERVED_AUTO_PREFIX = '_';

export function isValidCustomMetricName(name: string): boolean {
if (name.length === 0 || name.length > MAX_METRIC_NAME_LENGTH) {
return false;
}
return !name.startsWith(RESERVED_AUTO_PREFIX);
}
1 change: 1 addition & 0 deletions packages/polyfill/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import 'promise-polyfill/lib/polyfill';
import 'core-js/features/array/find';
import 'core-js/features/array/find-index';
import 'core-js/features/array/from';
import 'core-js/features/array/some';
import 'core-js/features/object/assign';
import 'core-js/features/object/entries';
import 'core-js/features/object/values';
Expand Down