Skip to content

Allow circular structures to be sent over the websocket, make it an error to send circular request bodies #4407

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

Merged
merged 18 commits into from
Jun 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/driver/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"url-parse": "1.4.7",
"vanilla-text-mask": "5.1.1",
"wait-on": "3.2.0",
"@cypress/what-is-circular": "1.0.0",
"zone.js": "0.9.0"
}
}
4 changes: 4 additions & 0 deletions packages/driver/src/cy/commands/navigation.coffee
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
_ = require("lodash")
whatIsCircular = require("@cypress/what-is-circular")
moment = require("moment")
UrlParse = require("url-parse")
Promise = require("bluebird")
Expand Down Expand Up @@ -522,6 +523,9 @@ module.exports = (Commands, Cypress, cy, state, config) ->
if not _.isObject(options.headers)
$utils.throwErrByPath("visit.invalid_headers")

if _.isObject(options.body) and path = whatIsCircular(options.body)
$utils.throwErrByPath("visit.body_circular", { args: { path }})

if options.log
message = url

Expand Down
4 changes: 4 additions & 0 deletions packages/driver/src/cy/commands/request.coffee
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
_ = require("lodash")
whatIsCircular = require("@cypress/what-is-circular")
Promise = require("bluebird")

$utils = require("../../cypress/utils")
Expand Down Expand Up @@ -137,6 +138,9 @@ module.exports = (Commands, Cypress, cy, state, config) ->
if needsFormSpecified(options)
options.form = true

if _.isObject(options.body) and path = whatIsCircular(options.body)
$utils.throwErrByPath("request.body_circular", { args: { path }})

## only set json to true if form isnt true
## and we have a valid object for body
if options.form isnt true and isValidJsonObj(options.body)
Expand Down
10 changes: 10 additions & 0 deletions packages/driver/src/cypress/error_messages.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,11 @@ module.exports = {
invalid_arguments: "#{cmd('reload')} can only accept a boolean or options as its arguments."

request:
body_circular: ({ path }) -> """
The `body` parameter supplied to #{cmd('request')} contained a circular reference at the path "#{path.join(".")}".

`body` can only be a string or an object with no circular references.
"""
status_code_flags_invalid: """
#{cmd('request')} was invoked with { failOnStatusCode: false, retryOnStatusCodeFailure: true }.

Expand Down Expand Up @@ -931,6 +936,11 @@ module.exports = {
missing_preset: "#{cmd('viewport')} could not find a preset for: '{{preset}}'. Available presets are: {{presets}}"

visit:
body_circular: ({ path }) -> """
The `body` parameter supplied to #{cmd('visit')} contained a circular reference at the path "#{path.join(".")}".

`body` can only be a string or an object with no circular references.
"""
status_code_flags_invalid: """
#{cmd('visit')} was invoked with { failOnStatusCode: false, retryOnStatusCodeFailure: true }.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1455,6 +1455,34 @@ describe "src/cy/commands/navigation", ->

cy.visit("https://google.com/foo")

it "displays body_circular when body is circular", (done) ->
foo = {
bar: {
baz: {}
}
}

foo.bar.baz.quux = foo

cy.visit({
method: "POST"
url: "http://foo.invalid/"
body: foo
})

cy.on "fail", (err) =>
lastLog = @lastLog
expect(@logs.length).to.eq(1)
expect(lastLog.get("error")).to.eq(err)
expect(lastLog.get("state")).to.eq("failed")
expect(err.message).to.eq """
The `body` parameter supplied to cy.visit() contained a circular reference at the path "bar.baz.quux".

`body` can only be a string or an object with no circular references.
"""

done()

context "#page load", ->
it "sets initial=true and then removes", ->
Cookie.remove("__cypress.initial")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,34 @@ describe "src/cy/commands/request", ->
}
})

it "displays body_circular when body is circular", (done) ->
foo = {
bar: {
baz: {}
}
}

foo.bar.baz.quux = foo

