Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(util): Add stringMatchesSomePattern helper function. #6205

Merged
merged 3 commits into from Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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);
});
});