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
2 changes: 1 addition & 1 deletion packages/driver/src/cy/commands/xhr.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->

Cypress.on "test:before:run:async", ->
## reset any state on the backend
Cypress.backend('reset:xhr:server')
Cypress.backend('reset:server:state')

Cypress.on "test:before:run", ->
## reset the existing server
Expand Down
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,27 +31,35 @@ 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
with Domain = superdomain
✓ is set properly with no redirects
✓ is set properly with redirects
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
with Domain = superdomain
✓ is set properly with no redirects
✓ is set properly with redirects
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


18 passing
22 passing


(Results)

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

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


`
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 following redirects that set a cookie
✓ can turn off automatically following redirects
Expand Down
29 changes: 8 additions & 21 deletions packages/server/lib/server.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,10 @@ class Server
options
})

## always clear buffers - reduces the possibility of a random HTTP request
## accidentally retrieving buffered content at the wrong time
buffers.reset()

startTime = new Date()

## if we have an existing url resolver
Expand Down Expand Up @@ -361,26 +365,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 Expand Up @@ -708,7 +692,10 @@ class Server
options.onResolveUrl = @_onResolveUrl.bind(@)
options.onRequest = @_onRequest.bind(@)
options.onIncomingXhr = @_xhrServer.onIncomingXhr
options.onResetXhrServer = @_xhrServer.reset

options.onResetServerState = =>
@_xhrServer.reset()
buffers.reset()

@_socket = Socket(config)
@_socket.startListening(@_server, automation, config, options)
Expand Down
6 changes: 3 additions & 3 deletions packages/server/lib/socket.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ class Socket
_.defaults options,
socketId: null
onIncomingXhr: ->
onResetXhrServer: ->
onResetServerState: ->
onSetRunnables: ->
onMocha: ->
onConnect: ->
Expand Down Expand Up @@ -300,8 +300,8 @@ class Socket
options.onResolveUrl(url, headers, automationRequest, resolveOpts)
when "http:request"
options.onRequest(headers, automationRequest, args[0])
when "reset:xhr:server"
options.onResetXhrServer()
when "reset:server:state"
options.onResetServerState()
when "incoming:xhr"
options.onIncomingXhr(args[0], args[1])
when "get:fixture"
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 @@ -77,6 +77,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