From c178ca15e7360ae4da6ce35e4d04455ab0f411ea Mon Sep 17 00:00:00 2001
From: Matteo Collina <hello@matteocollina.com>
Date: Wed, 27 Nov 2024 15:17:24 +0100
Subject: [PATCH] Correctly parse JSON contentType in responseError interceptor
 (#3892)

Signed-off-by: Matteo Collina <hello@matteocollina.com>
---
 lib/interceptor/response-error.js   |  8 ++-
 test/interceptors/response-error.js | 86 +++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/lib/interceptor/response-error.js b/lib/interceptor/response-error.js
index 07f9ec788b7..1df5c06165b 100644
--- a/lib/interceptor/response-error.js
+++ b/lib/interceptor/response-error.js
@@ -27,6 +27,10 @@ class ResponseErrorHandler extends DecoratorHandler {
     return this.#handler.onConnect(abort)
   }
 
+  #checkContentType (contentType) {
+    return this.#contentType.indexOf(contentType) === 0
+  }
+
   onHeaders (statusCode, rawHeaders, resume, statusMessage, headers = parseHeaders(rawHeaders)) {
     this.#statusCode = statusCode
     this.#headers = headers
@@ -36,7 +40,7 @@ class ResponseErrorHandler extends DecoratorHandler {
       return this.#handler.onHeaders(statusCode, rawHeaders, resume, statusMessage, headers)
     }
 
-    if (this.#contentType === 'application/json' || this.#contentType === 'text/plain') {
+    if (this.#checkContentType('application/json') || this.#checkContentType('text/plain')) {
       this.#decoder = new TextDecoder('utf-8')
     }
   }
@@ -53,7 +57,7 @@ class ResponseErrorHandler extends DecoratorHandler {
     if (this.#statusCode >= 400) {
       this.#body += this.#decoder?.decode(undefined, { stream: false }) ?? ''
 
-      if (this.#contentType === 'application/json') {
+      if (this.#checkContentType('application/json')) {
         try {
           this.#body = JSON.parse(this.#body)
         } catch {
diff --git a/test/interceptors/response-error.js b/test/interceptors/response-error.js
index fb0a1c91391..b0c0fb63498 100644
--- a/test/interceptors/response-error.js
+++ b/test/interceptors/response-error.js
@@ -82,3 +82,89 @@ test('should not throw error for ok response', async () => {
   assert.equal(response.statusCode, 200)
   assert.equal(await response.body.text(), 'hello')
 })
+
+test('should throw error for error response, parsing JSON', async () => {
+  const server = createServer()
+
+  server.on('request', (req, res) => {
+    res.writeHead(400, { 'content-type': 'application/json; charset=utf-8' })
+    res.end(JSON.stringify({ message: 'Bad Request' }))
+  })
+
+  server.listen(0)
+
+  await once(server, 'listening')
+
+  const client = new Client(
+    `http://localhost:${server.address().port}`
+  ).compose(responseError())
+
+  after(async () => {
+    await client.close()
+    server.close()
+
+    await once(server, 'close')
+  })
+
+  let error
+  try {
+    await client.request({
+      method: 'GET',
+      path: '/',
+      headers: {
+        'content-type': 'text/plain'
+      }
+    })
+  } catch (err) {
+    error = err
+  }
+
+  assert.equal(error.statusCode, 400)
+  assert.equal(error.message, 'Response Error')
+  assert.deepStrictEqual(error.body, {
+    message: 'Bad Request'
+  })
+})
+
+test('should throw error for error response, parsing JSON without charset', async () => {
+  const server = createServer()
+
+  server.on('request', (req, res) => {
+    res.writeHead(400, { 'content-type': 'application/json' })
+    res.end(JSON.stringify({ message: 'Bad Request' }))
+  })
+
+  server.listen(0)
+
+  await once(server, 'listening')
+
+  const client = new Client(
+    `http://localhost:${server.address().port}`
+  ).compose(responseError())
+
+  after(async () => {
+    await client.close()
+    server.close()
+
+    await once(server, 'close')
+  })
+
+  let error
+  try {
+    await client.request({
+      method: 'GET',
+      path: '/',
+      headers: {
+        'content-type': 'text/plain'
+      }
+    })
+  } catch (err) {
+    error = err
+  }
+
+  assert.equal(error.statusCode, 400)
+  assert.equal(error.message, 'Response Error')
+  assert.deepStrictEqual(error.body, {
+    message: 'Bad Request'
+  })
+})