Control Chrome cookies through CDP (#5297)

* try connecting to chrome remote interface

* linting

* print CRI targets for better debugging

* linting

* load empty tab first when connecting to CRI

* first load blank page, then navigate

* Page.navigate is working

* linting

* remove title

* add mocha banner

* more banners

* update some server unit tests

* update integration test

* document how to run single driver spec file

* set the focus back on the page before navigating from blank chrome tab

* update server unit test

* do not store Chrome remote interface reference for now

* record video of the Chrome tab using screencast API

* use dynamic port to connect to Chrome remote interface

* update unit tests

* refactoring

* wrap chrome remote interface in our interface, limit access to send

* resolved merge

* fix reference

* passing run unit spec

* stub canary search for CI to pass

* add build step to packages/server

* update chrome spec

* do not build js on install for server

* updated spec snapshots

* update 6_visit_spec snapshot

* update snapshot for 6_web_security_spec

* update snapshot for 3_plugins_spec

* update snapshot for 3_user_agent_spec

* update snapshot for 5_stdout_spec

* update snapshot for 2_browser_path_spec

* do not git ignore js files, add note why

* update several snapshots with video on Chrome

* update visit performance snapshot

* add chrome-remote-interface dependency

* cleanup coffeescript conversion to JS, fix some type errors, make parallel override clearer

* fix failing tests

* Fix snapshot - now we do record in Chrome, so warning message is no longer there.

* remove chrome warnings about not recording from snapshot

* Remove performance tests from 6_visit_spec snapshot

* Remove error from snapshot

* Add newline back to cy_visit_performance_spec snapshot

* Use CDP to control Chrome cookies + screenshot

* Add devtools types

* Cleanup

* Cleanup

* Add guards for minimum CDP version

* Fix failing tests

* Split cdp_automation_spec out of electron_spec

* Move takeScreenshot to cri-client

* Navigate to about:blank

* look for blank page url

* add note about avoiding Prettier

* disable prettier a little more

* call chrome remote interface close after each spec

* return promise when starting screencast

* update failing unit tests, add cri client close test

* update integration test

* Add verbose debug statements to cri-client

* Use connect.createRetryingSocket for CDP connection

* record video from chrome browsers

* add method for validating browser family

* update e2e spec snapshot

* update 4_request_spec snapshot

* update snapshot for spec 1_commands_outside_of_test_spec

* update snapshot for 3_plugins_spec

* update snapshot for spec 3_user_agent_spec

* try: Always log video capturing errors

* update snapshot for 2_browser_path_spec

* update snapshot for 2_cookies_spec

* better browser family test

* update snapshot for 5_stdout_spec

* update snapshot for 5_subdomain_spec

* Add protocol_spec tests

* do not capture video during performance test

* Add test for VIDEO_POST_PROCESSING_FAILED warning

* Add basic cookie validation in cy.setCookie

* Update cdp_automation to throw on Network.setCookie failure code

* Update tests 🎉

* Update snapshot

* Fix test

* Remove redundant logs, cleanup

* Add cri-client_spec, fix some small bugs, improve errors

* Update dep

* use client.on to register screencast callback

* use isCookieName

* strict-cookie-parser@3.1.0

* cleanup prettier, extract some functions, switch to browser.family

* moar cleanup and fixes

* add logging to the cri-client so we can see every message sent + received to the cdp protocol

* bump bluebird to 3.7.0 for .tapCatch addition

* Fix unit tests

* WIP: update e2e test to ensure that duration of the video matches what we expect

* Test duration of recorded video

* Run 6_video_compression in chrome + electron

* Cleanup

* finish ffmpeg duration verification

* Update 8_reporters_spec snapshot

* Fix cri-client test

* Update CRI close logic to monkey-patch browser.kill

* add isBrowserFamily back

* make it possible for remote-debugging-port to get overridden

* Make CDP timeout 5s; add unit, e2e tests for CDP failure; add user-friendly CDP failure error

* Update tests

* Use CYPRESS_REMOTE_DEBUGGING_PORT to set CDP port; update CDP error message

* Change new Buffer to Buffer.from

* Apply name validation on all cookie commands

* Just throw on Chrome start if the CDP version is < 1.3

* Fix cypress_spec

* Use CDP to set resolution + scale factor in Chrome e2e

* Revert "Use CDP to set resolution + scale factor in Chrome e2e"

This reverts commit a1b86d9.

* use CYPRESS_FORCE_BROWSER_SCALE to force standard resolution

* don't do --window-size --kiosk

* Use CDP to set resolution + scale factor in Chrome e2e

* Revert "use CYPRESS_FORCE_BROWSER_SCALE to force standard resolution"

This reverts commit 22c5e78.

* Use Page.captureScreenshot for Electron + Chrome, reduce logic

* Use before() task to force device metrics in Chrome

* Fix protocol_spec

* Update 7_record_spec to allow for before() hook

* Update 6_task_spec snapshot

* Appease eslint

* Update hooks in 5_spec_isolation snapshot

* some general promisification and cleanup

* feedback on pluginsfile

* cdp_automation feedback

* chrome.coff feedback

* feedback

* run e2e tests on port 4466, ensure no e2e test ever runs on 5566 to prevent conflicting with debugger port

* accept new 'remote:debugger:protocol' automation command to control device metrics overrides

* update web security e2e to run on electron + chrome

* run web security tests in electorn, disable context isolation

* pass disable-site-isolation-trials to Electron so webSecurity works

* Fix errors in e2e tests caused by extra log item

* fix cri-client unit tests

* fancy arrows in log message


Co-authored-by: Gleb Bahmutov <gleb.bahmutov@gmail.com>
Co-authored-by: Brian Mann <brian.mann86@gmail.com>
Co-authored-by: Jennifer Shehane <jennifer@cypress.io>

packages/driver/package.json

@@ -55,6 +55,7 @@
     "parse-domain": "2.0.0",
     "setimmediate": "1.0.5",
     "sinon": "3.3.0",
+    "strict-cookie-parser": "3.1.0",
     "text-mask-addons": "3.8.0",
     "underscore": "1.9.1",
     "underscore.string": "3.3.5",

packages/driver/src/cy/commands/cookies.coffee

@@ -1,4 +1,5 @@
 _ = require("lodash")
+cookieParser = require("strict-cookie-parser")
 Promise = require("bluebird")
 
 $dom = require("../../dom")
@@ -33,6 +34,10 @@ mergeDefaults = (obj) ->
   else
     merge(obj)
 
+validateCookieName = (cmd, name, onFail) ->
+  if cookieParser.isCookieName(name) isnt true
+    Cypress.utils.throwErrByPath("cookies.invalid_name", { args: { cmd, name }, onFail })
+
 module.exports = (Commands, Cypress, cy, state, config) ->
   automateCookies = (event, obj = {}, log, timeout) ->
     automate = ->
@@ -99,8 +104,12 @@ module.exports = (Commands, Cypress, cy, state, config) ->
             obj
         })
 
