Skip to content

Commit e1e9402

Browse files
committed
Don't allow webhook request loops
1 parent e20111c commit e1e9402

File tree

1 file changed

+33
-1
lines changed

1 file changed

+33
-1
lines changed

src/rules/requests/request-step-impls.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1498,7 +1498,9 @@ const encodeWebhookBody = (body: Buffer) => {
14981498

14991499
export class WebhookStepImpl extends WebhookStep {
15001500

1501-
static readonly fromDefinition = copyDefinitionToImpl;
1501+
// Actually run the constructor, to ensure we initialize fields:
1502+
static readonly fromDefinition = (defn: WebhookStep) => new WebhookStepImpl(defn.url, defn.events);
1503+
protected outgoingSockets = new Set<net.Socket>();
15021504

15031505
private sendEvent(data: {
15041506
eventType: string;
@@ -1513,6 +1515,23 @@ export class WebhookStepImpl extends WebhookStep {
15131515
}
15141516
}).end(content);
15151517

1518+
req.on('socket', (socket) => {
1519+
if (this.outgoingSockets.has(socket)) return;
1520+
1521+
// Add this port to our list of active ports, once it's connected (before then it has no port)
1522+
if (socket.connecting) {
1523+
socket.once('connect', () => {
1524+
this.outgoingSockets.add(socket)
1525+
});
1526+
} else if (socket.localPort !== undefined) {
1527+
this.outgoingSockets.add(socket);
1528+
}
1529+
1530+
// Remove this port from our list of active ports when it's closed
1531+
// This is called for both clean closes & errors.
1532+
socket.once('close', () => this.outgoingSockets.delete(socket));
1533+
});
1534+
15161535
req.on('error', (e) => {
15171536
console.warn(`Error sending webhook to ${this.url}:`, e);
15181537
});
@@ -1528,6 +1547,19 @@ export class WebhookStepImpl extends WebhookStep {
15281547
}
15291548

15301549
async handle(request: OngoingRequest, response: OngoingResponse) {
1550+
if (isSocketLoop(this.outgoingSockets, (request as any).socket)) {
1551+
// We refuse to fire webhooks for incoming webhook requests, to avoid infinite loops in the (quite reasonable)
1552+
// case where you send webhooks to Mockttp/HTTP Toolkit to debug their behaviour. Better to just return
1553+
// an error - you can still see the data, but the request isn't going anywhere (including via later steps).
1554+
throw new Error(oneLine`
1555+
Webhook loop detected. This probably means you're triggering a webhook to be sent directly back
1556+
to the server, which is matching the same rule and triggering another webhook, which is...` +
1557+
'\n\n' + oneLine`
1558+
You should either add a separate higher priority rule to match & handle requests for this URL
1559+
(${request.url}) or change the webhook URL.
1560+
`);
1561+
}
1562+
15311563
if (this.events.includes('request')) {
15321564
waitForCompletedRequest(request).then((completedReq) => {
15331565
const eventData = {

0 commit comments

Comments
 (0)