Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: hanging on basic auth requests in chromium browsers #27781

Merged
merged 3 commits into from Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cli/CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@ _Released 09/12/2023 (PENDING)_
**Bugfixes:**

- Fixed an issue where `enter`, `keyup`, and `space` events where not triggering `click` events properly in some versions of Firefox. Addressed in [#27715](https://github.com/cypress-io/cypress/pull/27715).
- Fixed a regression in `13.0.0` where tests using Basic Authorization can potentially hang indefinitely on chromium browsers. Addressed in [#27781](https://github.com/cypress-io/cypress/pull/27781)

**Dependency Updates:**

Expand Down
24 changes: 17 additions & 7 deletions packages/socket/lib/browser.ts
Expand Up @@ -4,18 +4,26 @@ import type { SocketShape } from './types'

export type { Socket } from 'socket.io-client'

const sockets: {[key: string]: CDPBrowserSocket} = {}
declare global {
interface Window {
cypressSockets: {[key: string]: CDPBrowserSocket}
}
}

window.cypressSockets ||= {}
Copy link
Member

Choose a reason for hiding this comment

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

why do you need to add sockets to window for this fix? Is it for testing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added some doc to explain this a little better: 27b44ed (#27781)

let chromium = false

export function client (uri: string, opts?: Partial<ManagerOptions & SocketOptions>): SocketShape {
if (chromium) {
const fullNamespace = `${opts?.path}${uri}`

if (!sockets[fullNamespace]) {
sockets[fullNamespace] = new CDPBrowserSocket(fullNamespace)
if (!window.cypressSockets[fullNamespace]) {
window.cypressSockets[fullNamespace] = new CDPBrowserSocket(fullNamespace)
}

return sockets[fullNamespace]
window.cypressSockets[fullNamespace].connect()
Copy link
Member

Choose a reason for hiding this comment

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

What does connect do that we weren't doing before and why does it fix the hang?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added some doc to explain this a little better: 27b44ed (#27781)


return window.cypressSockets[fullNamespace]
}

return io(uri, opts)
Expand All @@ -27,11 +35,13 @@ export function createWebsocket ({ path, browserFamily }: { path: string, browse

const fullNamespace = `${path}/default`

if (!sockets[fullNamespace]) {
sockets[fullNamespace] = new CDPBrowserSocket(fullNamespace)
if (!window.cypressSockets[fullNamespace]) {
window.cypressSockets[fullNamespace] = new CDPBrowserSocket(fullNamespace)
}

return sockets[fullNamespace]
window.cypressSockets[fullNamespace].connect()

return window.cypressSockets[fullNamespace]
}

return io({
Expand Down
2 changes: 2 additions & 0 deletions packages/socket/lib/cdp-browser.ts
Expand Up @@ -43,7 +43,9 @@ export class CDPBrowserSocket extends Emitter implements SocketShape {
if (!cypressSocket.send) {
cypressSocket.send = send
}
}

connect () {
// Set timeout so that the connect event is emitted after the constructor returns and the user has a chance to attach a listener
setTimeout(() => {
super.emit('connect')
Expand Down
55 changes: 55 additions & 0 deletions system-tests/__snapshots__/base_url_spec.js
Expand Up @@ -107,3 +107,58 @@ exports['e2e baseUrl / http / passes'] = `


`

exports['e2e baseUrl / https basic auth / passes'] = `

====================================================================================================

(Run Starting)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (base_url.cy.js) │
│ Searched: cypress/e2e/base_url.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


────────────────────────────────────────────────────────────────────────────────────────────────────

Running: base_url.cy.js (1 of 1)


base url
✓ can visit


1 passing


(Results)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 1 │
│ Passing: 1 │
│ Failing: 0 │
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 0 │
│ Video: false │
│ Duration: X seconds │
│ Spec Ran: base_url.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


====================================================================================================

(Run Finished)


Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ base_url.cy.js XX:XX 1 1 - - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! XX:XX 1 1 - - -


`
21 changes: 21 additions & 0 deletions system-tests/test/base_url_spec.js
Expand Up @@ -27,6 +27,27 @@ describe('e2e baseUrl', () => {
})
})

context('https basic auth', () => {
systemTests.setup({
servers: {
port: 443,
https: true,
onServer,
},
settings: {
e2e: {
baseUrl: 'https://test:test@localhost/app',
},
},
})

systemTests.it('passes', {
spec: 'base_url.cy.js',
browser: 'chrome',
snapshot: true,
})
})

context('http', () => {
systemTests.setup({
servers: {
Expand Down