Skip to content

Commit 22cc7ba

Browse files
committed
Update httpolyglot to fix unknown-over-TLS bug
This fixes a bug that this update causes. Previously, we depended on subtle ordering of events to detect whether the last hop of a request was encrypted or not (i.e. is a tunnelled request HTTPS or HTTP, given a series of TLS or not hops en route). This was buggy, and breaks with the latest httpolyglot, where this order can change for requests without ALPN. Now, we properly set defaults & reset encrypted status at each tunnel hop instead. Also fixed an unrelated TS issue that breaks things with the latest Node types in reuest-utils.
1 parent faeeb7b commit 22cc7ba

File tree

3 files changed

+9
-6
lines changed

3 files changed

+9
-6
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@
161161
"dependencies": {
162162
"@graphql-tools/schema": "^8.5.0",
163163
"@graphql-tools/utils": "^8.8.0",
164-
"@httptoolkit/httpolyglot": "^3.0.0",
164+
"@httptoolkit/httpolyglot": "^3.0.1",
165165
"@httptoolkit/subscriptions-transport-ws": "^0.11.2",
166166
"@httptoolkit/util": "^0.1.7",
167167
"@httptoolkit/websocket-stream": "^6.0.1",

src/server/http-combo-server.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,9 @@ export async function createComboServer(options: ComboServerOptions): Promise<De
296296
server.on('connection', (socket: net.Socket | http2.ServerHttp2Stream) => {
297297
socket[SocketTimingInfo] ||= buildSocketTimingInfo();
298298

299-
// All sockets are initially marked as using unencrypted upstream connections.
300-
// If TLS is used, this is upgraded to 'true' by secureConnection below.
301-
socket[LastHopEncrypted] = false;
299+
// All sockets are initially marked as using unencrypted upstream connections,
300+
// if not set elsewhere (TLS) or downgraded by intended hop (CONNECT):
301+
socket[LastHopEncrypted] ||= false;
302302

303303
// For actual sockets, set NODELAY to avoid any buffering whilst streaming. This is
304304
// off by default in Node HTTP, but likely to be enabled soon & is default in curl.
@@ -371,6 +371,7 @@ export async function createComboServer(options: ComboServerOptions): Promise<De
371371
socket.write('HTTP/' + req.httpVersion + ' 200 OK\r\n\r\n', 'utf-8', () => {
372372
socket[SocketTimingInfo]!.tunnelSetupTimestamp = now();
373373
socket[LastTunnelAddress] = connectUrl;
374+
socket[LastHopEncrypted] = false; // Will be updated if TLS is added later
374375
if (req.headers['proxy-authorization']) {
375376
socket[SocketMetadata] = getSocketMetadataFromProxyAuth(socket, req.headers['proxy-authorization']);
376377
}
@@ -394,6 +395,8 @@ export async function createComboServer(options: ComboServerOptions): Promise<De
394395
res.writeHead(200, {});
395396

396397
inheritSocketDetails(res.socket, res.stream);
398+
399+
res.stream[LastHopEncrypted] = false; // Will be updated if TLS is added later
397400
res.stream[LastTunnelAddress] = connectUrl;
398401
if (req.headers['proxy-authorization']) {
399402
res.stream[SocketMetadata] = getSocketMetadataFromProxyAuth(res.stream, req.headers['proxy-authorization']);

src/util/request-utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ export const writeHead = (
9898

9999
if (statusMessage === undefined) {
100100
// Cast is required as Node H2 types don't know about raw headers:
101-
response.writeHead(status, flatHeaders as http.OutgoingHttpHeaders);
101+
(response as http.ServerResponse).writeHead(status, flatHeaders);
102102
} else {
103-
response.writeHead(status, statusMessage, flatHeaders as http.OutgoingHttpHeaders);
103+
(response as http.ServerResponse).writeHead(status, statusMessage, flatHeaders);
104104
}
105105
};
106106

0 commit comments

Comments
 (0)