cy.request({
method: "POST"
url: "http://foo.invalid/"
body: foo
})

cy.on "fail", (err) =>
lastLog = @lastLog
expect(@logs.length).to.eq(1)
expect(lastLog.get("error")).to.eq(err)
expect(lastLog.get("state")).to.eq("failed")
expect(err.message).to.eq """
The `body` parameter supplied to cy.request() contained a circular reference at the path "bar.baz.quux".

`body` can only be a string or an object with no circular references.
"""

done()

it "does not include redirects when there were no redirects", (done) ->
backend = Cypress.backend
.withArgs("http:request")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,20 @@ describe "driver/src/cypress/index", ->

done()

## https://github.com/cypress-io/cypress/issues/4346
it "can complete if a circular reference is sent", ->
foo = {
bar: {}
}

foo.bar.baz = foo

Cypress.backend("foo", foo)
.then ->
throw new Error("should not reach")
.catch (e) ->
expect(e.message).to.eq("You requested a backend event we cannot handle: foo")

context ".isCy", ->
it "returns true on cy, cy chainable", ->
expect(Cypress.isCy(cy)).to.be.true
Expand Down Expand Up @@ -75,4 +89,4 @@ describe "driver/src/cypress/index", ->
fn = ->
Cypress.log({ message: 'My Log' })

expect(fn).to.not.throw()
expect(fn).to.not.throw()
34 changes: 16 additions & 18 deletions packages/extension/app/background.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ map = require("lodash/map")
pick = require("lodash/pick")
once = require("lodash/once")
Promise = require("bluebird")
{ client, circularParser } = require("@packages/socket")

HOST = "CHANGE_ME_HOST"
PATH = "CHANGE_ME_PATH"
Expand All @@ -12,38 +13,35 @@ firstOrNull = (cookies) ->
## normalize into null when empty array
cookies[0] ? null

connect = (host, path, io) ->
io ?= global.io

## bail if io isnt defined
return if not io

connect = (host, path) ->
listenToCookieChanges = once ->
chrome.cookies.onChanged.addListener (info) ->
if info.cause isnt "overwrite"
client.emit("automation:push:request", "change:cookie", info)
ws.emit("automation:push:request", "change:cookie", info)

