Skip to content

Commit 3170688

Browse files
blakebyrnesblakebyrnes
committed
fix(mitm): empty headers bug, clean errors
1 parent 98da380 commit 3170688

File tree

7 files changed

+121
-86
lines changed

7 files changed

+121
-86
lines changed

‎mitm-socket/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export default class MitmSocket extends TypedEventEmitter<{
8787
const socket = net.connect(this.socketPath);
8888
this.socket = socket;
8989
socket.on('error', error => {
90-
this.logger.error('SocketConnectDriver.SocketError', {
90+
this.logger.warn('SocketConnectDriver.SocketError', {
9191
sessionId: this.sessionId,
9292
error,
9393
socketPath: this.socketPath,

‎mitm/handlers/BaseHttpHandler.ts‎

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import Log from '@secret-agent/commons/Logger';
22
import * as http from 'http';
33
import * as http2 from 'http2';
4+
import { ClientHttp2Stream } from 'http2';
45
import IMitmRequestContext from '../interfaces/IMitmRequestContext';
56
import BlockHandler from './BlockHandler';
67
import HeadersHandler from './HeadersHandler';
@@ -59,25 +60,37 @@ export default abstract class BaseHttpHandler {
5960
// do one more check on the session before doing a connect
6061
if (session.isClosing) return context.setState(ResourceState.SessionClosed);
6162

62-
this.context.proxyToServerRequest = await session.requestAgent.request(context);
63-
this.context.proxyToServerRequest.on(
64-
'error',
65-
this.onError.bind(this, 'ProxyToServer.RequestError'),
66-
);
67-
const stream = <http2.Http2Stream>this.context.proxyToServerRequest;
68-
stream.on('streamClosed', code =>
69-
this.onError(
70-
'ProxyToH2Server.StreamClosedError',
71-
new Error(`Stream closed with code ${code}`),
72-
),
73-
);
74-
if (stream.session && stream.session.listenerCount('error') === 0) {
75-
stream.session?.on('error', this.onError.bind(this, 'ProxyToServer.SessionError'));
63+
const request = await session.requestAgent.request(context);
64+
this.context.proxyToServerRequest = request;
65+
request.on('error', this.onError.bind(this, 'ProxyToServer.RequestError'));
66+
67+
if (this.context.isServerHttp2) {
68+
const h2Request = request as ClientHttp2Stream;
69+
this.bindHttp2ErrorListeners('ProxyToH2Server', h2Request, h2Request.session);
7670
}
7771

7872
return this.context.proxyToServerRequest;
7973
} catch (err) {
8074
this.onError('ProxyToServer.RequestHandlerError', err);
8175
}
8276
}
77+
78+
protected bindHttp2ErrorListeners(
79+
source: string,
80+
stream: http2.Http2Stream,
81+
session: http2.Http2Session,
82+
): void {
83+
if (!stream.listenerCount('error')) {
84+
stream.on('error', this.onError.bind(this, `${source}.Http2StreamError`));
85+
}
86+
87+
stream.on('streamClosed', code => {
88+
if (!code) return;
89+
this.onError(`${source}.Http2StreamError`, new Error(`Stream Closed ${code}`));
90+
});
91+
92+
if (session && !session.listenerCount('error')) {
93+
session.on('error', this.onError.bind(this, `${source}.Http2SessionError`));
94+
}
95+
}
8396
}

‎mitm/handlers/HeadersHandler.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@ import * as http from 'http';
55
import * as http2 from 'http2';
66
import OriginType from '@secret-agent/core-interfaces/OriginType';
77
import ResourceType from '@secret-agent/core-interfaces/ResourceType';
8+
import { URL } from 'url';
89
import { parseRawHeaders } from '../lib/Utils';
910
import IMitmRequestContext from '../interfaces/IMitmRequestContext';
1011
import ResourceState from '../interfaces/ResourceState';
1112

13+
const redirectCodes = new Set([300, 301, 302, 303, 305, 307, 308]);
14+
1215
export default class HeadersHandler {
1316
public static async determineResourceType(ctx: IMitmRequestContext): Promise<void> {
1417
ctx.setState(ResourceState.DetermineResourceType);
@@ -107,6 +110,15 @@ export default class HeadersHandler {
107110
return headers;
108111
}
109112

113+
public static checkForRedirectResponseLocation(context: IMitmRequestContext): URL {
114+
if (redirectCodes.has(context.status)) {
115+
const redirectLocation = context.responseHeaders.location || context.responseHeaders.Location;
116+
if (redirectLocation) {
117+
return new URL(redirectLocation as string, context.url);
118+
}
119+
}
120+
}
121+
110122
public static sendRequestTrailers(ctx: IMitmRequestContext): void {
111123
const clientRequest = ctx.clientToProxyRequest;
112124
if (!clientRequest.trailers) return;

‎mitm/handlers/HttpRequestHandler.ts‎

Lines changed: 58 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as http from 'http';
22
import Log, { hasBeenLoggedSymbol } from '@secret-agent/commons/Logger';
33
import * as http2 from 'http2';
44
import { ClientHttp2Stream, Http2ServerRequest, Http2ServerResponse } from 'http2';
5-
import { URL } from 'url';
65
import { CanceledPromiseError } from '@secret-agent/commons/interfaces/IPendingWaitEvent';
76
import IMitmRequestContext from '../interfaces/IMitmRequestContext';
87
import HeadersHandler from './HeadersHandler';
@@ -14,7 +13,6 @@ import HttpResponseCache from '../lib/HttpResponseCache';
1413
import ResourceState from '../interfaces/ResourceState';
1514

1615
const { log } = Log(module);
17-
const redirectCodes = new Set([300, 301, 302, 303, 305, 307, 308]);
1816

1917
export default class HttpRequestHandler extends BaseHttpHandler {
2018
protected static responseCache = new HttpResponseCache();
@@ -29,32 +27,7 @@ export default class HttpRequestHandler extends BaseHttpHandler {
2927
this.context.setState(ResourceState.ClientToProxyRequest);
3028

3129
// register error listeners first
32-
const { clientToProxyRequest, proxyToClientResponse } = request;
33-
clientToProxyRequest.on('error', this.onError.bind(this, 'ClientToProxy.RequestError'));
34-
if (clientToProxyRequest instanceof Http2ServerRequest) {
35-
clientToProxyRequest.stream.on(
36-
'error',
37-
this.onError.bind(this, 'ClientToProxy.Http2StreamError'),
38-
);
39-
clientToProxyRequest.stream.on('streamClosed', code =>
40-
this.onError('ClientToProxy.Http2StreamError', new Error(`Stream Closed ${code}`)),
41-
);
42-
const http2Session = clientToProxyRequest.stream.session;
43-
if (!http2Session.listenerCount('error')) {
44-
http2Session.on('error', this.onError.bind(this, 'ClientToProxy.Http2SessionError'));
45-
}
46-
}
47-
if (proxyToClientResponse instanceof Http2ServerResponse) {
48-
proxyToClientResponse.stream.on(
49-
'error',
50-
this.onError.bind(this, 'ProxyToClient.Http2StreamError'),
51-
);
52-
const http2Session = proxyToClientResponse.stream.session;
53-
if (!http2Session.listenerCount('error')) {
54-
http2Session.on('error', this.onError.bind(this, 'ProxyToClient.Http2SessionError'));
55-
}
56-
}
57-
proxyToClientResponse.on('error', this.onError.bind(this, 'ProxyToClient.ResponseError'));
30+
this.bindErrorListeners();
5831
}
5932

6033
public async onRequest(): Promise<void> {
@@ -83,6 +56,7 @@ export default class HttpRequestHandler extends BaseHttpHandler {
8356
rawHeaders?: string[],
8457
): Promise<void> {
8558
const context = this.context;
59+
8660
context.setState(ResourceState.ServerToProxyOnResponse);
8761

8862
if (response instanceof http.IncomingMessage) {
@@ -95,25 +69,36 @@ export default class HttpRequestHandler extends BaseHttpHandler {
9569
rawHeaders,
9670
);
9771
}
98-
const { serverToProxyResponse } = context;
99-
serverToProxyResponse.on('error', this.onError.bind(this, 'ServerToProxy.ResponseError'));
72+
// wait for MitmRequestContext to read this
73+
context.serverToProxyResponse.on(
74+
'error',
75+
this.onError.bind(this, 'ServerToProxy.ResponseError'),
76+
);
10077

10178
try {
10279
context.cacheHandler.onResponseHeaders();
10380
} catch (err) {
10481
return this.onError('ServerToProxy.ResponseHeadersHandlerError', err);
10582
}
10683

107-
if (redirectCodes.has(context.status)) {
108-
const redirectLocation = context.responseHeaders.location || context.responseHeaders.Location;
109-
if (redirectLocation) {
110-
const redirectUrl = new URL(redirectLocation as string, context.url);
111-
context.redirectedToUrl = redirectUrl.href;
112-
context.responseUrl = context.redirectedToUrl;
113-
}
114-
}
11584
await CookieHandler.readServerResponseCookies(context);
11685

86+
/////// WRITE CLIENT RESPONSE //////////////////
87+
88+
if (!context.proxyToClientResponse) {
89+
log.warn('Error.NoProxyToClientResponse', {
90+
sessionId: context.requestSession.sessionId,
91+
});
92+
context.setState(ResourceState.PrematurelyClosed);
93+
return;
94+
}
95+
96+
try {
97+
this.writeResponseHead();
98+
} catch (err) {
99+
return this.onError('ServerToProxyToClient.WriteResponseHeadError', err);
100+
}
101+
117102
try {
118103
await this.writeResponse();
119104
} catch (err) {
@@ -164,6 +149,22 @@ export default class HttpRequestHandler extends BaseHttpHandler {
164149
}
165150
}
166151

152+
private bindErrorListeners(): void {
153+
const { clientToProxyRequest, proxyToClientResponse } = this.context;
154+
clientToProxyRequest.on('error', this.onError.bind(this, 'ClientToProxy.RequestError'));
155+
proxyToClientResponse.on('error', this.onError.bind(this, 'ProxyToClient.ResponseError'));
156+
157+
if (clientToProxyRequest instanceof Http2ServerRequest) {
158+
const stream = clientToProxyRequest.stream;
159+
this.bindHttp2ErrorListeners('ClientToProxy', stream, stream.session);
160+
}
161+
162+
if (proxyToClientResponse instanceof Http2ServerResponse) {
163+
const stream = proxyToClientResponse.stream;
164+
this.bindHttp2ErrorListeners('ProxyToClient', stream, stream.session);
165+
}
166+
}
167+
167168
private async writeRequest(): Promise<void> {
168169
this.context.setState(ResourceState.WriteProxyToServerRequestBody);
169170
const { proxyToServerRequest, clientToProxyRequest } = this.context;
@@ -185,17 +186,9 @@ export default class HttpRequestHandler extends BaseHttpHandler {
185186
this.context.requestPostData = Buffer.concat(data);
186187
}
187188

188-
private async writeResponse(): Promise<void> {
189+
private writeResponseHead(): void {
189190
const context = this.context;
190-
if (!context.proxyToClientResponse) {
191-
log.warn('Error.NoProxyToClientResponse', {
192-
sessionId: context.requestSession.sessionId,
193-
});
194-
context.setState(ResourceState.PrematurelyClosed);
195-
return;
196-
}
197-
198-
const { serverToProxyResponse, proxyToClientResponse } = this.context;
191+
const { serverToProxyResponse, proxyToClientResponse } = context;
199192

200193
proxyToClientResponse.statusCode = context.status;
201194
// write individually so we properly write header-lists
@@ -207,32 +200,24 @@ export default class HttpRequestHandler extends BaseHttpHandler {
207200
context.responseTrailers = headers;
208201
});
209202

210-
try {
211-
proxyToClientResponse.writeHead(proxyToClientResponse.statusCode);
212-
} catch (err) {
213-
return this.onError('ServerToProxy.WriteResponseHeadError', err);
214-
}
203+
proxyToClientResponse.writeHead(proxyToClientResponse.statusCode);
204+
}
205+
206+
private async writeResponse(): Promise<void> {
207+
const context = this.context;
208+
const { serverToProxyResponse, proxyToClientResponse } = context;
215209

216210
context.setState(ResourceState.WriteProxyToClientResponseBody);
217211

218212
await context.requestSession.willSendResponse(context);
219213

220214
for await (const chunk of serverToProxyResponse) {
221215
const data = context.cacheHandler.onResponseData(chunk as Buffer);
222-
if (data && !this.isClientConnectionDestroyed()) {
223-
proxyToClientResponse.write(data, error => {
224-
if (error && !this.isClientConnectionDestroyed())
225-
this.onError('ServerToProxy.WriteResponseError', error);
226-
});
227-
}
216+
this.safeWriteToClient(data);
228217
}
229218

230219
if (context.cacheHandler.shouldServeCachedData) {
231-
if (!this.isClientConnectionDestroyed())
232-
proxyToClientResponse.write(context.cacheHandler.cacheData, error => {
233-
if (error && !this.isClientConnectionDestroyed())
234-
this.onError('ServerToProxy.WriteCachedResponseError', error);
235-
});
220+
this.safeWriteToClient(context.cacheHandler.cacheData);
236221
}
237222

238223
if (serverToProxyResponse instanceof http.IncomingMessage) {
@@ -250,6 +235,15 @@ export default class HttpRequestHandler extends BaseHttpHandler {
250235
context.requestSession.emit('response', MitmRequestContext.toEmittedResource(context));
251236
}
252237

238+
private safeWriteToClient(data: Buffer): void {
239+
if (!data || this.isClientConnectionDestroyed()) return;
240+
241+
this.context.proxyToClientResponse.write(data, error => {
242+
if (error && !this.isClientConnectionDestroyed())
243+
this.onError('ServerToProxy.WriteResponseError', error);
244+
});
245+
}
246+
253247
private isClientConnectionDestroyed(): boolean {
254248
const proxyToClientResponse = this.context.proxyToClientResponse;
255249
return (
@@ -269,5 +263,3 @@ export default class HttpRequestHandler extends BaseHttpHandler {
269263
await handler.onRequest();
270264
}
271265
}
272-
273-
export { redirectCodes };

‎mitm/lib/MitmRequestAgent.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -334,10 +334,17 @@ export default class MitmRequestAgent {
334334

335335
const clientToProxyRequest = parentContext.clientToProxyRequest as http2.Http2ServerRequest;
336336
pushContext.setState(ResourceState.ProxyToClientPush);
337-
clientToProxyRequest.stream.pushStream(
338-
pushContext.requestHeaders,
339-
this.handleHttp2ProxyToClientPush.bind(this, pushContext, onResponseHeaders),
340-
);
337+
try {
338+
clientToProxyRequest.stream.pushStream(
339+
pushContext.requestHeaders,
340+
this.handleHttp2ProxyToClientPush.bind(this, pushContext, onResponseHeaders),
341+
);
342+
} catch (error) {
343+
log.warn('Http2.ClientToProxy.CreatePushStreamError', {
344+
sessionId,
345+
error,
346+
});
347+
}
341348
}
342349

343350
private async handleHttp2ProxyToClientPush(

‎mitm/lib/MitmRequestContext.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,12 @@ export default class MitmRequestContext {
228228
ctx.serverToProxyResponse = response;
229229
ctx.responseOriginalHeaders = parseRawHeaders(response.rawHeaders);
230230
ctx.responseHeaders = HeadersHandler.cleanResponseHeaders(ctx, ctx.responseOriginalHeaders);
231+
232+
const redirectUrl = HeadersHandler.checkForRedirectResponseLocation(ctx);
233+
if (redirectUrl) {
234+
ctx.redirectedToUrl = redirectUrl.href;
235+
ctx.responseUrl = ctx.redirectedToUrl;
236+
}
231237
}
232238

233239
public static readHttp2Response(
@@ -243,5 +249,11 @@ export default class MitmRequestContext {
243249
ctx.serverToProxyResponse = response;
244250
ctx.responseOriginalHeaders = headers;
245251
ctx.responseHeaders = HeadersHandler.cleanResponseHeaders(ctx, headers);
252+
253+
const redirectUrl = HeadersHandler.checkForRedirectResponseLocation(ctx);
254+
if (redirectUrl) {
255+
ctx.redirectedToUrl = redirectUrl.href;
256+
ctx.responseUrl = ctx.redirectedToUrl;
257+
}
246258
}
247259
}

‎mitm/lib/Utils.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import IResourceHeaders from '@secret-agent/core-interfaces/IResourceHeaders';
22

33
export function parseRawHeaders(rawHeaders: string[]): IResourceHeaders {
4-
if (!rawHeaders.length) return;
54
const headers = {};
65
for (let i = 0; i < rawHeaders.length; i += 2) {
76
const key = rawHeaders[i];

0 commit comments

Comments
 (0)