Fix cy.clearCookie, fix HTTP redirect behavior, fix cy.visit HTTPS baseurl behavior

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

@@ -18,13 +18,8 @@ mergeDefaults = (obj) ->
   ## we always want to be able to see and influence cookies
   ## on our superdomain
   { superDomain } = $Location.create(window.location.href)
-  # { hostname } = $Location.create(window.location.href)
 
   merge = (o) ->
-    ## we are hostOnly if we dont have an
-    ## explicit domain
-    # o.hostOnly = !o.domain
-
     ## and if the user did not provide a domain
     ## then we know to set the default to be origin
     _.defaults o, {domain: superDomain}
@@ -63,8 +58,8 @@ module.exports = (Commands, Cypress, cy, state, config) ->
           }
         }
 
-  getAndClear = (log, timeout) ->
-    automateCookies("get:cookies", {}, log, timeout)
+  getAndClear = (log, timeout, options = {}) ->
+    automateCookies("get:cookies", options, log, timeout)
     .then (resp) =>
       ## bail early if we got no cookies!
       return resp if resp and resp.length is 0
@@ -137,7 +132,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
             obj
         })
 
-      automateCookies("get:cookies", {}, options._log, options.timeout)
+      automateCookies("get:cookies", _.pick(options, 'domain'), options._log, options.timeout)
       .then (resp) ->
         options.cookies = resp
 
@@ -256,7 +251,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
             obj
         })
 
-      getAndClear(options._log, options.timeout)
+      getAndClear(options._log, options.timeout, { domain: options.domain })
       .then (resp) ->
         options.cookies = resp
 

packages/server/__snapshots__/2_cookies_spec.coffee.js

@@ -1,4 +1,4 @@
-exports['e2e cookies / passes'] = `
+exports['e2e cookies with baseurl'] = `
 
 ====================================================================================================
 
@@ -7,14 +7,92 @@ exports['e2e cookies / passes'] = `
   ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
   │ Cypress:    1.2.3                                                                              │
   │ Browser:    FooBrowser 88                                                                      │
-  │ Specs:      1 found (cookies_spec.coffee)                                                      │
-  │ Searched:   cypress/integration/cookies_spec.coffee                                            │
+  │ Specs:      1 found (cookies_spec_baseurl.coffee)                                              │
+  │ Searched:   cypress/integration/cookies_spec_baseurl.coffee                                    │
   └────────────────────────────────────────────────────────────────────────────────────────────────┘
 
 
 ────────────────────────────────────────────────────────────────────────────────────────────────────
 
-  Running:  cookies_spec.coffee                                                             (1 of 1)
+  Running:  cookies_spec_baseurl.coffee                                                     (1 of 1)
+
+
+  cookies
+    with whitelist
+      ✓ can get all cookies
+      ✓ resets cookies between tests correctly
+      ✓ should be only two left now
+      ✓ handles undefined cookies
+    without whitelist
+      ✓ sends set cookies to path
+      ✓ handles expired cookies secure
+      ✓ issue: #224 sets expired cookies between redirects
+      ✓ issue: #1321 failing to set or parse cookie
+      ✓ 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
+      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
+
+
+  14 passing
+
+
+  (Results)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Tests:        14                                                                               │
+  │ Passing:      14                                                                               │
+  │ Failing:      0                                                                                │
+  │ Pending:      0                                                                                │
+  │ Skipped:      0                                                                                │
+  │ Screenshots:  0                                                                                │
+  │ Video:        true                                                                             │
+  │ Duration:     X seconds                                                                        │
+  │ Spec Ran:     cookies_spec_baseurl.coffee                                                      │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+  (Video)
+
+  -  Started processing:  Compressing to 32 CRF                                                     
+  -  Finished processing: /XXX/XXX/XXX/cypress/videos/cookies_spec_baseurl.coffee.mp4     (X second)
+
+
+====================================================================================================
+
+  (Run Finished)
+
+
+       Spec                                              Tests  Passing  Failing  Pending  Skipped  
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ ✔  cookies_spec_baseurl.coffee              XX:XX       14       14        -        -        - │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+    ✔  All specs passed!                        XX:XX       14       14        -        -        -  
+
+
+`
+
+exports['e2e cookies with no baseurl'] = `
+
+====================================================================================================
+
+  (Run Starting)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Cypress:    1.2.3                                                                              │
+  │ Browser:    FooBrowser 88                                                                      │
+  │ Specs:      1 found (cookies_spec_no_baseurl.coffee)                                           │
+  │ Searched:   cypress/integration/cookies_spec_no_baseurl.coffee                                 │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+────────────────────────────────────────────────────────────────────────────────────────────────────
+
+  Running:  cookies_spec_no_baseurl.coffee                                                  (1 of 1)
 
 
   cookies
@@ -45,14 +123,15 @@ exports['e2e cookies / passes'] = `
   │ Screenshots:  0                                                                                │
   │ Video:        true                                                                             │
   │ Duration:     X seconds                                                                        │