fail = (id, err) ->
client.emit("automation:response", id, {
ws.emit("automation:response", id, {
__error: err.message
__stack: err.stack
__name: err.name
})

invoke = (method, id, args...) ->
respond = (data) ->
client.emit("automation:response", id, {response: data})
ws.emit("automation:response", id, {response: data})

Promise.try ->
automation[method].apply(automation, args.concat(respond))
.catch (err) ->
fail(id, err)

## cannot use required socket here due
## to bug in socket io client with browserify
client = io.connect(host, {path: path, transports: ["websocket"]})
ws = client.connect(host, {
path: path,
transports: ["websocket"]
parser: circularParser
})

client.on "automation:request", (id, msg, data) ->
ws.on "automation:request", (id, msg, data) ->
switch msg
when "get:cookies"
invoke("getCookies", id, data)
Expand All @@ -64,18 +62,18 @@ connect = (host, path, io) ->
else
fail(id, {message: "No handler registered for: '#{msg}'"})

client.on "connect", ->
ws.on "connect", ->
listenToCookieChanges()

client.emit("automation:client:connected")
ws.emit("automation:client:connected")

return client
return ws

## initially connect
connect(HOST, PATH, global.io)
connect(HOST, PATH)

automation = {
connect: connect
connect

getUrl: (cookie = {}) ->
prefix = if cookie.secure then "https://" else "http://"
Expand Down
1 change: 0 additions & 1 deletion packages/extension/app/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
},
"background": {
"scripts": [
"socket.io.js",
"background.js"
]
},
Expand Down
6 changes: 0 additions & 6 deletions packages/extension/gulpfile.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ Promise = require("bluebird")
coffeeify = require("coffeeify")
browserify = require("browserify")
icons = require("@cypress/icons")
ext = require("./")

gulp.task "copy:socket:client", ->
gulp.src(require("../socket").getPathToClientSource())
.pipe(gulp.dest("dist"))

gulp.task "clean", ->
gulp.src("dist")
Expand Down Expand Up @@ -71,7 +66,6 @@ gulp.task "watch", ["build"], ->

gulp.task "build", ->
runSeq "clean", [
"copy:socket:client"
"icons"
"logos"
"manifest"
Expand Down
7 changes: 0 additions & 7 deletions packages/extension/test/integration/background_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,6 @@ tab3 = {
}

describe "app/background", ->
before ->
@io = global.io
global.io = socket.client

beforeEach (done) ->
@httpSrv = http.createServer()
@server = socket.server(@httpSrv, {path: "/__socket.io"})
Expand All @@ -107,9 +103,6 @@ describe "app/background", ->
@server.close()
@httpSrv.close -> done()

after ->
global.io = @io

context ".connect", ->
it "can connect", (done) ->
@server.on "connection", -> done()
Expand Down
6 changes: 1 addition & 5 deletions packages/extension/test/spec_helper.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,9 @@ sinonChai = require("sinon-chai")
chai.use(sinonChai)

global.expect = chai.expect
global.io = {
connect: ->
return {on: ->}
}

beforeEach ->
@sandbox = sinon.sandbox.create()

afterEach ->
@sandbox.restore()
@sandbox.restore()
7 changes: 5 additions & 2 deletions packages/network/test/support/servers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ export interface AsyncServer {
function addDestroy(server: http.Server | https.Server) {
let connections = []

server.on('connection', function(conn) {
function trackConn(conn) {
connections.push(conn)

conn.on('close', () => {
connections = connections.filter(connection => connection !== conn)
})
})
}

server.on('connection', trackConn)
server.on('secureConnection', trackConn)

// @ts-ignore Property 'destroy' does not exist on type 'Server'.
server.destroy = function(cb) {
Expand Down
10 changes: 6 additions & 4 deletions packages/network/test/unit/agent_spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import Bluebird from 'bluebird'
import chai from 'chai'
import { EventEmitter } from 'events'
import http from 'http'
import https from 'https'
import net from 'net'
Expand Down Expand Up @@ -175,7 +174,8 @@ describe('lib/agent', function() {
return new Bluebird((resolve) => {
Io.client(`http://localhost:${HTTP_PORT}`, {
agent: this.agent,
transports: ['websocket']
transports: ['websocket'],
rejectUnauthorized: false
}).on('message', resolve)
})
.then(msg => {
Expand All @@ -191,7 +191,8 @@ describe('lib/agent', function() {
return new Bluebird((resolve) => {
Io.client(`https://localhost:${HTTPS_PORT}`, {
agent: this.agent,
transports: ['websocket']
transports: ['websocket'],
rejectUnauthorized: false
}).on('message', resolve)
})
.then(msg => {
Expand Down Expand Up @@ -357,7 +358,8 @@ describe('lib/agent', function() {
Io.client(`${testCase.protocol}://foo.bar.baz.invalid`, {
agent: <any>testCase.agent,
transports: ['websocket'],
timeout: 1
timeout: 1,
rejectUnauthorized: false
})
.on('message', reject)
.on('connect_error', resolve)
Expand Down
4 changes: 2 additions & 2 deletions packages/runner/lib/test-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const chai = require('chai')
const JSDOM = require('jsdom').JSDOM
const sinonChai = require('sinon-chai')
const $Cypress = require('@packages/driver')
const io = require('@packages/socket')
const { client } = require('@packages/socket')

// http://airbnb.io/enzyme/docs/guides/jsdom.html
const jsdom = new JSDOM('<!doctype html><html><body></body></html>')
Expand Down Expand Up @@ -52,6 +52,6 @@ class Runner {

global.Mocha = { Runnable, Runner }
$Cypress.create = () => {}
io.connect = () => {
client.connect = () => {
return { emit: () => {}, on: () => {} }
}
Loading