Skip to content

Commit

Permalink
fix: prevent redirect loop when chrome https upgrade is detected (#28650
Browse files Browse the repository at this point in the history
)
  • Loading branch information
chrisbreiding authored Jan 9, 2024
1 parent d1c4a8f commit ec89901
Show file tree
Hide file tree
Showing 5 changed files with 276 additions and 0 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 1/16/2024 (PENDING)_
- Fixed a regression in [`13.6.2`](https://docs.cypress.io/guides/references/changelog/13.6.2) where the `body` element was not highlighted correctly in Test Replay. Fixed in [#28627](https://github.com/cypress-io/cypress/pull/28627).
- Fixed an issue where some cross-origin logs, like assertions or cy.clock(), were getting too many dom snapshots. Fixes [#28609](https://github.com/cypress-io/cypress/issues/28609).
- Fixed asset capture for Test Replay for requests that are routed through service workers. This addresses an issue where styles were not being applied properly in Test Replay and `cy.intercept` was not working properly for requests in this scenario. Fixes [#28516](https://github.com/cypress-io/cypress/issues/28516).
- Fixed an issue where visiting an `http://` site would result in an infinite reload/redirect loop in Chrome 114+. Fixes [#25891](https://github.com/cypress-io/cypress/issues/25891).

**Performance:**

Expand Down
4 changes: 4 additions & 0 deletions packages/network/lib/cors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ export function getDomainNameFromParsedHost (parsedHost: ParsedHost) {
return _.compact([parsedHost.domain, parsedHost.tld]).join('.')
}

export function domainPropsToHostname ({ domain, subdomain, tld }: Record<string, any>) {
return _.compact([subdomain, domain, tld]).join('.')
}

/**
* same-origin: Whether or not a a urls scheme, port, and host match. @see https://developer.mozilla.org/en-US/docs/Web/Security/Same-origin_policy
* same-super-domain-origin: Whether or not a url's scheme, domain, top-level domain, and port match
Expand Down
6 changes: 6 additions & 0 deletions packages/server/lib/remote_states.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ export class RemoteStates {
return _.cloneDeep(state)
}

hasPrimary () {
const remoteStates = Array.from(this.remoteStates.entries())

return !!(remoteStates.length && remoteStates[0] && remoteStates[0][1])
}

getPrimary () {
const state = Array.from(this.remoteStates.entries())[0][1]

Expand Down
57 changes: 57 additions & 0 deletions packages/server/lib/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Debug from 'debug'
import { ErrorRequestHandler, Request, Router } from 'express'
import send from 'send'
import { getPathToDist } from '@packages/resolve-dist'
import { cors } from '@packages/network'
import type { NetworkProxy } from '@packages/proxy'
import type { Cfg } from './project-base'
import xhrs from './controllers/xhrs'
Expand Down Expand Up @@ -47,6 +48,62 @@ export const createCommonRoutes = ({
const router = Router()
const { clientRoute, namespace } = config

// When a test visits an http:// site and we load our main app page,
// (e.g. test has cy.visit('http://example.com'), we load http://example.com/__/)
// Chrome will make a request to the the https:// version (i.e. https://example.com/__/)
// to check if it's valid. If it is valid, it will load the https:// version
// instead. This leads to an infinite loop of Cypress trying to load
// the http:// version because that's what the test wants and Chrome
// loading the https:// version. Then since it doesn't match what the test
// is visiting, Cypress attempts to the load the http:// version and the loop
// continues.
// See https://blog.chromium.org/2023/08/towards-https-by-default.html for
// more info about Chrome's automatic https upgrades.
//
// The fix for Cypress is to signal to Chrome that the https:// version is
// not valid by replying with a 301 redirect when we detect that it's
// an https upgrade, which is when an https:// request comes through
// one of your own proxied routes, but the the primary domain (a.k.a remote state)
// is the http:// version of that domain
//
// https://github.com/cypress-io/cypress/issues/25891
// @ts-expect-error - TS doesn't like the Request intersection
router.use('/', (req: Request & { proxiedUrl: string }, res, next) => {
if (
// only these paths will receive the relevant https upgrade check
(req.path !== '/' && req.path !== clientRoute)
// not an https upgrade request if not https protocol
|| req.protocol !== 'https'
// primary has not been established by a cy.visit() yet
|| !remoteStates.hasPrimary()
) {
return next()
}

const primary = remoteStates.getPrimary()

// props can be null in certain circumstances even if the primary is established
if (!primary.props) {
return next()
}

const primaryHostname = cors.domainPropsToHostname(primary.props)

// domain matches (example.com === example.com), but incoming request is
// https:// (established above), while the domain the user is trying to
// visit (a.k.a primary origin) is http://
if (
primaryHostname === req.hostname
&& primary.origin.startsWith('http:')
) {
res.status(301).redirect(req.proxiedUrl.replace('https://', 'http://'))

return
}

next()
})

router.get(`/${config.namespace}/tests`, (req, res, next) => {
// slice out the cache buster
const test = CacheBuster.strip(req.query.p)
Expand Down
208 changes: 208 additions & 0 deletions packages/server/test/unit/routes_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
import type { NetworkProxy } from '@packages/proxy'
import type HttpProxy from 'http-proxy'
import type { RemoteStates } from '../../lib/remote_states'

import chai, { expect } from 'chai'
import sinon from 'sinon'
import proxyquire from 'proxyquire'
import { Cfg } from '../../lib/project-base'

chai.use(require('@cypress/sinon-chai'))

describe('lib/routes', () => {
// https://github.com/cypress-io/cypress/issues/25891
describe('https upgrade fix', () => {
let routeOptions

beforeEach(() => {
sinon.restore()

routeOptions = {
config: {
clientRoute: '/__/',
namespace: 'namespace',
} as Cfg,
getSpec: sinon.stub().returns({}),
// @ts-expect-error
networkProxy: {
handleHttpRequest: () => {},
} as NetworkProxy,
nodeProxy: {} as HttpProxy,
onError: () => {},
// @ts-expect-error
remoteStates: {
hasPrimary: sinon.stub().returns(true),
getPrimary: sinon.stub().returns({
origin: 'http://foobar.com',
props: {
domain: 'foobar',
tld: 'com',
},
}),
} as RemoteStates,
testingType: 'e2e',
}
})

function setupCommonRoutes () {
const router = {
get: () => {},
post: () => {},
all: () => {},
use: sinon.stub(),
}

const Router = sinon.stub().returns(router)

const { createCommonRoutes } = proxyquire('../../lib/routes', {
'express': { Router },
})

createCommonRoutes(routeOptions)

return {
router,
}
}

it('sends 301 if a chrome https upgrade is detected for /', () => {
const { router } = setupCommonRoutes()

const req = {
hostname: 'foobar.com',
path: '/',
proxiedUrl: 'https://foobar.com/',
protocol: 'https',
}
const res = {
status: sinon.stub(),
redirect: sinon.stub(),
}
const next = sinon.stub().throws('next() should not be called')

res.status.returns(res)

router.use.withArgs('/').yield(req, res, next)

expect(res.status).to.be.calledWith(301)
expect(res.redirect).to.be.calledWith('http://foobar.com/')
})

it('sends 301 if a chrome https upgrade is detected for /__/', () => {
const { router } = setupCommonRoutes()

const req = {
hostname: 'foobar.com',
path: '/__/',
proxiedUrl: 'https://foobar.com/__/',
protocol: 'https',
}
const res = {
status: sinon.stub(),
redirect: sinon.stub(),
}
const next = sinon.stub().throws('next() should not be called')

res.status.returns(res)

router.use.withArgs('/').yield(req, res, next)

expect(res.status).to.be.calledWith(301)
expect(res.redirect).to.be.calledWith('http://foobar.com/__/')
})

it('is a noop if not a matching route', () => {
const { router } = setupCommonRoutes()

const req = {
hostname: 'foobar.com',
path: '/other-route',
proxiedUrl: 'https://foobar.com/other-route',
protocol: 'https',
}
const res = {
status: sinon.stub().throws('res.status() should not be called'),
}
const next = sinon.stub()

res.status.returns(res)

router.use.withArgs('/').yield(req, res, next)

expect(next).to.be.called
})

it('is a noop if primary remote state has not been established', () => {
routeOptions.remoteStates.hasPrimary.returns(false)

const { router } = setupCommonRoutes()

const req = {
hostname: 'foobar.com',
path: '/',
proxiedUrl: 'https://foobar.com/',
protocol: 'https',
}
const res = {
status: sinon.stub().throws('res.status() should not be called'),
}
const next = sinon.stub()

res.status.returns(res)

router.use.withArgs('/').yield(req, res, next)

expect(next).to.be.called
})

it('is a noop if primary hostname and request hostname do not match', () => {
const { router } = setupCommonRoutes()

const req = {
hostname: 'other.com',
path: '/',
proxiedUrl: 'https://other.com/',
protocol: 'https',
}
const res = {
status: sinon.stub().throws('res.status() should not be called'),
}
const next = sinon.stub()

res.status.returns(res)

router.use.withArgs('/').yield(req, res, next)

expect(next).to.be.called
})

it('is a noop if primary origin is https', () => {
routeOptions.remoteStates.getPrimary.returns({
origin: 'https://foobar.com',
props: {
domain: 'foobar',
tld: 'com',
},
})

const { router } = setupCommonRoutes()

const req = {
hostname: 'foobar.com',
path: '/',
proxiedUrl: 'https://foobar.com/',
protocol: 'https',
}
const res = {
status: sinon.stub().throws('res.status() should not be called'),
}
const next = sinon.stub()

res.status.returns(res)

router.use.withArgs('/').yield(req, res, next)

expect(next).to.be.called
})
})
})

5 comments on commit ec89901

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ec89901 Jan 9, 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.6.3/linux-x64/develop-ec89901b9dc242fbd6199a37ad2157f7d16512d1/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ec89901 Jan 9, 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.6.3/linux-arm64/develop-ec89901b9dc242fbd6199a37ad2157f7d16512d1/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ec89901 Jan 9, 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.6.3/darwin-x64/develop-ec89901b9dc242fbd6199a37ad2157f7d16512d1/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ec89901 Jan 9, 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.6.3/win32-x64/develop-ec89901b9dc242fbd6199a37ad2157f7d16512d1/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ec89901 Jan 9, 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.6.3/darwin-arm64/develop-ec89901b9dc242fbd6199a37ad2157f7d16512d1/cypress.tgz

Please sign in to comment.