Skip to content

Commit

Permalink
fix: receiving HTTP response with invalid headers doesn't throw error c…
Browse files Browse the repository at this point in the history
…ypress-io#28865

When receiving the described HTTP response Cypress might reset the headers of the response. This would cause
the validateHeaderName method from node to be called which would cause an error, since the headers where invalid.
Now Crypress verifies all the headers before reseting them, discards invalid ones and sends a warning in the console when debug module is on.
  • Loading branch information
BernardoSousa03 committed Apr 23, 2024
1 parent c5fa260 commit d2bdd96
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 1 deletion.
8 changes: 8 additions & 0 deletions packages/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
## 13.8.1

_Released 4/23/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).

# Unreleased Cypress App Changes

## Breaking
Expand Down
22 changes: 21 additions & 1 deletion packages/proxy/lib/http/response-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ 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 } from 'http'

export interface ResponseMiddlewareProps {
/**
Expand Down Expand Up @@ -306,7 +307,26 @@ 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)

return true
} catch (err) {
this.debug('Warning: Found header with the invalid name \'%s\', taking it off the response', key)

return false
}
}),
)

this.res.set(filteredHeaders)

this.debug('the new response headers are %o', this.res.getHeaderNames())

span?.setAttributes({
experimentalCspAllowList: this.config.experimentalCspAllowList,
Expand Down
36 changes: 36 additions & 0 deletions packages/proxy/test/unit/http/response-middleware.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -443,6 +478,7 @@ describe('http/response-middleware', function () {
setHeader: sinon.stub(),
on: (event, listener) => {},
off: (event, listener) => {},
getHeaderNames: () => Object.keys(ctx.incomingRes.headers),
},
}
}
Expand Down

0 comments on commit d2bdd96

Please sign in to comment.