Skip to content

Commit

Permalink
Fix cy.clearCookie, fix HTTP redirect behavior, fix cy.visit HT… (#5478)
Browse files Browse the repository at this point in the history 
* Add a bunch of extra tests in 2_cookies_spec

* Try not doing weird things to cookies

* if cypress.env.debug is set, log command execution thru server

* improve elctron logging - we missed this when doing cri-client logging

* make video_capture frames part of verbose

* cleanup

* use the data from getCookie to run deleteCookies

* fix screenshots

* still resolve with cleared cookie

* cy.getcookie now gets ALL cookies from ALL domains

* return Promise for followRedirect, not req.init wrap

* allow passing domain: null to cy.getCookies to get all

* update request_spec to be clearer

* still need to call followRedirect option during sendStream

* beautify the e2e tests

* correct e2e test + snapshot

* always discard default ports when get/set buffer - fixes #5367

* make cy.clearCookies({ domain: null }) clear ALL cookies

* update spec

* use string url as key

* rebalance some e2e tests to make time for new cookies e2e tests

* always add default port to buffer url

* jk, remove default port

* set hostOnly: true when appropriate when setting cookies back on visit

* update tests

* Revert "if cypress.env.debug is set, log command execution thru server"

This reverts commit 623ed44.

* try to clean diff, cookies_no_baseurl didnt change

* WIP move the expected cookies array out of snapshot and expect inline

* finish updating tests

* add missing snapshot

* remove useless onstdout


Co-authored-by: Brian Mann <[email protected]>
  • Loading branch information
@flotwig @brian-mann
flotwig and brian-mann committed Nov 8, 2019
1 parent 8298881 commit 98063ae
Show file tree
Hide file tree
Showing 21 changed files with 576 additions and 148 deletions.
13 changes: 4 additions & 9 deletions packages/driver/src/cy/commands/cookies.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,8 @@ mergeDefaults = (obj) ->
## we always want to be able to see and influence cookies
## on our superdomain
{ superDomain } = $Location.create(window.location.href)
# { hostname } = $Location.create(window.location.href)

merge = (o) ->
## we are hostOnly if we dont have an
## explicit domain
# o.hostOnly = !o.domain

## and if the user did not provide a domain
## then we know to set the default to be origin
_.defaults o, {domain: superDomain}
Expand Down Expand Up @@ -63,8 +58,8 @@ module.exports = (Commands, Cypress, cy, state, config) ->
}
}

getAndClear = (log, timeout) ->
automateCookies("get:cookies", {}, log, timeout)
getAndClear = (log, timeout, options = {}) ->
automateCookies("get:cookies", options, log, timeout)
.then (resp) =>
## bail early if we got no cookies!
return resp if resp and resp.length is 0
Expand Down Expand Up @@ -137,7 +132,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
obj
})

automateCookies("get:cookies", {}, options._log, options.timeout)
automateCookies("get:cookies", _.pick(options, 'domain'), options._log, options.timeout)
.then (resp) ->
options.cookies = resp

Expand Down Expand Up @@ -257,7 +252,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
obj
})

getAndClear(options._log, options.timeout)
getAndClear(options._log, options.timeout, { domain: options.domain })
.then (resp) ->
options.cookies = resp

Expand Down
93 changes: 86 additions & 7 deletions packages/server/__snapshots__/2_cookies_spec.coffee.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
exports['e2e cookies / passes'] = `
exports['e2e cookies with baseurl'] = `
====================================================================================================
Expand All @@ -7,14 +7,92 @@ exports['e2e cookies / passes'] = `
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (cookies_spec.coffee) │
│ Searched: cypress/integration/cookies_spec.coffee │
│ Specs: 1 found (cookies_spec_baseurl.coffee) │
│ Searched: cypress/integration/cookies_spec_baseurl.coffee │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
────────────────────────────────────────────────────────────────────────────────────────────────────
Running: cookies_spec.coffee (1 of 1)
Running: cookies_spec_baseurl.coffee (1 of 1)
cookies
with whitelist
✓ can get all cookies
✓ resets cookies between tests correctly
✓ should be only two left now
✓ handles undefined cookies
without whitelist
✓ sends set cookies to path
✓ handles expired cookies secure
✓ issue: #224 sets expired cookies between redirects
✓ issue: #1321 failing to set or parse cookie
✓ issue: #2724 does not fail on invalid cookies
✓ can set and clear cookie
in a cy.visit
✓ can set cookies on way too many redirects with HTTP intermediary
✓ can set cookies on way too many redirects with HTTPS intermediary
in a cy.request
✓ can set cookies on way too many redirects with HTTP intermediary
✓ can set cookies on way too many redirects with HTTPS intermediary
14 passing
(Results)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 14 │
│ Passing: 14 │
│ Failing: 0 │
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 0 │
│ Video: true │
│ Duration: X seconds │
│ Spec Ran: cookies_spec_baseurl.coffee │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
(Video)
- Started processing: Compressing to 32 CRF
- Finished processing: /XXX/XXX/XXX/cypress/videos/cookies_spec_baseurl.coffee.mp4 (X second)
====================================================================================================
(Run Finished)
Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ cookies_spec_baseurl.coffee XX:XX 14 14 - - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! XX:XX 14 14 - - -
`

