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 regression in 3.5.0 where a cy.visit that changes superdoma… #5702

Merged
merged 11 commits into from
Nov 26, 2019
26 changes: 17 additions & 9 deletions packages/server/__snapshots__/2_cookies_spec.coffee.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,29 @@ exports['e2e cookies with baseurl'] = `
✓ 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
when redirected to a HTTP URL
✓ can set cookies on lots of redirects, ending with different domain
✓ can set cookies on lots of redirects, ending with same domain
when redirected to a HTTPS URL
✓ can set cookies on lots of redirects, ending with different domain
✓ can set cookies on lots of redirects, ending with same domain
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
when redirected to a HTTP URL
✓ can set cookies on lots of redirects, ending with different domain
✓ can set cookies on lots of redirects, ending with same domain
when redirected to a HTTPS URL
✓ can set cookies on lots of redirects, ending with different domain
✓ can set cookies on lots of redirects, ending with same domain


14 passing
18 passing


(Results)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 14
│ Passing: 14
│ Tests: 18
│ Passing: 18
│ Failing: 0 │
│ Pending: 0 │
│ Skipped: 0 │
Expand All @@ -69,9 +77,9 @@ exports['e2e cookies with baseurl'] = `

Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ cookies_spec_baseurl.coffee XX:XX 14 14 - - - │
│ ✔ cookies_spec_baseurl.coffee XX:XX 18 18 - - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! XX:XX 14 14 - - -
✔ All specs passed! XX:XX 18 18 - - -


`
Expand Down
2 changes: 1 addition & 1 deletion packages/server/__snapshots__/4_request_spec.coffee.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ exports['e2e requests / passes'] = `

redirects + requests
✓ gets and sets cookies from cy.request
✓ visits idempotant
✓ visits to a different superdomain will be resolved twice
✓ automatically follows redirects
✓ can turn off automatically following redirects
✓ follows all redirects even when they change methods
Expand Down
17 changes: 12 additions & 5 deletions packages/server/lib/request.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -435,18 +435,25 @@ module.exports = (options = {}) ->
cookies = [cookies]

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

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

Promise.map cookies, (cyCookie) ->
cookie = tough.Cookie.parse(cyCookie, { loose: true })

debug('parsing cookie %o', { cyCookie, toughCookie: cookie })

Promise.map cookies, (cookie) ->
cookie = tough.Cookie.parse(cookie, { loose: true })
cookie.name = cookie.key

if not cookie.domain
## take the domain from the URL
cookie.domain = parsedUrl.hostname
cookie.domain = defaultDomain
cookie.hostOnly = true

return if not tough.domainMatch(cookie.domain, parsedUrl.hostname)
if not tough.domainMatch(cookie.domain, defaultDomain)
debug('domain match failed:', { defaultDomain })
return

expiry = cookie.expiryTime()
if isFinite(expiry)
Expand Down
20 changes: 0 additions & 20 deletions packages/server/lib/server.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -360,26 +360,6 @@ class Server
_.invoke(reqStream, "abort")
_.invoke(currentPromisePhase, "cancel")

## if we have a buffer for this url
## then just respond with its details
## so we are idempotant and do not make
## another request
if obj = buffers.getByOriginalUrl(urlStr)
debug("got previous request buffer for url:", urlStr)

## reset the cookies from the buffer on the browser
return runPhase ->
resolve(
Promise.map obj.details.cookies, (cookie) ->
## prevent prepending a . to the cookie domain if top-level
## navigation occurs as a result of a cy.visit
if _.isUndefined(cookie.hostOnly) && !cookie.domain?.startsWith('.')
cookie.hostOnly = true

automationRequest('set:cookie', cookie)
.return(obj.details)
)

redirects = []
newUrl = null

Expand Down
45 changes: 19 additions & 26 deletions packages/server/lib/util/buffers.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const _ = require('lodash')
const debug = require('debug')('cypress:server:buffers')
const uri = require('./uri')

let buffers = []
let buffer = null

