Skip to content

Commit

Permalink
fix: #29171 set correct host header with fetch (#29452)
Browse files Browse the repository at this point in the history
* Patch node-fetch to set defaultPort based on protocol

* unit test that proper host headers are sent with fetch

* changelog

* make patch more robust

---------

Co-authored-by: Jennifer Shehane <jennifer@cypress.io>
  • Loading branch information
bjowes and jennifer-shehane authored May 7, 2024
1 parent 84b6bf2 commit 66dac23
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 2 deletions.
1 change: 1 addition & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ _Released 5/7/2024 (PENDING)_

- Fixed a bug where promises rejected with `undefined` were failing inside `cy.origin()`. Addresses [#23937](https://github.com/cypress-io/cypress/issues/23937).
- We now pass the same default Chromium flags to Electron as we do to Chrome. As a result of this change, the application under test's `navigator.webdriver` property will now correctly be `true` when testing in Electron. Fixes [#27939](https://github.com/cypress-io/cypress/issues/27939).
- Fixed network issues in requests using fetch for users where Cypress is run behind a proxy that performs HTTPS decryption (common among corporate proxies). Fixes [#29171](https://github.com/cypress-io/cypress/issues/29171)
- Fixed an issue where extra windows weren't being closed between specs in Firefox causing potential issues in subsequent specs. Fixes [#29473](https://github.com/cypress-io/cypress/issues/29473).

**Misc:**
Expand Down
9 changes: 7 additions & 2 deletions packages/network/test/support/servers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,18 @@ export interface AsyncServer {
listenAsync: (port) => Promise<void>
}

function createExpressApp () {
function createExpressApp (requestCallback: (req) => void) {
const app: express.Application = express()

app.get('/get', (req, res) => {
if (requestCallback) requestCallback(req)

res.send('It worked!')
})

app.get('/empty-response', (req, res) => {
if (requestCallback) requestCallback(req)

// ERR_EMPTY_RESPONSE in Chrome
setTimeout(() => res.connection.destroy(), 100)
})
Expand All @@ -45,10 +49,11 @@ export class Servers {
wsServer: any
wssServer: any
caCertificatePath: string
lastRequestHeaders: any

start (httpPort: number, httpsPort: number) {
return Promise.join(
createExpressApp(),
createExpressApp((req) => this.lastRequestHeaders = req.headers),
getCAInformation(),
)
.spread((app: Express.Application, [serverCertificateKeys, caCertificatePath]: [serverCertificateKeys: string[], caCertificatePath: string]) => {
Expand Down
27 changes: 27 additions & 0 deletions packages/network/test/unit/agent_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,15 @@ describe('lib/agent', function () {
})
})

it('HTTP pages are requested with correct host header when loaded via fetch', function () {
return this.fetch(`http://localhost:${HTTP_PORT}/get`)
.then(() => {
expect(this.servers.lastRequestHeaders).to.include({
host: `localhost:${HTTP_PORT}`,
})
})
})

it('HTTPS pages can be loaded', function () {
return this.request({
url: `https://localhost:${HTTPS_PORT}/get`,
Expand Down Expand Up @@ -255,6 +264,15 @@ describe('lib/agent', function () {
})
})

it('HTTPS pages are requested with correct host header when loaded via fetch', function () {
return this.fetch(`https://localhost:${HTTPS_PORT}/get`)
.then(() => {
expect(this.servers.lastRequestHeaders).to.include({
host: 'localhost',
})
})
})

it('HTTPS pages can be loaded via fetch with no explicit port', function () {
return this.fetch(`https://localhost/get`)
.then((response) => response.text())
Expand All @@ -269,6 +287,15 @@ describe('lib/agent', function () {
})
})

it('HTTPS pages requested with correct host header when loaded via fetch with no explicit port', function () {
return this.fetch(`https://localhost/get`)
.then(() => {
expect(this.servers.lastRequestHeaders).to.include({
host: 'localhost',
})
})
})

it('HTTP errors are catchable', function () {
return this.request({
url: `http://localhost:${HTTP_PORT}/empty-response`,
Expand Down
15 changes: 15 additions & 0 deletions patches/node-fetch+2.7.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
diff --git a/node_modules/node-fetch/lib/index.js b/node_modules/node-fetch/lib/index.js
index 567ff5d..c7e2bd9 100644
--- a/node_modules/node-fetch/lib/index.js
+++ b/node_modules/node-fetch/lib/index.js
@@ -1449,7 +1449,9 @@ function fetch(url, opts) {
const request = new Request(url, opts);
const options = getNodeRequestOptions(request);

- const send = (options.protocol === 'https:' ? https : http).request;
+ const isHttps = options.protocol === 'https:';
+ options.defaultPort = (isHttps ? 443 : 80);
+ const send = (isHttps ? https : http).request;
const signal = request.signal;

let response = null;

5 comments on commit 66dac23

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 66dac23 May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.9.0/linux-x64/develop-66dac2341cf5ea9a65a5564a261902eae0e0e979/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 66dac23 May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux arm64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.9.0/linux-arm64/develop-66dac2341cf5ea9a65a5564a261902eae0e0e979/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 66dac23 May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin arm64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.9.0/darwin-arm64/develop-66dac2341cf5ea9a65a5564a261902eae0e0e979/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 66dac23 May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.9.0/darwin-x64/develop-66dac2341cf5ea9a65a5564a261902eae0e0e979/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 66dac23 May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.9.0/win32-x64/develop-66dac2341cf5ea9a65a5564a261902eae0e0e979/cypress.tgz

Please sign in to comment.