-  │ Spec Ran:     cookies_spec.coffee                                                              │
+  │ Spec Ran:     cookies_spec_no_baseurl.coffee                                                   │
   └────────────────────────────────────────────────────────────────────────────────────────────────┘
 
 
   (Video)
 
   -  Started processing:  Compressing to 32 CRF                                                     
-  -  Finished processing: /XXX/XXX/XXX/cypress/videos/cookies_spec.coffee.mp4             (X second)
+  -  Finished processing: /XXX/XXX/XXX/cypress/videos/cookies_spec_no_baseurl.coffee.     (X second)
+                          mp4                                                                       
 
 
 ====================================================================================================
@@ -62,7 +141,7 @@ exports['e2e cookies / passes'] = `
 
        Spec                                              Tests  Passing  Failing  Pending  Skipped  
   ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
-  │ ✔  cookies_spec.coffee                      XX:XX        9        9        -        -        - │
+  │ ✔  cookies_spec_no_baseurl.coffee           XX:XX        9        9        -        -        - │
   └────────────────────────────────────────────────────────────────────────────────────────────────┘
     ✔  All specs passed!                        XX:XX        9        9        -        -        -  
 

packages/server/__snapshots__/request_spec.coffee.js

@@ -2,33 +2,26 @@ exports['lib/request #sendPromise followRedirect gets + attaches the cookies at
   "setCalls": [
     {
       "currentUrl": "http://localhost:1234/",
-      "setCookie": "foo=bar"
+      "setCookie": "one=1"
     },
     {
-      "currentUrl": "http://localhost:1234/B",
-      "setCookie": "bar=baz"
+      "currentUrl": "http://localhost:1234/second",
+      "setCookie": "two=2"
     },
     {
-      "currentUrl": "http://localhost:1234/B",
-      "setCookie": "foo=bar"
-    },
-    {
-      "currentUrl": "http://localhost:1234/",
-      "setCookie": "quuz=quux"
+      "currentUrl": "http://localhost:1234/third",
+      "setCookie": "three=3"
     }
   ],
   "getCalls": [
     {
       "newUrl": "http://localhost:1234/"
     },
     {
-      "newUrl": "http://localhost:1234/B"
-    },
-    {
-      "newUrl": "http://localhost:1234/B"
+      "newUrl": "http://localhost:1234/second"
     },
     {
-      "newUrl": "http://localhost:1234/B"
+      "newUrl": "http://localhost:1234/third"
     }
   ]
 }
@@ -37,29 +30,22 @@ exports['lib/request #sendStream gets + attaches the cookies at each redirect 1'
   "setCalls": [
     {
       "currentUrl": "http://localhost:1234/",
-      "setCookie": "foo=bar"
+      "setCookie": "one=1"
     },
     {
-      "currentUrl": "http://localhost:1234/B",
-      "setCookie": "bar=baz"
-    },
-    {
-      "currentUrl": "http://localhost:1234/B",
-      "setCookie": "foo=bar"
+      "currentUrl": "http://localhost:1234/second",
+      "setCookie": "two=2"
     }
   ],
   "getCalls": [
     {
       "newUrl": "http://localhost:1234/"
     },
     {
-      "newUrl": "http://localhost:1234/B"
-    },
-    {
-      "newUrl": "http://localhost:1234/B"
+      "newUrl": "http://localhost:1234/second"
     },
     {
-      "newUrl": "http://localhost:1234/B"
+      "newUrl": "http://localhost:1234/third"
     }
   ]
 }

packages/server/lib/browsers/cdp_automation.ts

@@ -53,17 +53,19 @@ export const CdpAutomation = (sendDebuggerCommandFn: SendDebuggerCommand) => {
   }
 
   const normalizeSetCookieProps = (cookie: CyCookie): cdp.Network.SetCookieRequest => {
+    // this logic forms a SetCookie request that will be received by Chrome
+    // see MakeCookieFromProtocolValues for information on how this cookie data will be parsed
+    // @see https://cs.chromium.org/chromium/src/content/browser/devtools/protocol/network_handler.cc?l=246&rcl=786a9194459684dc7a6fded9cabfc0c9b9b37174
+
     _.defaults(cookie, {
       name: '',
       value: '',
     })
 
-    // this logic forms a SetCookie request that will be received by Chrome
-    // see MakeCookieFromProtocolValues for information on how this cookie data will be parsed
-    // @see https://cs.chromium.org/chromium/src/content/browser/devtools/protocol/network_handler.cc?l=246&rcl=786a9194459684dc7a6fded9cabfc0c9b9b37174
-
     // @ts-ignore
     cookie.expires = cookie.expirationDate
+
+    // without this logic, a cookie being set on 'foo.com' will only be set for 'foo.com', not other subdomains
     if (!cookie.hostOnly && cookie.domain[0] !== '.') {
       let parsedDomain = cors.parseDomain(cookie.domain)
 
@@ -135,9 +137,15 @@ export const CdpAutomation = (sendDebuggerCommandFn: SendDebuggerCommand) => {
         })
       case 'clear:cookie':
         return getCookie(data)
-        // so we can resolve with the value of the removed cookie
-        .tap((_cookieToBeCleared) => {
-          return sendDebuggerCommandFn('Network.deleteCookies', data)
+        // tap, so we can resolve with the value of the removed cookie
+        // also, getting the cookie via CDP first will ensure that we send a cookie `domain` to CDP
+        // that matches the cookie domain that is really stored
+        .tap((cookieToBeCleared) => {
+          if (!cookieToBeCleared) {
+            return
+          }
+
+          return sendDebuggerCommandFn('Network.deleteCookies', _.pick(cookieToBeCleared, 'name', 'domain'))
         })
       case 'is:automation:client:connected':
         return true

packages/server/lib/browsers/electron.coffee

@@ -25,7 +25,15 @@ getAutomation = (win) ->
   sendDebuggerCommand = (message, data) ->
     ## wrap in bluebird
     tryToCall win, Promise.method ->
+      debug('debugger: sending %s with params %o', message, data)
       win.webContents.debugger.sendCommand(message, data)
+    .tap (res) ->
+      if debug.enabled && res.data && res.data.length > 100
+        res = _.clone(res)
+        res.data = res.data.slice(0, 100) + ' [truncated]'
+      debug('debugger: received response to %s: %o', message, res)
+    .tapCatch (err) ->
+      debug('debugger: received error on %s: %o', messsage, err)
 
   CdpAutomation(sendDebuggerCommand)
 

packages/server/lib/request.coffee

@@ -435,7 +435,7 @@ module.exports = (options = {}) ->
         cookies = [cookies]
 
       parsedUrl = url.parse(resUrl)
-      debug('setting cookies on browser %o', { url: parsedUrl, cookies })
+      debug('setting cookies on browser %o', { url: parsedUrl.href, cookies })
 
       Promise.map cookies, (cookie) ->
         cookie = tough.Cookie.parse(cookie, { loose: true })
@@ -478,24 +478,21 @@ module.exports = (options = {}) ->
       currentUrl = options.url
 
       options.followRedirect = (incomingRes) ->
-        req = @
+        if followRedirect and not followRedirect(incomingRes)
+          return false
 
         newUrl = url.resolve(currentUrl, incomingRes.headers.location)
 
         ## and when we know we should follow the redirect
         ## we need to override the init method and
         ## first set the received cookies on the browser
         ## and then grab the cookies for the new url
-        req.init = _.wrap req.init, (orig, opts) =>
-          options.onBeforeReqInit ->
-            self.setCookiesOnBrowser(incomingRes, currentUrl, automationFn)
-            .then (cookies) ->
-              self.setRequestCookieHeader(req, newUrl, automationFn)
-            .then (cookieHeader) ->
-              currentUrl = newUrl
-              orig.call(req, opts)
-
-        followRedirect.call(req, incomingRes)
+        self.setCookiesOnBrowser(incomingRes, currentUrl, automationFn)
+        .then (cookies) =>
+          self.setRequestCookieHeader(@, newUrl, automationFn)
+        .then =>
+          currentUrl = newUrl
+          true
 
       @setRequestCookieHeader(options, options.url, automationFn)
       .then =>
@@ -565,24 +562,16 @@ module.exports = (options = {}) ->
 
             push(incomingRes)
 
-            req = @
-
             ## and when we know we should follow the redirect
             ## we need to override the init method and
             ## first set the new cookies on the browser
             ## and then grab the cookies for the new url
-            req.init = _.wrap req.init, (orig, opts) =>
-              self.setCookiesOnBrowser(incomingRes, currentUrl, automationFn)
-              .then ->
-                self.setRequestCookieHeader(req, newUrl, automationFn)
-              .then ->
-                currentUrl = newUrl
-                orig.call(req, opts)
-
-            ## cause the redirect to happen
-            ## but swallow up the incomingRes
-            ## so we can build an array of responses
-            return true
+            self.setCookiesOnBrowser(incomingRes, currentUrl, automationFn)
+            .then =>
+              self.setRequestCookieHeader(@, newUrl, automationFn)
+            .then =>
+              currentUrl = newUrl
+              true
 
         @create(options, true)
         .then(@normalizeResponse.bind(@, push))
@@ -602,7 +591,7 @@ module.exports = (options = {}) ->
             ## the current url
             resp.redirectedToUrl = url.resolve(options.url, loc)
 
-          @setCookiesOnBrowser(resp, options.url, automationFn)
+          @setCookiesOnBrowser(resp, currentUrl, automationFn)
           .return(resp)
 
       if c = options.cookies