Skip to content

Commit e830880

Browse files
fix(playground): Harden open-url middleware and add tests
- Return 405 for non-POST requests instead of hanging - Validate URL scheme (http/https only) - Improve error messages when `open` package is unavailable - Fix inconsistent oxlint disable comments - Add tests for openURLMiddleware and open-url routing Co-Authored-By: Krystof Woldrich <krystofwoldrich@gmail.com>
1 parent ba5474e commit e830880

File tree

3 files changed

+174
-38
lines changed

3 files changed

+174
-38
lines changed

packages/core/src/js/metro/openUrlMiddleware.ts

Lines changed: 46 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ let open: ((url: string) => Promise<void>) | undefined = undefined;
1515
* Inspired by https://github.com/react-native-community/cli/blob/a856ce027a6b25f9363a8689311cdd4416c0fc89/packages/cli-server-api/src/openURLMiddleware.ts#L17
1616
*/
1717
export async function openURLMiddleware(req: IncomingMessage, res: ServerResponse): Promise<void> {
18+
if (req.method !== 'POST') {
19+
res.writeHead(405);
20+
res.end('Method not allowed. Use POST.');
21+
return;
22+
}
23+
1824
if (!open) {
1925
try {
2026
// oxlint-disable-next-line import/no-extraneous-dependencies
@@ -24,48 +30,50 @@ export async function openURLMiddleware(req: IncomingMessage, res: ServerRespons
2430
}
2531
}
2632

27-
if (req.method === 'POST') {
28-
const body = await getRawBody(req);
29-
let url: string | undefined = undefined;
30-
31-
try {
32-
const parsedBody = JSON.parse(body) as { url?: string };
33-
url = parsedBody.url;
34-
} catch (e) {
35-
res.writeHead(400);
36-
res.end('Invalid request body. Expected a JSON object with a url key.');
37-
return;
38-
}
39-
40-
try {
41-
if (!url) {
42-
res.writeHead(400);
43-
res.end('Invalid request body. Expected a JSON object with a url key.');
44-
return;
45-
}
33+
const body = await getRawBody(req);
34+
let url: string | undefined = undefined;
4635

47-
if (!open) {
48-
throw new Error('The "open" module is not available.');
49-
}
36+
try {
37+
const parsedBody = JSON.parse(body) as { url?: string };
38+
url = parsedBody.url;
39+
} catch (e) {
40+
res.writeHead(400);
41+
res.end('Invalid request body. Expected a JSON object with a url key.');
42+
return;
43+
}
5044

51-
await open(url);
52-
} catch (e) {
53-
// oxlint-disable-next-line no-console
54-
console.log(`${S} Open: ${url}`);
45+
if (!url) {
46+
res.writeHead(400);
47+
res.end('Invalid request body. Expected a JSON object with a url key.');
48+
return;
49+
}
5550

56-
res.writeHead(500);
51+
if (!url.startsWith('https://') && !url.startsWith('http://')) {
52+
res.writeHead(400);
53+
res.end('Invalid URL scheme. Only http:// and https:// URLs are allowed.');
54+
return;
55+
}
5756

58-
if (!open) {
59-
res.end('Failed to open URL. The "open" module is not available.');
60-
} else {
61-
res.end('Failed to open URL.');
62-
}
63-
return;
64-
}
57+
if (!open) {
58+
// oxlint-disable-next-line no-console
59+
console.log(`${S} Could not open URL automatically. Open manually: ${url}`);
60+
res.writeHead(500);
61+
res.end('Failed to open URL. The "open" package is not available. Install it or open the URL manually.');
62+
return;
63+
}
6564

66-
// eslint-disable-next-line no-console
67-
console.log(`${S} Opened URL: ${url}`);
68-
res.writeHead(200);
69-
res.end();
65+
try {
66+
await open(url);
67+
} catch (e) {
68+
// oxlint-disable-next-line no-console
69+
console.log(`${S} Failed to open URL automatically. Open manually: ${url}`);
70+
res.writeHead(500);
71+
res.end('Failed to open URL.');
72+
return;
7073
}
74+
75+
// oxlint-disable-next-line no-console
76+
console.log(`${S} Opened URL: ${url}`);
77+
res.writeHead(200);
78+
res.end();
7179
}

packages/core/test/tools/metroMiddleware.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { StackFrame } from '@sentry/core';
33
import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals';
44
import * as fs from 'fs';
55

6+
import * as openUrlMiddlewareModule from '../../src/js/metro/openUrlMiddleware';
67
import * as metroMiddleware from '../../src/js/tools/metroMiddleware';
78

89
const { withSentryMiddleware, createSentryMetroMiddleware, stackFramesContextMiddleware } = metroMiddleware;
@@ -83,6 +84,24 @@ describe('metroMiddleware', () => {
8384
expect(spiedStackFramesContextMiddleware).toHaveBeenCalledWith(sentryRequest, response, next);
8485
});
8586

87+
it('should call openURLMiddleware for sentry open-url requests', () => {
88+
const spiedOpenURLMiddleware = jest
89+
.spyOn(openUrlMiddlewareModule, 'openURLMiddleware')
90+
.mockReturnValue(undefined as any);
91+
92+
const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware);
93+
94+
const openUrlRequest = {
95+
url: '/__sentry/open-url',
96+
} as any;
97+
testedMiddleware(openUrlRequest, response, next);
98+
expect(defaultMiddleware).not.toHaveBeenCalled();
99+
expect(spiedStackFramesContextMiddleware).not.toHaveBeenCalled();
100+
expect(spiedOpenURLMiddleware).toHaveBeenCalledWith(openUrlRequest, response);
101+
102+
spiedOpenURLMiddleware.mockRestore();
103+
});
104+
86105
it('should call default middleware for non-sentry requests', () => {
87106
const testedMiddleware = createSentryMetroMiddleware(defaultMiddleware);
88107

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import { describe, expect, it, jest, beforeEach, afterEach } from '@jest/globals';
2+
3+
import { openURLMiddleware } from '../../src/js/metro/openUrlMiddleware';
4+
5+
jest.mock('../../src/js/metro/openUrlMiddleware', () => jest.requireActual('../../src/js/metro/openUrlMiddleware'));
6+
7+
let mockOpen: jest.Mock | undefined;
8+
9+
jest.mock('open', () => {
10+
mockOpen = jest.fn().mockResolvedValue(undefined);
11+
return mockOpen;
12+
});
13+
14+
function createRequest(method: string, body: string) {
15+
return {
16+
method,
17+
on: jest.fn((event: string, cb: (data?: string) => void) => {
18+
if (event === 'data') {
19+
cb(body);
20+
} else if (event === 'end') {
21+
cb();
22+
}
23+
}),
24+
} as any;
25+
}
26+
27+
function createResponse() {
28+
return {
29+
writeHead: jest.fn(),
30+
end: jest.fn(),
31+
} as any;
32+
}
33+
34+
describe('openURLMiddleware', () => {
35+
beforeEach(() => {
36+
jest.clearAllMocks();
37+
});
38+
39+
afterEach(() => {
40+
jest.restoreAllMocks();
41+
});
42+
43+
it('should return 405 for non-POST requests', async () => {
44+
const req = createRequest('GET', '');
45+
const res = createResponse();
46+
47+
await openURLMiddleware(req, res);
48+
49+
expect(res.writeHead).toHaveBeenCalledWith(405);
50+
});
51+
52+
it('should return 400 for invalid JSON body', async () => {
53+
const req = createRequest('POST', 'not json');
54+
const res = createResponse();
55+
56+
await openURLMiddleware(req, res);
57+
58+
expect(res.writeHead).toHaveBeenCalledWith(400);
59+
});
60+
61+
it('should return 400 for missing url in body', async () => {
62+
const req = createRequest('POST', '{}');
63+
const res = createResponse();
64+
65+
await openURLMiddleware(req, res);
66+
67+
expect(res.writeHead).toHaveBeenCalledWith(400);
68+
});
69+
70+
it('should return 400 for non-http URL schemes', async () => {
71+
const req = createRequest('POST', JSON.stringify({ url: 'file:///etc/passwd' }));
72+
const res = createResponse();
73+
74+
await openURLMiddleware(req, res);
75+
76+
expect(res.writeHead).toHaveBeenCalledWith(400);
77+
expect(res.end).toHaveBeenCalledWith(expect.stringContaining('Invalid URL scheme'));
78+
});
79+
80+
it('should open https URLs', async () => {
81+
const req = createRequest('POST', JSON.stringify({ url: 'https://sentry.io/issues/' }));
82+
const res = createResponse();
83+
84+
await openURLMiddleware(req, res);
85+
86+
expect(mockOpen).toHaveBeenCalledWith('https://sentry.io/issues/');
87+
expect(res.writeHead).toHaveBeenCalledWith(200);
88+
});
89+
90+
it('should open http URLs', async () => {
91+
const req = createRequest('POST', JSON.stringify({ url: 'http://localhost:3000' }));
92+
const res = createResponse();
93+
94+
await openURLMiddleware(req, res);
95+
96+
expect(mockOpen).toHaveBeenCalledWith('http://localhost:3000');
97+
expect(res.writeHead).toHaveBeenCalledWith(200);
98+
});
99+
100+
it('should return 500 when open fails', async () => {
101+
mockOpen!.mockRejectedValueOnce(new Error('open failed'));
102+
const req = createRequest('POST', JSON.stringify({ url: 'https://sentry.io/' }));
103+
const res = createResponse();
104+
105+
await openURLMiddleware(req, res);
106+
107+
expect(res.writeHead).toHaveBeenCalledWith(500);
108+
});
109+
});

0 commit comments

Comments
 (0)