Skip to content

Commit 4d0d246

Browse files
authored
Validate performance metrics and attributes before storing them (#1917)
* Add validation for custom metrics and custom attributes, and throw a Firebase Performance error when a specified metric, attribute name, or attribute value fails validation. * [AUTOMATED]: Prettier Code Styling * Respond to comments: include check that values are not blank, and adjust test format. * [AUTOMATED]: Prettier Code Styling * Use single quotes in trace.test.ts. * [AUTOMATED]: Prettier Code Styling * Correct cases of == and != to === and !==. * Change "doesn't" to "doesnt" in test strings in order to use single quotes. Code formatter automatically converts strings containing \' to use double quotes. * Add import for Array.some to packages/polyfill/index.ts.
1 parent 394ea1b commit 4d0d246

File tree

8 files changed

+207
-7
lines changed

8 files changed

+207
-7
lines changed

packages/performance/src/resources/trace.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,14 @@ describe('Firebase Performance > trace', () => {
116116

117117
expect(trace.getMetric('cacheHits')).to.eql(600);
118118
});
119+
120+
it('throws error if metric doesnt exist and has invalid name', () => {
121+
expect(() => trace.incrementMetric('_invalidMetric', 1)).to.throw();
122+
});
119123
});
120124

121125
describe('#putMetric', () => {
122-
it('creates new metric if one doesnt exist.', () => {
126+
it('creates new metric if one doesnt exist and has valid name.', () => {
123127
trace.putMetric('cacheHits', 200);
124128

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

132136
expect(trace.getMetric('cacheHits')).to.eql(400);
133137
});
138+
139+
it('throws error if metric doesnt exist and has invalid name', () => {
140+
expect(() => trace.putMetric('_invalidMetric', 1)).to.throw();
141+
});
134142
});
135143

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

173181
expect(trace.getAttributes()).to.eql({ level: '7' });
174182
});
183+
184+
it('throws error if attribute name is invalid', () => {
185+
expect(() => trace.putAttribute('_invalidAttribute', '1')).to.throw();
186+
});
187+
188+
it('throws error if attribute value is invalid', () => {
189+
const longAttributeValue =
190+
'too-long-attribute-value-over-one-hundred-characters-too-long-attribute-value-over-one-' +
191+
'hundred-charac';
192+
expect(() =>
193+
trace.putAttribute('validName', longAttributeValue)
194+
).to.throw();
195+
});
175196
});
176197

