From 0046cc55129a37604ac3e47e8a84b1ed5a4d9181 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Wed, 8 Apr 2020 11:26:53 -0400 Subject: [PATCH] Ignore invalid Set-Cookie values (#6948) * add invalid header char repro * ignore tough-cookie/automation failures see https://github.com/cypress-io/cypress/issues/6890#issuecomment-609913483 * restore snapshot * Update packages/server/lib/request.coffee * Update packages/server/lib/request.coffee Co-Authored-By: Jennifer Shehane Co-authored-by: Jennifer Shehane --- .../__snapshots__/2_cookies_spec.coffee.js | 12 +++++++----- packages/server/lib/request.coffee | 18 ++++++++++++++---- packages/server/test/e2e/2_cookies_spec.coffee | 11 +++++++++++ .../integration/cookies_spec_baseurl.coffee | 12 ++++++++++++ 4 files changed, 44 insertions(+), 9 deletions(-) diff --git a/packages/server/__snapshots__/2_cookies_spec.coffee.js b/packages/server/__snapshots__/2_cookies_spec.coffee.js index b41056012b0b..39c46f5cdafa 100644 --- a/packages/server/__snapshots__/2_cookies_spec.coffee.js +++ b/packages/server/__snapshots__/2_cookies_spec.coffee.js @@ -33,6 +33,7 @@ exports['e2e cookies with baseurl'] = ` ✓ can set and clear cookie in a cy.visit ✓ can successfully send cookies as a Cookie header + ✓ ignores invalid set-cookie headers that contain control chars with Domain = superdomain ✓ is set properly with no redirects ✓ is set properly with redirects @@ -48,6 +49,7 @@ exports['e2e cookies with baseurl'] = ` ✓ can set cookies on lots of redirects, ending with same domain in a cy.request ✓ can successfully send cookies as a Cookie header + ✓ ignores invalid set-cookie headers that contain control chars with Domain = superdomain ✓ is set properly with no redirects ✓ is set properly with redirects @@ -63,14 +65,14 @@ exports['e2e cookies with baseurl'] = ` ✓ can set cookies on lots of redirects, ending with same domain - 30 passing + 32 passing (Results) ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ - │ Tests: 30 │ - │ Passing: 30 │ + │ Tests: 32 │ + │ Passing: 32 │ │ Failing: 0 │ │ Pending: 0 │ │ Skipped: 0 │ @@ -94,9 +96,9 @@ exports['e2e cookies with baseurl'] = ` Spec Tests Passing Failing Pending Skipped ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ - │ ✔ cookies_spec_baseurl.coffee XX:XX 30 30 - - - │ + │ ✔ cookies_spec_baseurl.coffee XX:XX 32 32 - - - │ └────────────────────────────────────────────────────────────────────────────────────────────────┘ - ✔ All specs passed! XX:XX 30 30 - - - + ✔ All specs passed! XX:XX 32 32 - - - ` diff --git a/packages/server/lib/request.coffee b/packages/server/lib/request.coffee index 422b24f46cea..c900cf6baca1 100644 --- a/packages/server/lib/request.coffee +++ b/packages/server/lib/request.coffee @@ -489,6 +489,12 @@ module.exports = (options = {}) -> debug('parsing cookie %o', { cyCookie, toughCookie: cookie }) + if not cookie + ## ignore invalid cookies (same as browser behavior) + ## https://github.com/cypress-io/cypress/issues/6890 + debug('tough-cookie failed to parse, ignoring') + return + cookie.name = cookie.key if not cookie.domain @@ -504,14 +510,18 @@ module.exports = (options = {}) -> if isFinite(expiry) cookie.expiry = expiry / 1000 + cookie.sameSite = convertSameSiteToughToExtension(cookie.sameSite, cyCookie) + cookie = _.pick(cookie, SERIALIZABLE_COOKIE_PROPS) - if expiry <= 0 - return automationFn('clear:cookie', cookie) + automationCmd = 'set:cookie' - cookie.sameSite = convertSameSiteToughToExtension(cookie.sameSite, cyCookie) + if expiry <= 0 + automationCmd = 'clear:cookie' - automationFn('set:cookie', cookie) + automationFn(automationCmd, cookie) + .catch (err) -> + debug('automation threw an error during cookie change %o', { automationCmd, cyCookie, cookie, err }) sendStream: (headers, automationFn, options = {}) -> _.defaults options, { diff --git a/packages/server/test/e2e/2_cookies_spec.coffee b/packages/server/test/e2e/2_cookies_spec.coffee index bcc6bee51aff..fc411774a6a3 100644 --- a/packages/server/test/e2e/2_cookies_spec.coffee +++ b/packages/server/test/e2e/2_cookies_spec.coffee @@ -105,6 +105,17 @@ onServer = (app) -> res.setHeader("Set-Cookie", header) res.type('html').end() + app.get "/invalidControlCharCookie", (req, res) -> + ## `http` lib throws an error if we use .setHeader to set this + res.connection.end(""" + HTTP/1.1 200 OK + Content-Type: text/html + Set-Cookie: ___utmvaFvuoaRv=TkE\u0001sCvZ; path=/; Max-Age=900 + Set-Cookie: _valid=true; path=/; Max-Age=900 + + foo + """) + haveRoot = !process.env.USE_HIGH_PORTS && process.geteuid() == 0 if not haveRoot diff --git a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/cookies_spec_baseurl.coffee b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/cookies_spec_baseurl.coffee index cbe39349b94f..a712c2597fe6 100644 --- a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/cookies_spec_baseurl.coffee +++ b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/cookies_spec_baseurl.coffee @@ -195,6 +195,18 @@ describe "cookies", -> c: 's:PtCc3lNiuqN0AtR9ffgKUnUsDzR5n_4B.qzFDJDvqx8PZNvmOkmcexDs7fRJLOel56Z8Ii6PL+Fo' }) + ## https://github.com/cypress-io/cypress/issues/6890 + it "ignores invalid set-cookie headers that contain control chars", -> + cy[cmd]("/invalidControlCharCookie") + + cy.request("/requestCookies") + .then (res) -> + return res.body + .then (cookies) -> + expect(cookies).to.deep.eq({ + _valid: 'true' + }) + context "with Domain = superdomain", -> requestCookiesUrl = "#{Cypress.config('baseUrl')}/requestCookies" setDomainCookieUrl = "#{Cypress.config('baseUrl')}/setDomainCookie?domain=#{setCookieDomain}"