Skip to content

Commit

Permalink
fix: HTTP response with invalid headers doesn't throw error #28865
Browse files Browse the repository at this point in the history
When receiving the described HTTP response Cypress resets the headers.
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 26, 2024
1 parent f14a11a commit 114fe47
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 1 deletion.
8 changes: 8 additions & 0 deletions 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_
Expand Down
22 changes: 21 additions & 1 deletion packages/proxy/lib/http/response-middleware.ts
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
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 114fe47

Please sign in to comment.