8000 feat(utils): use `req.cookies` if available instead of parsing (#2985) · szechyjs/sentry-javascript@83c8e45 · GitHub
[go: up one dir, main page]

Skip to content

Commit 83c8e45

Browse files
authored
feat(utils): use req.cookies if available instead of parsing (getsentry#2985)
1 parent b22791e commit 83c8e45

File tree

2 files changed

+179
-2
lines changed

2 files changed

+179
-2
lines changed

packages/utils/src/node.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ export function extractNodeRequestData(
7171
// url (including path and query string):
7272
// node, express: req.originalUrl
7373
// koa: req.url
74-
const originalUrl = (req.originalUrl || req.url) as string;
74+
const originalUrl = (req.originalUrl || req.url || '') as string;
7575
// absolute url
7676
const absoluteUrl = `${protocol}://${host}${originalUrl}`;
7777

@@ -89,8 +89,9 @@ export function extractNodeRequestData(
8989
case 'cookies':
9090
// cookies:
9191
// node, express, koa: req.headers.cookie
92+
// vercel, sails.js, express (w/ cookie middleware): req.cookies
9293
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
93-
requestData.cookies = dynamicRequire(module, 'cookie').parse(headers.cookie || '');
94+
requestData.cookies = req.cookies || dynamicRequire(module, 'cookie').parse(headers.cookie || '');
9495
break;
9596
case 'query_string':
9697
// query string:

packages/utils/test/node.test.ts

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
import { extractNodeRequestData } from '../src/node';
2+
3+
describe('extractNodeRequestData()', () => {
4+
describe('default behaviour', () => {
5+
test('node', () => {
6+
expect(
7+
extractNodeRequestData({
8+
headers: { host: 'example.com' },
9+
method: 'GET',
10+
secure: true,
11+
originalUrl: '/',
12+
}),
13+
).toEqual({
14+
cookies: {},
15+
headers: {
16+
host: 'example.com',
17+
},
18+
method: 'GET',
19+
query_string: null,
20+
url: 'https://example.com/',
21+
});
22+
});
23+
24+
test('degrades gracefully without request data', () => {
25+
expect(extractNodeRequestData({})).toEqual({
26+
cookies: {},
27+
headers: {},
28+
method: undefined,
29+
query_string: null,
30+
url: 'http://<no host>',
31+
});
32+
});
33+
});
34+
35+
describe('cookies', () => {
36+
it('uses `req.cookies` if available', () => {
37+
expect(
38+
extractNodeRequestData(
39+
{
40+
cookies: { foo: 'bar' },
41+
},
42+
['cookies'],
43+
),
44+
).toEqual({
45+
cookies: { foo: 'bar' },
46+
});
47+
});
48+
49+
it('parses the cookie header', () => {
50+
expect(
51+
extractNodeRequestData(
52+
{
53+
headers: {
54+
cookie: 'foo=bar;',
55+
},
56+
},
57+
['cookies'],
58+
),
59+
).toEqual({
60+
cookies: { foo: 'bar' },
61+
});
62+
});
63+
64+
it('falls back if no cookies are defined', () => {
65+
expect(extractNodeRequestData({}, ['cookies'])).toEqual({
66+
cookies: {},
67+
});
68+
});
69+
});
70+
71+
describe('data', () => {
72+
it('includes data from `req.body` if available', () => {
73+
expect(
74+
extractNodeRequestData(
75+
{
76+
method: 'POST',
77+
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
78+
body: 'foo=bar',
79+
},
80+
['data'],
81+
),
82+
).toEqual({
83+
data: 'foo=bar',
84+
});
85+
});
86+
87+
it('encodes JSON body contents back to a string', () => {
88+
expect(
89+
extractNodeRequestData(
90+
{
91+
method: 'POST',
92+
headers: { 'Content-Type': 'application/json' },
93+
body: { foo: 'bar' },
94+
},
95+
['data'],
96+
),
97+
).toEqual({
98+
data: '{"foo":"bar"}',
99+
});
100+
});
101+
});
102+
103+
describe('query_string', () => {
104+
it('parses the query parms from the url', () => {
105+
expect(
106+
extractNodeRequestData(
107+
{
108+
headers: { host: 'example.com' },
109+
secure: true,
110+
originalUrl: '/?foo=bar',
111+
},
112+
['query_string'],
113+
),
114+
).toEqual({
115+
query_string: 'foo=bar',
116+
});
117+
});
118+
119+
it('gracefully degrades if url cannot be determined', () => {
120+
expect(extractNodeRequestData({}, ['query_string'])).toEqual({
121+
query_string: null,
122+
});
123+
});
124+
});
125+
126+
describe('url', () => {
127+
test('express/koa', () => {
128+
expect(
129+
extractNodeRequestData(
130+
{
131+
host: 'example.com',
132+
protocol: 'https',
133+
url: '/',
134+
},
135+
['url'],
136+
),
137+
).toEqual({
138+
url: 'https://example.com/',
139+
});
140+
});
141+
142+
test('node', () => {
143+
expect(
144+
extractNodeRequestData(
145+
{
146+
headers: { host: 'example.com' },
147+
secure: true,
148+
originalUrl: '/',
149+
},
150+
['url'],
151+
),
152+
).toEqual({
153+
url: 'https://example.com/',
154+
});
155+
});
156+
});
157+
158+
describe('custom key', () => {
159+
it('includes the custom key if present', () => {
160+
expect(
161+
extractNodeRequestData(
162+
{
163+
httpVersion: '1.1',
164+
},
165+
['httpVersion'],
166+
),
167+
).toEqual({
168+
httpVersion: '1.1',
169+
});
170+
});
171+
172+
it('gracefully degrades if the custom key is missing', () => {
173+
expect(extractNodeRequestData({}, ['httpVersion'])).toEqual({});
174+
});
175+
});
176+
});

0 commit comments

Comments
 (0)
0