177198
describe('#getAttribute', () => {

packages/performance/src/resources/trace.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ import {
2727
import { Api } from '../services/api_service';
2828
import { logTrace } from '../services/perf_logger';
2929
import { ERROR_FACTORY, ErrorCode } from '../utils/errors';
30+
import {
31+
isValidCustomAttributeName,
32+
isValidCustomAttributeValue
33+
} from '../utils/attributes_utils';
34+
import { isValidCustomMetricName } from '../utils/metric_utils';
3035
import { PerformanceTrace } from '@firebase/performance-types';
3136

3237
const enum TraceState {
@@ -146,7 +151,7 @@ export class Trace implements PerformanceTrace {
146151
*/
147152
incrementMetric(counter: string, num = 1): void {
148153
if (this.counters[counter] === undefined) {
149-
this.counters[counter] = 0;
154+
this.putMetric(counter, 0);
150155
}
151156
this.counters[counter] += num;
152157
}
@@ -158,7 +163,13 @@ export class Trace implements PerformanceTrace {
158163
* @param num Set custom metric to this value
159164
*/
160165
putMetric(counter: string, num: number): void {
161-
this.counters[counter] = num;
166+
if (isValidCustomMetricName(counter)) {
167+
this.counters[counter] = num;
168+
} else {
169+
throw ERROR_FACTORY.create(ErrorCode.INVALID_CUSTOM_METRIC_NAME, {
170+
customMetricName: counter
171+
});
172+
}
162173
}
163174

164175
/**
@@ -176,7 +187,23 @@ export class Trace implements PerformanceTrace {
176187
* @param value
177188
*/
178189
putAttribute(attr: string, value: string): void {
179-
this.customAttributes[attr] = value;
190+
const isValidName = isValidCustomAttributeName(attr);
191+
const isValidValue = isValidCustomAttributeValue(value);
192+
if (isValidName && isValidValue) {
193+
this.customAttributes[attr] = value;
194+
return;
195+
}
196+
// Throw appropriate error when the attribute name or value is invalid.
197+
if (!isValidName) {
198+
throw ERROR_FACTORY.create(ErrorCode.INVALID_ATTRIBUTE_NAME, {
199+
attributeName: attr
200+
});
201+
}
202+
if (!isValidValue) {
203+
throw ERROR_FACTORY.create(ErrorCode.INVALID_ATTRIBUTE_VALUE, {
204+
attributeValue: value
205+
});
206+
}
180207
}
181208

182209
/**

packages/performance/src/utils/attribute_utils.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ import {
2323
getVisibilityState,
2424
VisibilityState,
2525
getServiceWorkerStatus,
26-
getEffectiveConnectionType
26+
getEffectiveConnectionType,
27+
isValidCustomAttributeName,
28+
isValidCustomAttributeValue
2729
} from './attributes_utils';
2830

2931
import '../../test/setup';
@@ -172,4 +174,51 @@ describe('Firebase Performance > attribute_utils', () => {
172174
expect(getEffectiveConnectionType()).to.be.eql(0);
173175
});
174176
});
177+
178+
describe('#isValidCustomAttributeName', () => {
179+
it('returns true when name is valid', () => {
180+
expect(isValidCustomAttributeName('validCustom_Attribute_Name')).to.be
181+
.true;
182+
});
183+
184+
it('returns false when name is blank', () => {
185+
expect(isValidCustomAttributeName('')).to.be.false;
186+
});
187+
188+
it('returns false when name is too long', () => {
189+
expect(
190+
isValidCustomAttributeName('invalid_custom_name_over_forty_characters')
191+
).to.be.false;
192+
});
193+
194+
it('returns false when name starts with a reserved prefix', () => {
195+
expect(isValidCustomAttributeName('firebase_invalidCustomName')).to.be
196+
.false;
197+
});
198+
199+
it('returns false when name does not begin with a letter', () => {
200+
expect(isValidCustomAttributeName('_invalidCustomName')).to.be.false;
201+
});
202+
203+
it('returns false when name contains prohibited characters', () => {
204+
expect(isValidCustomAttributeName('invalidCustomName&')).to.be.false;
205+
});
206+
});
207+
208+
describe('#isValidCustomAttributeValue', () => {
209+
it('returns true when value is valid', () => {
210+
expect(isValidCustomAttributeValue('valid_attribute_value')).to.be.true;
211+
});
212+
213+
it('returns false when value is blank', () => {
214+
expect(isValidCustomAttributeValue('')).to.be.false;
215+
});
216+
217+
it('returns false when value is too long', () => {
218+
const longAttributeValue =
219+
'too_long_attribute_value_over_one_hundred_characters_too_long_attribute_value_over_one_' +
220+
'hundred_charac';
221+
expect(isValidCustomAttributeValue(longAttributeValue)).to.be.false;
222+
});
223+
});
175224
});

packages/performance/src/utils/attributes_utils.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ const enum EffectiveConnectionType {
4141
CONNECTION_4G = 4
4242
}
4343

44+
const RESERVED_ATTRIBUTE_PREFIXES = ['firebase_', 'google_', 'ga_'];
45+
const ATTRIBUTE_FORMAT_REGEX = new RegExp('^[a-zA-Z]\\w*$');
46+
const MAX_ATTRIBUTE_NAME_LENGTH = 40;
47+
const MAX_ATTRIBUTE_VALUE_LENGTH = 100;
48+
4449
export function getServiceWorkerStatus(): ServiceWorkerStatus {
4550
const navigator = Api.getInstance().navigator;
4651
if ('serviceWorker' in navigator) {
@@ -88,3 +93,17 @@ export function getEffectiveConnectionType(): EffectiveConnectionType {
8893
return EffectiveConnectionType.UNKNOWN;
8994
}
9095
}
96+
97+
export function isValidCustomAttributeName(name: string): boolean {
98+
if (name.length === 0 || name.length > MAX_ATTRIBUTE_NAME_LENGTH) {
99+
return false;
100+
}
101+
const matchesReservedPrefix = RESERVED_ATTRIBUTE_PREFIXES.some(prefix =>
102+
name.startsWith(prefix)
103+
);
104+
return !matchesReservedPrefix && !!name.match(ATTRIBUTE_FORMAT_REGEX);
105+
}
106+
107+
export function isValidCustomAttributeValue(value: string): boolean {
108+
return value.length !== 0 && value.length <= MAX_ATTRIBUTE_VALUE_LENGTH;
109+
}

packages/performance/src/utils/errors.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ export const enum ErrorCode {
2727
NO_API_KEY = 'no api key',
2828
INVALID_CC_LOG = 'invalid cc log',
2929
FB_NOT_DEFAULT = 'FB not default',
30-
RC_NOT_OK = 'RC response not ok'
30+
RC_NOT_OK = 'RC response not ok',
31+
INVALID_ATTRIBUTE_NAME = 'invalid attribute name',
32+
INVALID_ATTRIBUTE_VALUE = 'invalid attribute value',
33+
INVALID_CUSTOM_METRIC_NAME = 'invalide custom metric name'
3134
}
3235

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

4655
interface ErrorParams {
4756
[ErrorCode.TRACE_STARTED_BEFORE]: { traceName: string };
4857
[ErrorCode.TRACE_STOPPED_BEFORE]: { traceName: string };
58+
[ErrorCode.INVALID_ATTRIBUTE_NAME]: { attributeName: string };
59+
[ErrorCode.INVALID_ATTRIBUTE_VALUE]: { attributeValue: string };
60+
[ErrorCode.INVALID_CUSTOM_METRIC_NAME]: { customMetricName: string };
4961
}
5062

5163
export const ERROR_FACTORY = new ErrorFactory<ErrorCode, ErrorParams>(
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* @license
3+
* Copyright 2019 Google Inc.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF unknown KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
import { expect } from 'chai';
19+
20+
import { isValidCustomMetricName } from './metric_utils';
21+
22+
import '../../test/setup';
23+
24+
describe('Firebase Performance > metric_utils', () => {
25+
describe('#isValidCustomMetricName', () => {
26+
it('returns true when name is valid', () => {
27+
expect(isValidCustomMetricName('validCustom_Metric_Name')).to.be.true;
28+
});
29+
30+
it('returns false when name is blank', () => {
31+
expect(isValidCustomMetricName('')).to.be.false;
32+
});
33+
34+
it('returns false when name is too long', () => {
35+
const longMetricName =
36+
'too_long_metric_name_over_one_hundred_characters_too_long_metric_name_over_one_' +
37+
'hundred_characters_too';
38+
expect(isValidCustomMetricName(longMetricName)).to.be.false;
39+
});
40+
41+
it('returns false when name starts with a reserved prefix', () => {
42+
expect(isValidCustomMetricName('_invalidMetricName')).to.be.false;
43+
});
44+
});
45+
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @license
3+
* Copyright 2019 Google Inc.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
const MAX_METRIC_NAME_LENGTH = 100;
19+
const RESERVED_AUTO_PREFIX = '_';
20+
21+
export function isValidCustomMetricName(name: string): boolean {
22+
if (name.length === 0 || name.length > MAX_METRIC_NAME_LENGTH) {
23+
return false;
24+
}
25+
return !name.startsWith(RESERVED_AUTO_PREFIX);
26+
}

packages/polyfill/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import 'promise-polyfill/lib/polyfill';
2525
import 'core-js/features/array/find';
2626
import 'core-js/features/array/find-index';
2727
import 'core-js/features/array/from';
28+
import 'core-js/features/array/some';
2829
import 'core-js/features/object/assign';
2930
import 'core-js/features/object/entries';
3031
import 'core-js/features/object/values';

0 commit comments

Comments
 (0)