Skip to content

Commit

Permalink
feat(util): Add stringMatchesSomePattern helper function. (#6205)
Browse files Browse the repository at this point in the history
This adds a new utility function, `stringMatchesSomePattern`, which tests a string against an entire array of strings and/or regexes at once (which is something we do in a number of spots in the SDK). It also adds an optional parameter to `isMatchingPattern` to allow the string matching to be exact rather than a substring match.
  • Loading branch information
lobsterkatie committed Nov 15, 2022
1 parent 23c19bf commit 6627882
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 21 deletions.
10 changes: 4 additions & 6 deletions packages/core/src/integrations/inboundfilters.ts
@@ -1,5 +1,5 @@
import { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types';
import { getEventDescription, isMatchingPattern, logger } from '@sentry/utils';
import { getEventDescription, logger, stringMatchesSomePattern } from '@sentry/utils';

// "Script error." is hard coded into browsers for errors that it can't read.
// this is the result of a script being pulled in from an external domain and CORS.
Expand Down Expand Up @@ -107,9 +107,7 @@ function _isIgnoredError(event: Event, ignoreErrors?: Array<string | RegExp>): b
return false;
}

return _getPossibleEventMessages(event).some(message =>
ignoreErrors.some(pattern => isMatchingPattern(message, pattern)),
);
return _getPossibleEventMessages(event).some(message => stringMatchesSomePattern(message, ignoreErrors));
}

function _isDeniedUrl(event: Event, denyUrls?: Array<string | RegExp>): boolean {
Expand All @@ -118,7 +116,7 @@ function _isDeniedUrl(event: Event, denyUrls?: Array<string | RegExp>): boolean
return false;
}
const url = _getEventFilterUrl(event);
return !url ? false : denyUrls.some(pattern => isMatchingPattern(url, pattern));
return !url ? false : stringMatchesSomePattern(url, denyUrls);
}

function _isAllowedUrl(event: Event, allowUrls?: Array<string | RegExp>): boolean {
Expand All @@ -127,7 +125,7 @@ function _isAllowedUrl(event: Event, allowUrls?: Array<string | RegExp>): boolea
return true;
}
const url = _getEventFilterUrl(event);
return !url ? true : allowUrls.some(pattern => isMatchingPattern(url, pattern));
return !url ? true : stringMatchesSomePattern(url, allowUrls);
}

function _getPossibleEventMessages(event: Event): string[] {
Expand Down
6 changes: 2 additions & 4 deletions packages/node/src/integrations/http.ts
Expand Up @@ -3,9 +3,9 @@ import { EventProcessor, Integration, Span, TracePropagationTargets } from '@sen
import {
dynamicSamplingContextToSentryBaggageHeader,
fill,
isMatchingPattern,
logger,
parseSemver,
stringMatchesSomePattern,
} from '@sentry/utils';
import * as http from 'http';
import * as https from 'https';
Expand Down Expand Up @@ -169,9 +169,7 @@ function _createWrappedRequestMethodFactory(
return headersUrlMap[url];
}

headersUrlMap[url] = tracingOptions.tracePropagationTargets.some(tracePropagationTarget =>
isMatchingPattern(url, tracePropagationTarget),
);
headersUrlMap[url] = stringMatchesSomePattern(url, tracingOptions.tracePropagationTargets);

return headersUrlMap[url];
};
Expand Down
5 changes: 2 additions & 3 deletions packages/tracing/src/browser/request.ts
Expand Up @@ -5,7 +5,7 @@ import {
BAGGAGE_HEADER_NAME,
dynamicSamplingContextToSentryBaggageHeader,
isInstanceOf,
isMatchingPattern,
stringMatchesSomePattern,
} from '@sentry/utils';

import { getActiveTransaction, hasTracingEnabled } from '../utils';
Expand Down Expand Up @@ -123,8 +123,7 @@ export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumenta
typeof shouldCreateSpanForRequest === 'function' ? shouldCreateSpanForRequest : (_: string) => true;

const shouldAttachHeaders = (url: string): boolean =>
tracingOrigins.some(origin => isMatchingPattern(url, origin)) ||
tracePropagationTargets.some(origin => isMatchingPattern(url, origin));
stringMatchesSomePattern(url, tracingOrigins) || stringMatchesSomePattern(url, tracePropagationTargets);

const spans: Record<string, Span> = {};

Expand Down
38 changes: 32 additions & 6 deletions packages/utils/src/string.ts
Expand Up @@ -84,24 +84,50 @@ export function safeJoin(input: any[], delimiter?: string): string {
}