const stripPort = (url) => {
try {
Expand All @@ -13,53 +13,46 @@ const stripPort = (url) => {
}

module.exports = {
all () {
return buffers
},

keys () {
return _.map(buffers, 'url')
getAny () {
return buffer
},

reset () {
debug('resetting buffers')

buffers = []
buffer = null
},

set (obj = {}) {
obj = _.cloneDeep(obj)
obj.url = stripPort(obj.url)
obj.originalUrl = stripPort(obj.originalUrl)

debug('setting buffer %o', _.pick(obj, 'url'))

return buffers.push(_.pick(obj, 'url', 'originalUrl', 'jar', 'stream', 'response', 'details'))
},

getByOriginalUrl (str) {
const b = _.find(buffers, { originalUrl: stripPort(str) })

if (b) {
debug('found request buffer by original url %o', { str, buffer: _.pick(b, 'url', 'originalUrl', 'details'), bufferCount: buffers.length })
if (buffer) {
debug('warning: overwriting existing buffer...', { buffer: _.pick(buffer, 'url') })
}

return b
debug('setting buffer %o', _.pick(obj, 'url'))

buffer = obj
},

get (str) {
return _.find(buffers, { url: stripPort(str) })
if (buffer && buffer.url === stripPort(str)) {
return buffer
}
},

take (str) {
const buffer = this.get(str)
const foundBuffer = this.get(str)

if (buffer) {
buffers = _.without(buffers, buffer)
if (foundBuffer) {
buffer = null

debug('found request buffer %o', { buffer: _.pick(buffer, 'url', 'originalUrl'), bufferCount: buffers.length })
}
debug('found request buffer %o', { buffer: _.pick(foundBuffer, 'url') })

return buffer
return foundBuffer
}
},

}
1 change: 1 addition & 0 deletions packages/server/test/e2e/2_cookies_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ onServer = (app) ->

res.header("Set-Cookie", [
"namefoo#{n}=valfoo#{n}"
"namebar#{n}=valbar#{n}"
])

console.log('to', a, 'from', b)
Expand Down
23 changes: 13 additions & 10 deletions packages/server/test/integration/server_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ describe "Server", ->
cookies: []
})

expect(buffers.keys()).to.deep.eq(["http://localhost:2000/index.html"])
expect(buffers.getAny()).to.include({ url: "http://localhost:2000/index.html" })
.then =>
@server._onResolveUrl("/index.html", {}, @automationRequest)
.then (obj = {}) =>
Expand All @@ -187,15 +187,15 @@ describe "Server", ->
cookies: []
})

expect(@server._request.sendStream).to.be.calledOnce
expect(@server._request.sendStream).to.be.calledTwice
.then =>
@rp("http://localhost:2000/index.html")
.then (res) =>
expect(res.statusCode).to.eq(200)
expect(res.body).to.include("document.domain")
expect(res.body).to.include("localhost")
expect(res.body).to.include("Cypress")
expect(buffers.keys()).to.deep.eq([])
expect(buffers.getAny()).to.be.null

it "can follow static file redirects", ->
@server._onResolveUrl("/sub", {}, @automationRequest)
Expand Down Expand Up @@ -266,13 +266,13 @@ describe "Server", ->
cookies: []
})

expect(buffers.keys()).to.deep.eq(["http://localhost:2000/index.html"])
expect(buffers.getAny()).to.include({ url: "http://localhost:2000/index.html" })
.then =>
@rp("http://localhost:2000/index.html")
.then (res) ->
expect(res.statusCode).to.eq(200)

expect(buffers.keys()).to.deep.eq([])
expect(buffers.getAny()).to.be.null

describe "http", ->
beforeEach ->
Expand Down Expand Up @@ -496,16 +496,19 @@ describe "Server", ->

nock("http://espn.com")
.get("/")
.times(2)
.reply 301, undefined, {
"Location": "/foo"
}
.get("/foo")
.times(2)
.reply 302, undefined, {
"Location": "http://espn.go.com/"
}

nock("http://espn.go.com")
.get("/")
.times(2)
.reply 200, "<html><head></head><body>espn</body></html>", {
"Content-Type": "text/html"
}
Expand All @@ -527,7 +530,7 @@ describe "Server", ->
]
})

expect(buffers.keys()).to.deep.eq(["http://espn.go.com/"])
expect(buffers.getAny()).to.include({ url: "http://espn.go.com/" })
.then =>
@server._onResolveUrl("http://espn.com/", {}, @automationRequest)
.then (obj = {}) =>
Expand All @@ -546,15 +549,15 @@ describe "Server", ->
]
})

expect(@server._request.sendStream).to.be.calledOnce
expect(@server._request.sendStream).to.be.calledTwice
.then =>
@rp("http://espn.go.com/")
.then (res) =>
expect(res.statusCode).to.eq(200)
expect(res.body).to.include("document.domain")
expect(res.body).to.include("go.com")
expect(res.body).to.include("Cypress.action('app:window:before:load', window); </script></head><body>espn</body></html>")
expect(buffers.keys()).to.deep.eq([])
expect(buffers.getAny()).to.be.null

it "does not buffer 'bad' responses", ->
sinon.spy(@server._request, "sendStream")
Expand Down Expand Up @@ -665,13 +668,13 @@ describe "Server", ->
cookies: []
})

expect(buffers.keys()).to.deep.eq(["http://getbootstrap.com/"])
expect(buffers.getAny()).to.include({ url: "http://getbootstrap.com/" })
.then =>
@rp("http://getbootstrap.com/")
.then (res) ->
expect(res.statusCode).to.eq(200)

expect(buffers.keys()).to.deep.eq([])
expect(buffers.getAny()).to.be.null

it "can serve non 2xx status code requests when option set", ->
nock("http://google.com")
Expand Down
Loading