Skip to content

Commit 3acc9ce

Browse files
pimterrypimterry
committed
Handle errors in request preprocessing (particularly URLs)
Previously a totally invalid URL (generally caused by an invalid Host header) would throw an uncaught error - now we treat it like any other client error.
1 parent 75ddb7b commit 3acc9ce

File tree

2 files changed

+187
-129
lines changed

2 files changed

+187
-129
lines changed

‎src/server/mockttp-server.ts

Lines changed: 169 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ import {
3232
RuleEvent,
3333
RawTrailers,
3434
RawPassthroughEvent,
35-
RawPassthroughDataEvent
35+
RawPassthroughDataEvent,
36+
RawHeaders
3637
} from "../types";
3738
import { DestroyableServer } from "destroyable-server";
3839
import {
@@ -596,141 +597,170 @@ export class MockttpServer extends AbstractMockttp implements Mockttp {
596597
* For both normal requests & websockets, we do some standard preprocessing to ensure we have the absolute
597598
* URL destination in place, and timing, tags & id metadata all ready for an OngoingRequest.
598599
*/
599-
private preprocessRequest(req: ExtendedRawRequest, type: 'request' | 'websocket'): OngoingRequest {
600-
parseRequestBody(req, { maxSize: this.maxBodySize });
601-
602-
let rawHeaders = pairFlatRawHeaders(req.rawHeaders);
603-
let socketMetadata: SocketMetadata | undefined = req.socket[SocketMetadata];
604-
605-
// Make req.url always absolute, if it isn't already, using the host header.
606-
// It might not be if this is a direct request, or if it's being transparently proxied.
607-
if (!isAbsoluteUrl(req.url!)) {
608-
req.protocol = getHeaderValue(rawHeaders, ':scheme') ||
609-
(req.socket[LastHopEncrypted] ? 'https' : 'http');
610-
req.path = req.url;
611-
612-
const tunnelDestination = req.socket[LastTunnelAddress]
613-
? getDestination(req.protocol, req.socket[LastTunnelAddress])
614-
: undefined;
615-
616-
const isTunnelToIp = !!tunnelDestination && isIP(tunnelDestination.hostname);
617-
618-
const urlDestination = getDestination(req.protocol,
619-
(!isTunnelToIp
620-
? (
621-
req.socket[LastTunnelAddress] ?? // Tunnel domain name is preferred if available
622-
getHeaderValue(rawHeaders, ':authority') ??
623-
getHeaderValue(rawHeaders, 'host') ??
624-
req.socket[TlsMetadata]?.sniHostname
625-
)
626-
: (
627-
getHeaderValue(rawHeaders, ':authority') ??
628-
getHeaderValue(rawHeaders, 'host') ??
629-
req.socket[TlsMetadata]?.sniHostname ??
630-
req.socket[LastTunnelAddress] // We use the IP iff we have no hostname available at all
631-
))
632-
?? `localhost:${this.port}` // If you specify literally nothing, it's a direct request
633-
);
600+
private preprocessRequest(req: ExtendedRawRequest, type: 'request' | 'websocket'): OngoingRequest | null {
601+
try {
602+
parseRequestBody(req, { maxSize: this.maxBodySize });
603+
604+
let rawHeaders = pairFlatRawHeaders(req.rawHeaders);
605+
let socketMetadata: SocketMetadata | undefined = req.socket[SocketMetadata];
606+
607+
// Make req.url always absolute, if it isn't already, using the host header.
608+
// It might not be if this is a direct request, or if it's being transparently proxied.
609+
if (!isAbsoluteUrl(req.url!)) {
610+
req.protocol = getHeaderValue(rawHeaders, ':scheme') ||
611+
(req.socket[LastHopEncrypted] ? 'https' : 'http');
612+
req.path = req.url;
613+
614+
const tunnelDestination = req.socket[LastTunnelAddress]
615+
? getDestination(req.protocol, req.socket[LastTunnelAddress])
616+
: undefined;
617+
618+
const isTunnelToIp = !!tunnelDestination && isIP(tunnelDestination.hostname);
619+
620+
const urlDestination = getDestination(req.protocol,
621+
(!isTunnelToIp
622+
? (
623+
req.socket[LastTunnelAddress] ?? // Tunnel domain name is preferred if available
624+
getHeaderValue(rawHeaders, ':authority') ??
625+
getHeaderValue(rawHeaders, 'host') ??
626+
req.socket[TlsMetadata]?.sniHostname
627+
)
628+
: (
629+
getHeaderValue(rawHeaders, ':authority') ??
630+
getHeaderValue(rawHeaders, 'host') ??
631+
req.socket[TlsMetadata]?.sniHostname ??
632+
req.socket[LastTunnelAddress] // We use the IP iff we have no hostname available at all
633+
))
634+
?? `localhost:${this.port}` // If you specify literally nothing, it's a direct request
635+
);
634636

635-
// Actual destination always follows the tunnel - even if it's an IP
636-
req.destination = tunnelDestination
637-
?? urlDestination;
637+
// Actual destination always follows the tunnel - even if it's an IP
638+
req.destination = tunnelDestination
639+
?? urlDestination;
638640

639-
// URL port should always match the real port - even if (e.g) the Host header is lying.
640-
urlDestination.port = req.destination.port;
641+
// URL port should always match the real port - even if (e.g) the Host header is lying.
642+
urlDestination.port = req.destination.port;
641643

642-
const absoluteUrl = `${req.protocol}://${
643-
normalizeHost(req.protocol, `${urlDestination.hostname}:${urlDestination.port}`)
644-
}${req.path}`;
644+
const absoluteUrl = `${req.protocol}://${
645+
normalizeHost(req.protocol, `${urlDestination.hostname}:${urlDestination.port}`)
646+
}${req.path}`;
647+
648+
let effectiveUrl: string;
649+
try {
650+
effectiveUrl = new URL(absoluteUrl).toString();
651+
} catch (e: any) {
652+
req.url = absoluteUrl;
653+
throw e;
654+
}
645655

646-
if (!getHeaderValue(rawHeaders, ':path')) {
647-
(req as Mutable<ExtendedRawRequest>).url = new url.URL(absoluteUrl).toString();
656+
if (!getHeaderValue(rawHeaders, ':path')) {
657+
(req as Mutable<ExtendedRawRequest>).url = effectiveUrl;
658+
} else {
659+
// Node's HTTP/2 compat logic maps .url to headers[':path']. We want them to
660+
// diverge: .url should always be absolute, while :path may stay relative,
661+
// so we override the built-in getter & setter:
662+
Object.defineProperty(req, 'url', {
663+
value: effectiveUrl
664+
});
665+
}
648666
} else {
649-
// Node's HTTP/2 compat logic maps .url to headers[':path']. We want them to
650-
// diverge: .url should always be absolute, while :path may stay relative,
651-
// so we override the built-in getter & setter:
667+
// We have an absolute request. This is effectively a combined tunnel + end-server request,
668+
// so we need to handle both of those, and hide the proxy-specific bits from later logic.
669+
req.protocol = req.url!.split('://', 1)[0];
670+
req.path = getPathFromAbsoluteUrl(req.url!);
671+
req.destination = getDestination(
672+
req.protocol,
673+
req.socket[LastTunnelAddress] ?? getHostFromAbsoluteUrl(req.url!)
674+
);
675+
676+
const proxyAuthHeader = getHeaderValue(rawHeaders, 'proxy-authorization');
677+
if (proxyAuthHeader) {
678+
// Use this metadata for this request, but _only_ this request - it's not relevant
679+
// to other requests on the same socket so we don't add it to req.socket.
680+
socketMetadata = getSocketMetadataFromProxyAuth(req.socket, proxyAuthHeader);
681+
}
682+
683+
rawHeaders = rawHeaders.filter(([key]) => {
684+
const lcKey = key.toLowerCase();
685+
return lcKey !== 'proxy-connection' &&
686+
lcKey !== 'proxy-authorization';
687+
})
688+
}
689+
690+
if (type === 'websocket') {
691+
req.protocol = req.protocol === 'https'
692+
? 'wss'
693+
: 'ws';
694+
695+
// Transform the protocol in req.url too:
652696
Object.defineProperty(req, 'url', {
653-
value: new url.URL(absoluteUrl).toString()
697+
value: req.url!.replace(/^http/, 'ws')
654698
});
655699
}
656-
} else {
657-
// We have an absolute request. This is effectively a combined tunnel + end-server request,
658-
// so we need to handle both of those, and hide the proxy-specific bits from later logic.
659-
req.protocol = req.url!.split('://', 1)[0];
660-
req.path = getPathFromAbsoluteUrl(req.url!);
661-
req.destination = getDestination(
662-
req.protocol,
663-
req.socket[LastTunnelAddress] ?? getHostFromAbsoluteUrl(req.url!)
664-
);
665700

666-
const proxyAuthHeader = getHeaderValue(rawHeaders, 'proxy-authorization');
667-
if (proxyAuthHeader) {
668-
// Use this metadata for this request, but _only_ this request - it's not relevant
669-
// to other requests on the same socket so we don't add it to req.socket.
670-
socketMetadata = getSocketMetadataFromProxyAuth(req.socket, proxyAuthHeader);
671-
}
701+
const id = crypto.randomUUID();
672702

673-
rawHeaders = rawHeaders.filter(([key]) => {
674-
const lcKey = key.toLowerCase();
675-
return lcKey !== 'proxy-connection' &&
676-
lcKey !== 'proxy-authorization';
677-
})
678-
}
703+
const tags: string[] = getSocketMetadataTags(socketMetadata);
679704

680-
if (type === 'websocket') {
681-
req.protocol = req.protocol === 'https'
682-
? 'wss'
683-
: 'ws';
705+
const timingEvents: TimingEvents = {
706+
startTime: Date.now(),
707+
startTimestamp: now()
708+
};
684709

685-
// Transform the protocol in req.url too:
686-
Object.defineProperty(req, 'url', {
687-
value: req.url!.replace(/^http/, 'ws')
710+
req.on('end', () => {
711+
timingEvents.bodyReceivedTimestamp ||= now();
688712
});
689-
}
690-
691-
const id = crypto.randomUUID();
692713

693-
const tags: string[] = getSocketMetadataTags(socketMetadata);
714+
const headers = rawHeadersToObject(rawHeaders);
694715

695-
const timingEvents: TimingEvents = {
696-
startTime: Date.now(),
697-
startTimestamp: now()
698-
};
716+
// Not writable for HTTP/2:
717+
makePropertyWritable(req, 'headers');
718+
makePropertyWritable(req, 'rawHeaders');
699719

700-
req.on('end', () => {
701-
timingEvents.bodyReceivedTimestamp ||= now();
702-
});
720+
let rawTrailers: RawTrailers | undefined;
721+
Object.defineProperty(req, 'rawTrailers', {
722+
get: () => rawTrailers,
723+
set: (flatRawTrailers) => {
724+
rawTrailers = flatRawTrailers
725+
? pairFlatRawHeaders(flatRawTrailers)
726+
: undefined;
727+
}
728+
});
703729

704-
const headers = rawHeadersToObject(rawHeaders);
730+
return Object.assign(req, {
731+
id,
732+
headers,
733+
rawHeaders,
734+
rawTrailers, // Just makes the type happy - really managed by property above
735+
remoteIpAddress: req.socket.remoteAddress,
736+
remotePort: req.socket.remotePort,
737+
timingEvents,
738+
tags
739+
}) as OngoingRequest;
740+
} catch (e: any) {
741+
const error: Error = Object.assign(e, {
742+
code: e.code ?? 'PREPROCESSING_FAILED',
743+
badRequest: req
744+
});
705745

706-
// Not writable for HTTP/2:
707-
makePropertyWritable(req, 'headers');
708-
makePropertyWritable(req, 'rawHeaders');
746+
const h2Session = req.httpVersionMajor > 1 &&
747+
(req as any).stream?.session;
709748

710-
let rawTrailers: RawTrailers | undefined;
711-
Object.defineProperty(req, 'rawTrailers', {
712-
get: () => rawTrailers,
713-
set: (flatRawTrailers) => {
714-
rawTrailers = flatRawTrailers
715-
? pairFlatRawHeaders(flatRawTrailers)
716-
: undefined;
749+
if (h2Session) {
750+
this.handleInvalidHttp2Request(error, h2Session);
751+
} else {
752+
this.handleInvalidHttp1Request(error, req.socket)
717753
}
718-
});
719754

720-
return Object.assign(req, {
721-
id,
722-
headers,
723-
rawHeaders,
724-
rawTrailers, // Just makes the type happy - really managed by property above
725-
remoteIpAddress: req.socket.remoteAddress,
726-
remotePort: req.socket.remotePort,
727-
timingEvents,
728-
tags
729-
}) as OngoingRequest;
755+
return null; // Null -> preprocessing failed, error already handled here
756+
}
757+
730758
}
731759

732760
private async handleRequest(rawRequest: ExtendedRawRequest, rawResponse: http.ServerResponse) {
733761
const request = this.preprocessRequest(rawRequest, 'request');
762+
if (request === null) return; // Preprocessing failed - don't handle this
763+
734764
if (this.debug) console.log(`Handling request for ${rawRequest.url}`);
735765

736766
let result: 'responded' | 'aborted' | null = null;
@@ -824,9 +854,10 @@ export class MockttpServer extends AbstractMockttp implements Mockttp {
824854
}
825855

826856
private async handleWebSocket(rawRequest: ExtendedRawRequest, socket: net.Socket, head: Buffer) {
827-
if (this.debug) console.log(`Handling websocket for ${rawRequest.url}`);
828-
829857
const request = this.preprocessRequest(rawRequest, 'websocket');
858+
if (request === null) return; // Preprocessing failed - don't handle this
859+
860+
if (this.debug) console.log(`Handling websocket for ${rawRequest.url}`);
830861

831862
socket.on('error', (error) => {
832863
console.log('Response error:', this.debug ? error : error.message);
@@ -1008,7 +1039,7 @@ ${await this.suggestRule(request)}`
10081039
// Called on server clientError, e.g. if the client disconnects during initial
10091040
// request data, or sends totally invalid gibberish. Only called for HTTP/1.1 errors.
10101041
private handleInvalidHttp1Request(
1011-
error: Error & { code?: string, rawPacket?: Buffer },
1042+
error: Error & { code?: string, rawPacket?: Buffer, badRequest?: ExtendedRawRequest },
10121043
socket: net.Socket
10131044
) {
10141045
if (socket[ClientErrorInProgress]) {
@@ -1065,12 +1096,18 @@ ${await this.suggestRule(request)}`
10651096
?? Buffer.from([]);
10661097

10671098
// For packets where we get more than just httpolyglot-peeked data, guess-parse them:
1068-
const parsedRequest = rawPacket.byteLength > 1
1069-
? tryToParseHttpRequest(rawPacket, socket)
1070-
: {};
1099+
const parsedRequest = error.badRequest ??
1100+
(rawPacket.byteLength > 1
1101+
? tryToParseHttpRequest(rawPacket, socket)
1102+
: {}
1103+
);
10711104

10721105
if (isHeaderOverflow) commonParams.tags.push('header-overflow');
10731106

1107+
const rawHeaders = parsedRequest.rawHeaders?.[0] && typeof parsedRequest.rawHeaders[0] === 'string'
1108+
? pairFlatRawHeaders(parsedRequest.rawHeaders as string[])
1109+
: parsedRequest.rawHeaders as RawHeaders | undefined;
1110+
10741111
const request: ClientError['request'] = {
10751112
...commonParams,
10761113
httpVersion: parsedRequest.httpVersion || '1.1',
@@ -1079,7 +1116,7 @@ ${await this.suggestRule(request)}`
10791116
url: parsedRequest.url,
10801117
path: parsedRequest.path,
10811118
headers: parsedRequest.headers || {},
1082-
rawHeaders: parsedRequest.rawHeaders || [],
1119+
rawHeaders: rawHeaders || [],
10831120
remoteIpAddress: socket.remoteAddress,
10841121
remotePort: socket.remotePort,
10851122
destination: parsedRequest.destination
@@ -1131,7 +1168,7 @@ ${await this.suggestRule(request)}`
11311168
// Handle HTTP/2 client errors. This is a work in progress, but usefully reports
11321169
// some of the most obvious cases.
11331170
private handleInvalidHttp2Request(
1134-
error: Error & { code?: string, errno?: number },
1171+
error: Error & { code?: string, errno?: number, badRequest?: ExtendedRawRequest },
11351172
session: http2.Http2Session
11361173
) {
11371174
// Unlike with HTTP/1.1, we have no control of the actual handling of
@@ -1142,6 +1179,10 @@ ${await this.suggestRule(request)}`
11421179

11431180
const isBadPreface = (error.errno === -903);
11441181

1182+
const rawHeaders = error.badRequest?.rawHeaders?.[0] && typeof error.badRequest?.rawHeaders[0] === 'string'
1183+
? pairFlatRawHeaders(error.badRequest?.rawHeaders as string[])
1184+
: error.badRequest?.rawHeaders as RawHeaders | undefined;
1185+
11451186
this.announceClientErrorAsync(session.initialSocket, {
11461187
errorCode: error.code,
11471188
request: {
@@ -1151,19 +1192,18 @@ ${await this.suggestRule(request)}`
11511192
...(isBadPreface ? ['client-error:bad-preface'] : []),
11521193
...getSocketMetadataTags(socket?.[SocketMetadata])
11531194
],
1154-
httpVersion: '2',
1195+
httpVersion: error.badRequest?.httpVersion ?? '2',
11551196

11561197
// Best guesses:
11571198
timingEvents: { startTime: Date.now(), startTimestamp: now() },
1158-
protocol: isTLS ? "https" : "http",
1159-
url: isTLS ? `https://${
1160-
(socket as tls.TLSSocket).servername // Use the hostname from SNI
1161-
}/` : undefined,
1162-
1163-
// Unknowable:
1164-
path: undefined,
1165-
headers: {},
1166-
rawHeaders: []
1199+
protocol: error.badRequest?.protocol || (isTLS ? "https" : "http"),
1200+
url: error.badRequest?.url ||
1201+
(isTLS ? `https://${(socket as tls.TLSSocket).servername}/` : undefined),
1202+
1203+
path: error.badRequest?.path,
1204+
headers: error.badRequest?.headers || {},
1205+
rawHeaders: rawHeaders || [],
1206+
destination: error.badRequest?.destination
11671207
},
11681208
response: 'aborted' // These h2 errors get no app-level response, just a shutdown.
11691209
});

0 commit comments

Comments
 (0)