+      onFail = options._log
+
       if not _.isString(name)
-        $utils.throwErrByPath("getCookie.invalid_argument", { onFail: options._log })
+        $utils.throwErrByPath("getCookie.invalid_argument", { onFail })
+
+      validateCookieName('getCookie', name, onFail)
 
       automateCookies("get:cookie", {name: name}, options._log, options.timeout)
       .then (resp) ->
@@ -161,14 +170,29 @@ module.exports = (Commands, Cypress, cy, state, config) ->
             obj
         })
 
+      onFail = options._log
+
       if not _.isString(name) or not _.isString(value)
-        $utils.throwErrByPath("setCookie.invalid_arguments", { onFail: options._log })
+        Cypress.utils.throwErrByPath("setCookie.invalid_arguments", { onFail })
+
+      validateCookieName('setCookie', name, onFail)
+
+      if cookieParser.parseCookieValue(value) == null
+        Cypress.utils.throwErrByPath("setCookie.invalid_value", { args: { value }, onFail })
 
       automateCookies("set:cookie", cookie, options._log, options.timeout)
       .then (resp) ->
         options.cookie = resp
 
         return resp
+      .catch (err) ->
+        Cypress.utils.throwErrByPath("setCookie.backend_error", {
+          args: {
+            browserDisplayName: Cypress.browser.displayName,
+            errStack: err.stack
+          }
+          onFail
+        })
 
     clearCookie: (name, options = {}) ->
       _.defaults options, {
@@ -193,8 +217,12 @@ module.exports = (Commands, Cypress, cy, state, config) ->
             obj
         })
 
+      onFail = options._log
+
       if not _.isString(name)
-        $utils.throwErrByPath("clearCookie.invalid_argument", { onFail: options._log })
+        $utils.throwErrByPath("clearCookie.invalid_argument", { onFail })
+
+      validateCookieName('clearCookie', name, onFail)
 
       ## TODO: prevent clearing a cypress namespace
       automateCookies("clear:cookie", {name: name}, options._log, options.timeout)

packages/driver/src/cypress/error_messages.coffee

