fix: HTTP response with invalid headers doesn't throw error #28865

cli/CHANGELOG.md

@@ -1,4 +1,12 @@
 <!-- See the ../guides/writing-the-cypress-changelog.md for details on writing the changelog. -->
+## 13.8.2
+
+_Released 4/27/2024 (PENDING)_
+
+**Bugfixes:**
+
+- Fixed an issue where receiving HTTP responses with invalid headers raised an error. Now cypress removes the invalid headers and gives a warning in the console with debug mode on. Fixes [#28865](https://github.com/cypress-io/cypress/issues/28865).
+
 ## 13.8.1
 
 _Released 4/23/2024_

packages/errors/src/errors.ts

@@ -1726,6 +1726,26 @@ export const AllCypressErrors = {
       If you're experiencing problems, downgrade dependencies and restart Cypress.
     `
   },
+
+  PROXY_ENCOUNTERED_INVALID_HEADER_NAME: (header: any, method: string, url: string, error: Error) => {
+    return errTemplate`
+    Warning: While proxying a ${fmt.highlight(method)} request to ${fmt.url(url)}, an HTTP header did not pass validation, and was removed. This header will not be present in the response received by the application under test.
+
+    Invalid header name: ${fmt.code(JSON.stringify(header, undefined, 2))}
+
+    ${fmt.highlightSecondary(error)}
+    `
+  },
+
+  PROXY_ENCOUNTERED_INVALID_HEADER_VALUE: (header: any, method: string, url: string, error: Error) => {
+    return errTemplate`
+    Warning: While proxying a ${fmt.highlight(method)} request to ${fmt.url(url)}, an HTTP header value did not pass validation, and was removed. This header will not be present in the response received by the application under test.
+
+    Invalid header value: ${fmt.code(JSON.stringify(header, undefined, 2))}
+
+    ${fmt.highlightSecondary(error)}
+    `
+  },
 } as const
 
 // eslint-disable-next-line @typescript-eslint/no-unused-vars

packages/errors/test/unit/visualSnapshotErrors_spec.ts

@@ -1307,5 +1307,21 @@ describe('visual error templates', () => {
         default: [],
       }
     },
+
+    PROXY_ENCOUNTERED_INVALID_HEADER_NAME: () => {
+      const err = makeErr()
+
+      return {
+        default: [{ invalidHeaderName: 'Value' }, 'GET', 'http://localhost:8080', err],
+      }
+    },
+
+    PROXY_ENCOUNTERED_INVALID_HEADER_VALUE: () => {
+      const err = makeErr()
+
+      return {
+        default: [{ invalidHeaderValue: 'Value' }, 'GET', 'http://localhost:8080', err],
+      }
+    },
   })
 })

packages/proxy/lib/http/response-middleware.ts

@@ -22,6 +22,8 @@ import type { IncomingMessage, IncomingHttpHeaders } from 'http'
 
 import { cspHeaderNames, generateCspDirectives, nonceDirectives, parseCspHeaders, problematicCspDirectives, unsupportedCSPDirectives } from './util/csp-header'
 import { injectIntoServiceWorker } from './util/service-worker-injector'
+import { validateHeaderName, validateHeaderValue } from 'http'
+import error from '@packages/errors'
 
 export interface ResponseMiddlewareProps {
   /**
@@ -306,7 +308,35 @@ const OmitProblematicHeaders: ResponseMiddleware = function () {
     'connection',
   ])
 
-  this.res.set(headers)
+  this.debug('The headers are %o', headers)
+
+  // Filter for invalid headers
+  const filteredHeaders = Object.fromEntries(
+    Object.entries(headers).filter(([key, value]) => {
+      try {
+        validateHeaderName(key)
+        if (Array.isArray(value)) {
+          value.forEach((v) => validateHeaderValue(key, v))
+        } else {
+          validateHeaderValue(key, value)
+        }
+
+        return true
+      } catch (err) {
+        if (err.code === 'ERR_INVALID_HTTP_TOKEN') {
+          error.warning('PROXY_ENCOUNTERED_INVALID_HEADER_NAME', { [key]: value }, this.req.method, this.req.originalUrl, err)
+        } else if (err.code === 'ERR_INVALID_HEADER_VALUE') {
+          error.warning('PROXY_ENCOUNTERED_INVALID_HEADER_VALUE', { [key]: value }, this.req.method, this.req.originalUrl, err)
+        }
+
+        return false
+      }
+    }),
+  )
+
+  this.res.set(filteredHeaders)
+
+  this.debug('the new response headers are %o', this.res.getHeaderNames())
 
   span?.setAttributes({
     experimentalCspAllowList: this.config.experimentalCspAllowList,

packages/proxy/test/unit/http/response-middleware.spec.ts

@@ -274,6 +274,41 @@ describe('http/response-middleware', function () {
       })
     })
 
+    let badHeaders = {
+      'bad-header ': 'value', //(contains trailling space)
+      'Content Type': 'value', //(contains a space)
+      'User-Agent:': 'value', //(contains a colon)
+      'Accept-Encoding;': 'value', //(contains a semicolon)
+      '@Origin': 'value', //(contains an at symbol)
+      'Authorization?': 'value', //(contains a question mark)
+      'X-My-Header/Version': 'value', //(contains a slash)
+      'Referer[1]': 'value', //(contains square brackets)
+      'If-None-Match{1}': 'value', //(contains curly braces)
+      'X-Forwarded-For<1>': 'value', //(contains angle brackets)
+    }
+
+    it('removes invalid headers and leaves valid headers', function () {
+      prepareContext({ ...badHeaders, 'good-header': 'value' })
+
+      return testMiddleware([OmitProblematicHeaders], ctx)
+      .then(() => {
+        expect(ctx.res.set).to.have.been.calledOnce
+        expect(ctx.res.set).to.be.calledWith(sinon.match(function (actual) {
+          // Check if the invalid headers are removed
+          for (let header in actual) {
+            if (header in badHeaders) {
+              throw new Error(`Unexpected header "${header}"`)
+            }
+          }
+
+          // Check if the valid header is present
+          expect(actual['good-header']).to.equal('value')
+
+          return true
+        }))
+      })
+    })
+
     const validCspHeaderNames = [
       'content-security-policy',
       'Content-Security-Policy',
@@ -443,6 +478,7 @@ describe('http/response-middleware', function () {
           setHeader: sinon.stub(),
           on: (event, listener) => {},
           off: (event, listener) => {},
+          getHeaderNames: () => Object.keys(ctx.incomingRes.headers),
         },
       }
     }