Allow circular structures to be sent over the websocket, make it an e…

…rror to send circular request bodies (#4407) * use a parser that supports circular json * update tests to work with new socketio version * add error message when users supply circular body to visit/request * show the path of the circular reference detected * Revert "use a parser that supports circular json" This reverts commit c052f44. * add failing driver and server tests for circular objs over websocket * use a parser that supports circular json * add has-binary2 patch that enables circular objects to be inspected * update socket spec * rejectUnauthorized: false * use @packages/socket instead of copying client source * prevent false positive * use commit hash for socket.io-circular-parser * cleanup bundling of socket.io for node + browser - keep the interfaces identical - browser simply has less properties than the node variant * properly import client + circularParser from socket package * @cypress/what-is-circular * dont require the extension, it causes gulp to hang * fix runner tests hanging Co-authored-by: Brian Mann <brian.mann86@gmail.com>
cypress-io · Jun 13, 2019 · d185a74 · d185a74
1 parent 4bd3cf2
commit d185a74
Show file tree

Hide file tree

Showing 25 changed files with 279 additions and 75 deletions.
diff --git a/packages/driver/package.json b/packages/driver/package.json
@@ -63,6 +63,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"
   }
 }
diff --git a/packages/driver/src/cy/commands/navigation.coffee b/packages/driver/src/cy/commands/navigation.coffee
@@ -1,4 +1,5 @@
 _ = require("lodash")
+whatIsCircular = require("@cypress/what-is-circular")
 moment = require("moment")
 UrlParse = require("url-parse")
 Promise = require("bluebird")
@@ -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
 

diff --git a/packages/driver/src/cy/commands/request.coffee b/packages/driver/src/cy/commands/request.coffee
@@ -1,4 +1,5 @@
 _ = require("lodash")
+whatIsCircular = require("@cypress/what-is-circular")
 Promise = require("bluebird")
 
 $utils = require("../../cypress/utils")
@@ -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)

diff --git a/packages/driver/src/cypress/error_messages.coffee b/packages/driver/src/cypress/error_messages.coffee
@@ -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 }.
 
@@ -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 }.
 

diff --git a/packages/driver/test/cypress/integration/commands/navigation_spec.coffee b/packages/driver/test/cypress/integration/commands/navigation_spec.coffee
@@ -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")

diff --git a/packages/driver/test/cypress/integration/commands/request_spec.coffee b/packages/driver/test/cypress/integration/commands/request_spec.coffee
@@ -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")

diff --git a/packages/driver/test/cypress/integration/cypress/cypress_spec.coffee b/packages/driver/test/cypress/integration/cypress/cypress_spec.coffee
@@ -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
@@ -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()
diff --git a/packages/extension/app/background.coffee b/packages/extension/app/background.coffee
@@ -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"
@@ -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)
@@ -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://"

diff --git a/packages/extension/app/manifest.json b/packages/extension/app/manifest.json
@@ -24,7 +24,6 @@
   },
   "background": {
     "scripts": [
-      "socket.io.js",
       "background.js"
     ]
   },

diff --git a/packages/extension/gulpfile.coffee b/packages/extension/gulpfile.coffee
@@ -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")
@@ -71,7 +66,6 @@ gulp.task "watch", ["build"], ->
 
 gulp.task "build", ->
   runSeq "clean", [
-    "copy:socket:client"
     "icons"
     "logos"
     "manifest"

diff --git a/packages/extension/test/integration/background_spec.coffee b/packages/extension/test/integration/background_spec.coffee
@@ -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"})
@@ -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()

diff --git a/packages/extension/test/spec_helper.coffee b/packages/extension/test/spec_helper.coffee
@@ -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()
diff --git a/packages/network/test/support/servers.ts b/packages/network/test/support/servers.ts
@@ -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) {

diff --git a/packages/network/test/unit/agent_spec.ts b/packages/network/test/unit/agent_spec.ts
@@ -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'
@@ -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 => {
@@ -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 => {
@@ -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)

diff --git a/packages/runner/lib/test-setup.js b/packages/runner/lib/test-setup.js
@@ -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>')
@@ -52,6 +52,6 @@ class Runner {
 
 global.Mocha = { Runnable, Runner }
 $Cypress.create = () => {}
-io.connect = () => {
+client.connect = () => {
   return { emit: () => {}, on: () => {} }
 }