@@ -141,6 +141,7 @@ module.exports = {
     length_option: "#{cmd('contains')} cannot be passed a length option because it will only ever return 1 element."
 
   cookies:
+    invalid_name: "#{cmd('{{cmd}}')} must be passed an RFC-6265-compliant cookie name. You passed:\n\n`{{name}}`"
     removed_method: """
       The Cypress.Cookies.{{method}}() method has been removed.
 
@@ -812,7 +813,13 @@ module.exports = {
     unavailable: "The XHR server is unavailable or missing. This should never happen and likely is a bug. Open an issue if you see this message."
 
   setCookie:
+    backend_error: """
+    #{cmd('setCookie')} had an unexpected error setting the requested cookie in {{browserDisplayName}}.
+
+    {{errStack}}
+    """
     invalid_arguments: "#{cmd('setCookie')} must be passed two string arguments for name and value."
+    invalid_value: "#{cmd('setCookie')} must be passed an RFC-6265-compliant cookie value. You passed:\n\n`{{value}}`"
 
   spread:
     invalid_type: "#{cmd('spread')} requires the existing subject be array-like."

packages/driver/test/cypress/integration/commands/cookies_spec.coffee

@@ -292,6 +292,15 @@ describe "src/cy/commands/cookies", ->
 
         cy.getCookie(123)
 
+      it "throws an error if the cookie name is invalid", (done) ->
+        cy.on "fail", (err) =>
+          expect(err.message).to.include("cy.getCookie() must be passed an RFC-6265-compliant cookie name.")
+          expect(err.message).to.include('You passed:\n\n`m=m`')
+
+          done()
+
+        cy.getCookie("m=m")
+
     describe ".log", ->
       beforeEach ->
         cy.on "log:added", (attrs, log) =>
@@ -427,17 +436,16 @@ describe "src/cy/commands/cookies", ->
       it "logs once on error", (done) ->
         error = new Error("some err message")
         error.name = "foo"
-        error.stack = "stack"
 
         Cypress.automation.rejects(error)
 
         cy.on "fail", (err) =>
           lastLog = @lastLog
 
           expect(@logs.length).to.eq(1)
-          expect(lastLog.get("error").message).to.eq "some err message"
-          expect(lastLog.get("error").name).to.eq "foo"
-          expect(lastLog.get("error").stack).to.eq error.stack
+          expect(lastLog.get("error").message).to.include "some err message"
+          expect(lastLog.get("error").name).to.eq "CypressError"
+          expect(lastLog.get("error").stack).to.include error.stack
           done()
 
         cy.setCookie("foo", "bar")
@@ -453,7 +461,7 @@ describe "src/cy/commands/cookies", ->
           expect(lastLog.get("state")).to.eq("failed")
           expect(lastLog.get("name")).to.eq("setCookie")
           expect(lastLog.get("message")).to.eq("foo, bar")
-          expect(err.message).to.eq("cy.setCookie() timed out waiting '50ms' to complete.")
+          expect(err.message).to.include("cy.setCookie() timed out waiting '50ms' to complete.")
           done()
 
         cy.setCookie("foo", "bar", {timeout: 50})
@@ -480,6 +488,46 @@ describe "src/cy/commands/cookies", ->
 
         cy.setCookie("foo", 123)
 
+      context "when setting an invalid cookie", ->
+        it "throws an error if the cookie name is invalid", (done) ->
+          cy.on "fail", (err) =>
+            expect(err.message).to.include("cy.setCookie() must be passed an RFC-6265-compliant cookie name.")
+            expect(err.message).to.include('You passed:\n\n`m=m`')
+
+            done()
+
+          ## cookie names may not contain =
+          ## https://stackoverflow.com/a/6109881/3474615
+          cy.setCookie("m=m", "foo")
+
+        it "throws an error if the cookie value is invalid", (done) ->
+          cy.on "fail", (err) =>
+            expect(err.message).to.include('must be passed an RFC-6265-compliant cookie value.')
+            expect(err.message).to.include('You passed:\n\n` bar`')
+
+            done()
+
+          ## cookies may not contain unquoted whitespace
+          cy.setCookie("foo", " bar")
+
+        it "throws an error if the backend responds with an error", (done) ->
+          cy.on "fail", (err) =>
+            expect(skipErrStub).to.be.calledOnce
+            expect(errStub).to.be.calledTwice
+            expect(err.message).to.contain('unexpected error setting the requested cookie')
+            done()
+
+          errStub = cy.stub(Cypress.utils, "throwErrByPath")
+          errStub.callThrough()
+
+          ## stub cookie validation so this invalid cookie can make it to the backend
+          skipErrStub = errStub
+          .withArgs("setCookie.invalid_value")
+          .returns()
+
+          ## browser backend should yell since this is invalid
+          cy.setCookie("foo", " bar")
+
     describe ".log", ->
       beforeEach ->
         cy.on "log:added", (attrs, log) =>
@@ -624,6 +672,15 @@ describe "src/cy/commands/cookies", ->
 
         cy.clearCookie(123)
 
+      it "throws an error if the cookie name is invalid", (done) ->
+        cy.on "fail", (err) =>
+          expect(err.message).to.include("cy.clearCookie() must be passed an RFC-6265-compliant cookie name.")
+          expect(err.message).to.include('You passed:\n\n`m=m`')
+
+          done()
+
+        cy.clearCookie("m=m")
+
     describe ".log", ->
       beforeEach ->
         cy.on "log:added", (attrs, log) =>