exports['e2e cookies with no baseurl'] = `
====================================================================================================
(Run Starting)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (cookies_spec_no_baseurl.coffee) │
│ Searched: cypress/integration/cookies_spec_no_baseurl.coffee │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
────────────────────────────────────────────────────────────────────────────────────────────────────
Running: cookies_spec_no_baseurl.coffee (1 of 1)
cookies
Expand Down Expand Up @@ -45,14 +123,15 @@ exports['e2e cookies / passes'] = `
│ Screenshots: 0 │
│ Video: true │
│ Duration: X seconds │
│ Spec Ran: cookies_spec.coffee │
│ Spec Ran: cookies_spec_no_baseurl.coffee │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
(Video)
- Started processing: Compressing to 32 CRF
- Finished processing: /XXX/XXX/XXX/cypress/videos/cookies_spec.coffee.mp4 (X second)
- Finished processing: /XXX/XXX/XXX/cypress/videos/cookies_spec_no_baseurl.coffee. (X second)
mp4
====================================================================================================
Expand All @@ -62,7 +141,7 @@ exports['e2e cookies / passes'] = `
Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ cookies_spec.coffee XX:XX 9 9 - - - │
│ ✔ cookies_spec_no_baseurl.coffee XX:XX 9 9 - - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! XX:XX 9 9 - - -
Expand Down
0 ...ver/__snapshots__/2_config_spec.coffee.js → ...ver/__snapshots__/3_config_spec.coffee.js
View file
File renamed without changes.
0 ...snapshots__/2_browser_path_spec.coffee.js → ...snapshots__/4_browser_path_spec.coffee.js
View file
File renamed without changes.
0 ...shots__/2_form_submissions_spec.coffee.js → ...shots__/4_form_submissions_spec.coffee.js
View file
File renamed without changes.
38 changes: 12 additions & 26 deletions packages/server/__snapshots__/request_spec.coffee.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,26 @@ exports['lib/request #sendPromise followRedirect gets + attaches the cookies at
"setCalls": [
{
"currentUrl": "http://localhost:1234/",
"setCookie": "foo=bar"
"setCookie": "one=1"
},
{
"currentUrl": "http://localhost:1234/B",
"setCookie": "bar=baz"
"currentUrl": "http://localhost:1234/second",
"setCookie": "two=2"
},
{
"currentUrl": "http://localhost:1234/B",
"setCookie": "foo=bar"
},
{
"currentUrl": "http://localhost:1234/",
"setCookie": "quuz=quux"
"currentUrl": "http://localhost:1234/third",
"setCookie": "three=3"
}
],
"getCalls": [
{
"newUrl": "http://localhost:1234/"
},
{
"newUrl": "http://localhost:1234/B"
},
{
"newUrl": "http://localhost:1234/B"
"newUrl": "http://localhost:1234/second"
},
{
"newUrl": "http://localhost:1234/B"
"newUrl": "http://localhost:1234/third"
}
]
}
Expand All @@ -37,29 +30,22 @@ exports['lib/request #sendStream gets + attaches the cookies at each redirect 1'
"setCalls": [
{
"currentUrl": "http://localhost:1234/",
"setCookie": "foo=bar"
"setCookie": "one=1"
},
{
"currentUrl": "http://localhost:1234/B",
"setCookie": "bar=baz"
},
{
"currentUrl": "http://localhost:1234/B",
"setCookie": "foo=bar"
"currentUrl": "http://localhost:1234/second",
"setCookie": "two=2"
}
],
"getCalls": [
{
"newUrl": "http://localhost:1234/"
},
{
"newUrl": "http://localhost:1234/B"
},
{
"newUrl": "http://localhost:1234/B"
"newUrl": "http://localhost:1234/second"
},
{
"newUrl": "http://localhost:1234/B"
"newUrl": "http://localhost:1234/third"
}
]
}
22 changes: 15 additions & 7 deletions packages/server/lib/browsers/cdp_automation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,19 @@ export const CdpAutomation = (sendDebuggerCommandFn: SendDebuggerCommand) => {
}

const normalizeSetCookieProps = (cookie: CyCookie): cdp.Network.SetCookieRequest => {
// this logic forms a SetCookie request that will be received by Chrome
// see MakeCookieFromProtocolValues for information on how this cookie data will be parsed
// @see https://cs.chromium.org/chromium/src/content/browser/devtools/protocol/network_handler.cc?l=246&rcl=786a9194459684dc7a6fded9cabfc0c9b9b37174

_.defaults(cookie, {
name: '',
value: '',
})

// this logic forms a SetCookie request that will be received by Chrome
// see MakeCookieFromProtocolValues for information on how this cookie data will be parsed
// @see https://cs.chromium.org/chromium/src/content/browser/devtools/protocol/network_handler.cc?l=246&rcl=786a9194459684dc7a6fded9cabfc0c9b9b37174

// @ts-ignore
cookie.expires = cookie.expirationDate

// without this logic, a cookie being set on 'foo.com' will only be set for 'foo.com', not other subdomains
if (!cookie.hostOnly && cookie.domain[0] !== '.') {
let parsedDomain = cors.parseDomain(cookie.domain)

Expand Down Expand Up @@ -135,9 +137,15 @@ export const CdpAutomation = (sendDebuggerCommandFn: SendDebuggerCommand) => {
})
case 'clear:cookie':
return getCookie(data)
// so we can resolve with the value of the removed cookie
.tap((_cookieToBeCleared) => {
return sendDebuggerCommandFn('Network.deleteCookies', data)
// tap, so we can resolve with the value of the removed cookie
// also, getting the cookie via CDP first will ensure that we send a cookie `domain` to CDP
// that matches the cookie domain that is really stored
.tap((cookieToBeCleared) => {
if (!cookieToBeCleared) {
return
}

return sendDebuggerCommandFn('Network.deleteCookies', _.pick(cookieToBeCleared, 'name', 'domain'))
})
case 'is:automation:client:connected':
return true
Expand Down
8 changes: 8 additions & 0 deletions packages/server/lib/browsers/electron.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,15 @@ getAutomation = (win) ->
sendDebuggerCommand = (message, data) ->
## wrap in bluebird
tryToCall win, Promise.method ->
debug('debugger: sending %s with params %o', message, data)
win.webContents.debugger.sendCommand(message, data)
.tap (res) ->
if debug.enabled && res.data && res.data.length > 100
res = _.clone(res)
res.data = res.data.slice(0, 100) + ' [truncated]'
debug('debugger: received response to %s: %o', message, res)
.tapCatch (err) ->
debug('debugger: received error on %s: %o', messsage, err)

CdpAutomation(sendDebuggerCommand)

Expand Down
43 changes: 16 additions & 27 deletions packages/server/lib/request.coffee
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ module.exports = (options = {}) ->
cookies = [cookies]

parsedUrl = url.parse(resUrl)
debug('setting cookies on browser %o', { url: parsedUrl, cookies })
debug('setting cookies on browser %o', { url: parsedUrl.href, cookies })

Promise.map cookies, (cookie) ->
cookie = tough.Cookie.parse(cookie, { loose: true })
Expand Down Expand Up @@ -478,24 +478,21 @@ module.exports = (options = {}) ->
currentUrl = options.url

options.followRedirect = (incomingRes) ->
req = @
if followRedirect and not followRedirect(incomingRes)
return false

newUrl = url.resolve(currentUrl, incomingRes.headers.location)

## and when we know we should follow the redirect
## we need to override the init method and
## first set the received cookies on the browser
## and then grab the cookies for the new url
req.init = _.wrap req.init, (orig, opts) =>
options.onBeforeReqInit ->
self.setCookiesOnBrowser(incomingRes, currentUrl, automationFn)
.then (cookies) ->
self.setRequestCookieHeader(req, newUrl, automationFn)
.then (cookieHeader) ->
currentUrl = newUrl
orig.call(req, opts)

followRedirect.call(req, incomingRes)
self.setCookiesOnBrowser(incomingRes, currentUrl, automationFn)
.then (cookies) =>
self.setRequestCookieHeader(@, newUrl, automationFn)
.then =>
currentUrl = newUrl
true

@setRequestCookieHeader(options, options.url, automationFn)
.then =>
Expand Down Expand Up @@ -565,24 +562,16 @@ module.exports = (options = {}) ->

push(incomingRes)

req = @

## and when we know we should follow the redirect
## we need to override the init method and
## first set the new cookies on the browser
## and then grab the cookies for the new url
req.init = _.wrap req.init, (orig, opts) =>
self.setCookiesOnBrowser(incomingRes, currentUrl, automationFn)
.then ->
self.setRequestCookieHeader(req, newUrl, automationFn)
.then ->
currentUrl = newUrl
orig.call(req, opts)

## cause the redirect to happen
## but swallow up the incomingRes
## so we can build an array of responses
return true
self.setCookiesOnBrowser(incomingRes, currentUrl, automationFn)
.then =>
self.setRequestCookieHeader(@, newUrl, automationFn)
.then =>
currentUrl = newUrl
true

@create(options, true)
.then(@normalizeResponse.bind(@, push))
Expand All @@ -602,7 +591,7 @@ module.exports = (options = {}) ->
## the current url
resp.redirectedToUrl = url.resolve(options.url, loc)

@setCookiesOnBrowser(resp, options.url, automationFn)
@setCookiesOnBrowser(resp, currentUrl, automationFn)
.return(resp)

if c = options.cookies
Expand Down
Loading

5 comments on commit 98063ae

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 98063ae Nov 8, 2019

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.

You can install this pre-release platform-specific build using instructions at https://on.cypress.io/installing-cypress#Install-pre-release-version.

You will need to use custom CYPRESS_INSTALL_BINARY url and install Cypress using an url instead of the version.

export CYPRESS_INSTALL_BINARY=https://cdn.cypress.io/beta/binary/3.6.1/darwin-x64/circle-develop-98063aec64fe42df6680f227a2aaf5692b73af88-188088/cypress.zip
npm install https://cdn.cypress.io/beta/npm/3.6.1/circle-develop-98063aec64fe42df6680f227a2aaf5692b73af88-188085/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 98063ae Nov 8, 2019

Choose a reason for hiding this comment

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

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

You can install this pre-release platform-specific build using instructions at https://on.cypress.io/installing-cypress#Install-pre-release-version.

You will need to use custom CYPRESS_INSTALL_BINARY url and install Cypress using an url instead of the version.

set CYPRESS_INSTALL_BINARY=https://cdn.cypress.io/beta/binary/3.6.1/win32-x64/appveyor-develop-98063aec64fe42df6680f227a2aaf5692b73af88-28703659/cypress.zip
npm install https://cdn.cypress.io/beta/binary/3.6.1/win32-x64/appveyor-develop-98063aec64fe42df6680f227a2aaf5692b73af88-28703659/cypress.zip

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 98063ae Nov 8, 2019

Choose a reason for hiding this comment

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

AppVeyor has built the win32 ia32 version of the Test Runner.

You can install this pre-release platform-specific build using instructions at https://on.cypress.io/installing-cypress#Install-pre-release-version.

You will need to use custom CYPRESS_INSTALL_BINARY url and install Cypress using an url instead of the version.

set CYPRESS_INSTALL_BINARY=https://cdn.cypress.io/beta/binary/3.6.1/win32-ia32/appveyor-develop-98063aec64fe42df6680f227a2aaf5692b73af88-28703659/cypress.zip
npm install https://cdn.cypress.io/beta/binary/3.6.1/win32-ia32/appveyor-develop-98063aec64fe42df6680f227a2aaf5692b73af88-28703659/cypress.zip

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 98063ae Nov 8, 2019

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.

You can install this pre-release platform-specific build using instructions at https://on.cypress.io/installing-cypress#Install-pre-release-version.

You will need to use custom CYPRESS_INSTALL_BINARY url and install Cypress using an url instead of the version.

export CYPRESS_INSTALL_BINARY=https://cdn.cypress.io/beta/binary/3.6.1/linux-x64/circle-develop-98063aec64fe42df6680f227a2aaf5692b73af88-188635/cypress.zip
npm install https://cdn.cypress.io/beta/npm/3.6.1/circle-develop-98063aec64fe42df6680f227a2aaf5692b73af88-188080/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 98063ae Nov 8, 2019

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.

You can install this pre-release platform-specific build using instructions at https://on.cypress.io/installing-cypress#Install-pre-release-version.

You will need to use custom CYPRESS_INSTALL_BINARY url and install Cypress using an url instead of the version.

export CYPRESS_INSTALL_BINARY=https://cdn.cypress.io/beta/binary/3.6.1/linux-x64/circle-develop-98063aec64fe42df6680f227a2aaf5692b73af88-188635/cypress.zip
npm install https://cdn.cypress.io/beta/npm/3.6.1/circle-develop-98063aec64fe42df6680f227a2aaf5692b73af88-188080/cypress.tgz

Please sign in to comment.