fix(ClientRequest): correctly free HTTP parsers (#757)

Author: kettanaito

.github/workflows/ci.yml

@@ -11,20 +11,21 @@ jobs:
     runs-on: macos-latest
     strategy:
       matrix:
-        node: [18, 20]
+        node: [20, 22]
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
-
-      - name: Set up Node.js
-        uses: actions/setup-node@v3
-        with:
-          node-version: ${{ matrix.node }}
+        uses: actions/checkout@v4
 
       - name: Set up pnpm
         uses: pnpm/action-setup@v4
         with:
-          version: 9.14.0
+          version: 9.15.0
+
+      - name: Set up Node.js
+        uses: actions/setup-node@v4
+        with:
+          node-version: ${{ matrix.node }}
+          cache: 'pnpm'
 
       - name: Install dependencies
         run: pnpm install
@@ -39,7 +40,7 @@ jobs:
         run: pnpm test:node
 
       - name: Install Playwright browsers
-        run: npx playwright install
+        run: npx playwright install chromium --with-deps --only-shell
 
       - name: Browser tests
         run: pnpm test:browser

.nvmrc

@@ -1 +1 @@
-v20
+v22

_http_common.d.ts

@@ -1,3 +1,6 @@
+import type { Socket } from 'node:net'
+import type { IncomingMessage, OutgoingMessage } from 'node:http'
+
 declare var HTTPParser: {
   new (): HTTPParser<number>
   REQUEST: 0
@@ -18,26 +21,35 @@ declare var HTTPParser: {
 export interface HTTPParser<ParserType extends number> {
   new (): HTTPParser<ParserType>
 
-  [HTTPParser.kOnMessageBegin]: () => void
-  [HTTPParser.kOnHeaders]: HeadersCallback
+  [HTTPParser.kOnMessageBegin]: (() => void) | null
+  [HTTPParser.kOnHeaders]: HeadersCallback | null
   [HTTPParser.kOnHeadersComplete]: ParserType extends 0
-    ? RequestHeadersCompleteCallback
-    : ResponseHeadersCompleteCallback
-  [HTTPParser.kOnBody]: (chunk: Buffer) => void
-  [HTTPParser.kOnMessageComplete]: () => void
-  [HTTPParser.kOnExecute]: () => void
-  [HTTPParser.kOnTimeout]: () => void
+    ? RequestHeadersCompleteCallback | null
+    : ResponseHeadersCompleteCallback | null
+  [HTTPParser.kOnBody]: ((chunk: Buffer) => void) | null
+  [HTTPParser.kOnMessageComplete]: (() => void) | null
+  [HTTPParser.kOnExecute]: (() => void) | null
+  [HTTPParser.kOnTimeout]: (() => void) | null
 
   initialize(type: ParserType, asyncResource: object): void
   execute(buffer: Buffer): void
+  _url: string
+  _headers?: Array<unknown>
+  _consumed?: boolean
+  maxHeaderPairs: number
+  socket?: Socket | null
+  incoming?: IncomingMessage | null
+  outgoing?: OutgoingMessage | null
+  onIncoming?: (() => void) | null
+  joinDuplicateHeaders?: unknown
   finish(): void
+  unconsume(): void
+  remove(): void
+  close(): void
   free(): void
 }
 
-export type HeadersCallback = (
-  rawHeaders: Array<string>,
-  url: string
-) => void
+export type HeadersCallback = (rawHeaders: Array<string>, url: string) => void
 
 export type RequestHeadersCompleteCallback = (
   versionMajor: number,

src/interceptors/ClientRequest/MockHttpSocket.ts

@@ -19,6 +19,7 @@ import { getRawFetchHeaders } from './utils/recordRawHeaders'
 import { FetchResponse } from '../../utils/fetchUtils'
 import { setRawRequest } from '../../getRawRequest'
 import { setRawRequestBodyStream } from '../../utils/node'
+import { freeParser } from './utils/parserUtils'
 
 type HttpConnectionOptions = any
 
@@ -136,7 +137,7 @@ export class MockHttpSocket extends MockSocket {
 
     // Once the socket is finished, nothing can write to it
     // anymore. It has also flushed any buffered chunks.
-    this.once('finish', () => this.requestParser.free())
+    this.once('finish', () => freeParser(this.requestParser, this))
 
     if (this.baseUrl.protocol === 'https:') {
       Reflect.set(this, 'encrypted', true)
@@ -146,7 +147,11 @@ export class MockHttpSocket extends MockSocket {
       Reflect.set(this, 'getProtocol', () => 'TLSv1.3')
       Reflect.set(this, 'getSession', () => undefined)
       Reflect.set(this, 'isSessionReused', () => false)
-      Reflect.set(this, 'getCipher', () => ({ name: 'AES256-SHA', standardName: 'TLS_RSA_WITH_AES_256_CBC_SHA', version: 'TLSv1.3' }))
+      Reflect.set(this, 'getCipher', () => ({
+        name: 'AES256-SHA',
+        standardName: 'TLS_RSA_WITH_AES_256_CBC_SHA',
+        version: 'TLSv1.3',
+      }))
     }
   }
 
@@ -165,7 +170,7 @@ export class MockHttpSocket extends MockSocket {
     // Destroy the response parser when the socket gets destroyed.
     // Normally, we should listen to the "close" event but it
     // can be suppressed by using the "emitClose: false" option.
-    this.responseParser.free()
+    freeParser(this.responseParser, this)
 
     if (error) {
       this.emit('error', error)
@@ -259,7 +264,7 @@ export class MockHttpSocket extends MockSocket {
         'getProtocol',
         'getSession',
         'isSessionReused',
-        'getCipher'
+        'getCipher',
       ]
 
       tlsProperties.forEach((propertyName) => {

src/interceptors/ClientRequest/utils/parserUtils.ts

@@ -0,0 +1,48 @@
+import type { Socket } from 'node:net'
+import { HTTPParser } from '_http_common'
+
+/**
+ * @see https://github.com/nodejs/node/blob/f3adc11e37b8bfaaa026ea85c1cf22e3a0e29ae9/lib/_http_common.js#L180
+ */
+export function freeParser(parser: HTTPParser<any>, socket?: Socket): void {
+  if (parser._consumed) {
+    parser.unconsume()
+  }
+
+  parser._headers = []
+  parser._url = ''
+  parser.socket = null
+  parser.incoming = null
+  parser.outgoing = null
+  parser.maxHeaderPairs = 2000
+  parser._consumed = false
+  parser.onIncoming = null
+
+  parser[HTTPParser.kOnHeaders] = null
+  parser[HTTPParser.kOnHeadersComplete] = null
+  parser[HTTPParser.kOnMessageBegin] = null
+  parser[HTTPParser.kOnMessageComplete] = null
+  parser[HTTPParser.kOnBody] = null
+  parser[HTTPParser.kOnExecute] = null
+  parser[HTTPParser.kOnTimeout] = null
+
+  parser.remove()
+  parser.free()
+
+  if (socket) {
+    /**
+     * @note Unassigning the socket's parser will fail this assertion
+     * if there's still some data being processed on the socket:
+     * @see https://github.com/nodejs/node/blob/4e1f39b678b37017ac9baa0971e3aeecd3b67b51/lib/_http_client.js#L613
+     */
+    if (socket.destroyed) {
+      // @ts-expect-error Node.js internals.
+      socket.parser = null
+    } else {
+      socket.once('end', () => {
+        // @ts-expect-error Node.js internals.
+        socket.parser = null
+      })
+    }
+  }
+}

src/interceptors/XMLHttpRequest/utils/getBodyByteLength.test.ts

@@ -88,7 +88,7 @@ it('calculates body length from the FormData request body', async () => {
         body: formData,
       })
     )
-  ).resolves.toBe(127)
+  ).resolves.toBe(129)
 })
 
 it('calculates body length from the ReadableStream request body', async () => {
@@ -149,7 +149,7 @@ it('calculates body length from the FormData response body', async () => {
   const formData = new FormData()
   formData.append('hello', 'world')
 
-  await expect(getBodyByteLength(new Response(formData))).resolves.toBe(127)
+  await expect(getBodyByteLength(new Response(formData))).resolves.toBe(129)
 })
 
 it('calculates body length from the ReadableStream response body', async () => {