Skip to content

Commit e77cb8f

Browse files
authored
feat(node) add max lineno and colno limits (#12514)
We want to have some safeguards in place to avoid reading minified files or large bundled files as we could incur a large processing time cost. The output of such files is likely to be less useful anyways, so contextlines would be low value
1 parent fdad379 commit e77cb8f

File tree

2 files changed

+56
-2
lines changed

2 files changed

+56
-2
lines changed

packages/node/src/integrations/contextlines.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ const LRU_FILE_CONTENTS_CACHE = new LRUMap<string, Record<number, string>>(10);
1010
const LRU_FILE_CONTENTS_FS_READ_FAILED = new LRUMap<string, 1>(20);
1111
const DEFAULT_LINES_OF_CONTEXT = 7;
1212
const INTEGRATION_NAME = 'ContextLines';
13+
// Determines the upper bound of lineno/colno that we will attempt to read. Large colno values are likely to be
14+
// minified code while large lineno values are likely to be bundled code.
15+
// Exported for testing purposes.
16+
export const MAX_CONTEXTLINES_COLNO: number = 1000;
17+
export const MAX_CONTEXTLINES_LINENO: number = 10000;
1318

1419
interface ContextLinesOptions {
1520
/**
@@ -58,6 +63,15 @@ function shouldSkipContextLinesForFile(path: string): boolean {
5863
if (path.startsWith('data:')) return true;
5964
return false;
6065
}
66+
67+
/**
68+
* Determines if we should skip contextlines based off the max lineno and colno values.
69+
*/
70+
function shouldSkipContextLinesForFrame(frame: StackFrame): boolean {
71+
if (frame.lineno !== undefined && frame.lineno > MAX_CONTEXTLINES_LINENO) return true;
72+
if (frame.colno !== undefined && frame.colno > MAX_CONTEXTLINES_COLNO) return true;
73+
return false;
74+
}
6175
/**
6276
* Checks if we have all the contents that we need in the cache.
6377
*/
@@ -216,7 +230,8 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
216230
!frame ||
217231
typeof filename !== 'string' ||
218232
typeof frame.lineno !== 'number' ||
219-
shouldSkipContextLinesForFile(filename)
233+
shouldSkipContextLinesForFile(filename) ||
234+
shouldSkipContextLinesForFrame(frame)
220235
) {
221236
continue;
222237
}

packages/node/test/integrations/contextlines.test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@ import * as fs from 'node:fs';
22
import type { StackFrame } from '@sentry/types';
33
import { parseStackFrames } from '@sentry/utils';
44

5-
import { _contextLinesIntegration, resetFileContentCache } from '../../src/integrations/contextlines';
5+
import {
6+
MAX_CONTEXTLINES_COLNO,
7+
MAX_CONTEXTLINES_LINENO,
8+
_contextLinesIntegration,
9+
resetFileContentCache,
10+
} from '../../src/integrations/contextlines';
611
import { defaultStackParser } from '../../src/sdk/api';
712
import { getError } from '../helpers/error';
813

@@ -22,6 +27,40 @@ describe('ContextLines', () => {
2227
jest.clearAllMocks();
2328
});
2429

30+
describe('limits', () => {
31+
test(`colno above ${MAX_CONTEXTLINES_COLNO}`, async () => {
32+
expect.assertions(1);
33+
const frames: StackFrame[] = [
34+
{
35+
colno: MAX_CONTEXTLINES_COLNO + 1,
36+
filename: 'file:///var/task/index.js',
37+
lineno: 1,
38+
function: 'fxn1',
39+
},
40+
];
41+
42+
const readStreamSpy = jest.spyOn(fs, 'createReadStream');
43+
await addContext(frames);
44+
expect(readStreamSpy).not.toHaveBeenCalled();
45+
});
46+
47+
test(`lineno above ${MAX_CONTEXTLINES_LINENO}`, async () => {
48+
expect.assertions(1);
49+
const frames: StackFrame[] = [
50+
{
51+
colno: 1,
52+
filename: 'file:///var/task/index.js',
53+
lineno: MAX_CONTEXTLINES_LINENO + 1,
54+
function: 'fxn1',
55+
},
56+
];
57+
58+
const readStreamSpy = jest.spyOn(fs, 'createReadStream');
59+
await addContext(frames);
60+
expect(readStreamSpy).not.toHaveBeenCalled();
61+
});
62+
});
63+
2564
describe('lru file cache', () => {
2665
test('parseStack when file does not exist', async () => {
2766
expect.assertions(4);

0 commit comments

Comments
 (0)