/**
* Checks if the value matches a regex or includes the string
* @param value The string value to be checked against
* @param pattern Either a regex or a string that must be contained in value
* Checks if the given value matches a regex or string
*
* @param value The string to test
* @param pattern Either a regex or a string against which `value` will be matched
* @param requireExactStringMatch If true, `value` must match `pattern` exactly. If false, `value` will match
* `pattern` if it contains `pattern`. Only applies to string-type patterns.
*/
export function isMatchingPattern(value: string, pattern: RegExp | string): boolean {
export function isMatchingPattern(
value: string,
pattern: RegExp | string,
requireExactStringMatch: boolean = false,
): boolean {
if (!isString(value)) {
return false;
}

if (isRegExp(pattern)) {
return pattern.test(value);
}
if (typeof pattern === 'string') {
return value.indexOf(pattern) !== -1;
if (isString(pattern)) {
return requireExactStringMatch ? value === pattern : value.includes(pattern);
}

return false;
}

/**
* Test the given string against an array of strings and regexes. By default, string matching is done on a
* substring-inclusion basis rather than a strict equality basis
*
* @param testString The string to test
* @param patterns The patterns against which to test the string
* @param requireExactStringMatch If true, `testString` must match one of the given string patterns exactly in order to
* count. If false, `testString` will match a string pattern if it contains that pattern.
* @returns
*/
export function stringMatchesSomePattern(
testString: string,
patterns: Array<string | RegExp> = [],
requireExactStringMatch: boolean = false,
): boolean {
return patterns.some(pattern => isMatchingPattern(testString, pattern, requireExactStringMatch));
}

/**
* Given a string, escape characters which have meaning in the regex grammar, such that the result is safe to feed to
* `new RegExp()`.
Expand Down
67 changes: 65 additions & 2 deletions packages/utils/test/string.test.ts
@@ -1,4 +1,4 @@
import { isMatchingPattern, truncate } from '../src/string';
import { isMatchingPattern, stringMatchesSomePattern, truncate } from '../src/string';

describe('truncate()', () => {
test('it works as expected', () => {
Expand All @@ -18,13 +18,31 @@ describe('truncate()', () => {
});

describe('isMatchingPattern()', () => {
test('match using string substring', () => {
test('match using string substring if `requireExactStringMatch` not given', () => {
expect(isMatchingPattern('foobar', 'foobar')).toEqual(true);
expect(isMatchingPattern('foobar', 'foo')).toEqual(true);
expect(isMatchingPattern('foobar', 'bar')).toEqual(true);
expect(isMatchingPattern('foobar', 'nope')).toEqual(false);
});

test('match using string substring if `requireExactStringMatch` is `false`', () => {
expect(isMatchingPattern('foobar', 'foobar', false)).toEqual(true);
expect(isMatchingPattern('foobar', 'foo', false)).toEqual(true);
expect(isMatchingPattern('foobar', 'bar', false)).toEqual(true);
expect(isMatchingPattern('foobar', 'nope', false)).toEqual(false);
});

test('match using exact string match if `requireExactStringMatch` is `true`', () => {
expect(isMatchingPattern('foobar', 'foobar', true)).toEqual(true);
expect(isMatchingPattern('foobar', 'foo', true)).toEqual(false);
expect(isMatchingPattern('foobar', 'nope', true)).toEqual(false);
});

test('matches when `value` constains `pattern` but not vice-versa', () => {
expect(isMatchingPattern('foobar', 'foo')).toEqual(true);
expect(isMatchingPattern('foobar', 'foobarbaz')).toEqual(false);
});

test('match using regexp test', () => {
expect(isMatchingPattern('foobar', /^foo/)).toEqual(true);
expect(isMatchingPattern('foobar', /foo/)).toEqual(true);
Expand All @@ -45,3 +63,48 @@ describe('isMatchingPattern()', () => {
expect(isMatchingPattern([] as any, 'foo')).toEqual(false);
});
});

describe('stringMatchesSomePattern()', () => {
test('match using string substring if `requireExactStringMatch` not given', () => {
expect(stringMatchesSomePattern('foobar', ['foobar', 'nope'])).toEqual(true);
expect(stringMatchesSomePattern('foobar', ['foo', 'nope'])).toEqual(true);
expect(stringMatchesSomePattern('foobar', ['baz', 'nope'])).toEqual(false);
});

test('match using string substring if `requireExactStringMatch` is `false`', () => {
expect(stringMatchesSomePattern('foobar', ['foobar', 'nope'], false)).toEqual(true);
expect(stringMatchesSomePattern('foobar', ['foo', 'nope'], false)).toEqual(true);
expect(stringMatchesSomePattern('foobar', ['baz', 'nope'], false)).toEqual(false);
});

test('match using exact string match if `requireExactStringMatch` is `true`', () => {
expect(stringMatchesSomePattern('foobar', ['foobar', 'nope'], true)).toEqual(true);
expect(stringMatchesSomePattern('foobar', ['foo', 'nope'], true)).toEqual(false);
expect(stringMatchesSomePattern('foobar', ['baz', 'nope'], true)).toEqual(false);
});

test('matches when `testString` constains a pattern but not vice-versa', () => {
expect(stringMatchesSomePattern('foobar', ['foo', 'nope'])).toEqual(true);
expect(stringMatchesSomePattern('foobar', ['foobarbaz', 'nope'])).toEqual(false);
});

test('match using regexp test', () => {
expect(stringMatchesSomePattern('foobar', [/^foo/, 'nope'])).toEqual(true);
expect(stringMatchesSomePattern('foobar', [/foo/, 'nope'])).toEqual(true);
expect(stringMatchesSomePattern('foobar', [/b.{1}r/, 'nope'])).toEqual(true);
expect(stringMatchesSomePattern('foobar', [/^foo$/, 'nope'])).toEqual(false);
});

test('should match empty pattern as true', () => {
expect(stringMatchesSomePattern('foo', ['', 'nope'])).toEqual(true);
expect(stringMatchesSomePattern('bar', ['', 'nope'])).toEqual(true);
expect(stringMatchesSomePattern('', ['', 'nope'])).toEqual(true);
});

test('should bail out with false when given non-string value', () => {
expect(stringMatchesSomePattern(null as any, ['foo', 'nope'])).toEqual(false);
expect(stringMatchesSomePattern(undefined as any, ['foo', 'nope'])).toEqual(false);
expect(stringMatchesSomePattern({} as any, ['foo', 'nope'])).toEqual(false);
expect(stringMatchesSomePattern([] as any, ['foo', 'nope'])).toEqual(false);
});
});

0 comments on commit 6627882

Please sign in to comment.