From bb7252b29126ad7fdda3ef6f54eadd3327289a5f Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 16 Dec 2019 10:45:19 -0500 Subject: [PATCH] Remove strict cookie validation from cookie commands (#5957) * Remove strict cookie validation from cookie commands * fix test * wrap cookie automation errors with something a lil friendlier * show message and stack * fix test --- packages/driver/package.json | 1 - packages/driver/src/cy/commands/cookies.js | 52 ++++++------- .../driver/src/cypress/error_messages.coffee | 21 +++-- .../integration/commands/cookies_spec.coffee | 77 ++++--------------- 4 files changed, 50 insertions(+), 101 deletions(-) diff --git a/packages/driver/package.json b/packages/driver/package.json index 9da16c2d16..e237126e24 100644 --- a/packages/driver/package.json +++ b/packages/driver/package.json @@ -55,7 +55,6 @@ "parse-domain": "2.3.4", "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", diff --git a/packages/driver/src/cy/commands/cookies.js b/packages/driver/src/cy/commands/cookies.js index a74d82e0fe..6155272255 100644 --- a/packages/driver/src/cy/commands/cookies.js +++ b/packages/driver/src/cy/commands/cookies.js @@ -1,8 +1,5 @@ -/* global Cypress */ - const _ = require('lodash') const Promise = require('bluebird') -const cookieParser = require('strict-cookie-parser') const $utils = require('../../cypress/utils') const $Location = require('../../cypress/location') @@ -35,12 +32,6 @@ const mergeDefaults = function (obj) { return merge(obj) } -const validateCookieName = function (cmd, name, onFail) { - if (cookieParser.isCookieName(name) !== true) { - return Cypress.utils.throwErrByPath('cookies.invalid_name', { args: { cmd, name }, onFail }) - } -} - module.exports = function (Commands, Cypress, cy, state, config) { const automateCookies = function (event, obj = {}, log, timeout) { const automate = () => { @@ -87,6 +78,25 @@ module.exports = function (Commands, Cypress, cy, state, config) { }) } + const handleBackendError = (command, action, onFail) => { + return (err) => { + if (err.name === 'CypressError') { + throw err + } + + Cypress.utils.throwErrByPath('cookies.backend_error', { + args: { + action, + command, + browserDisplayName: Cypress.browser.displayName, + errMessage: err.message, + errStack: err.stack, + }, + onFail, + }) + } + } + // TODO: handle failure here somehow // maybe by tapping into the Cypress reset // stuff, or handling this in the runner itself? @@ -129,14 +139,13 @@ module.exports = function (Commands, Cypress, cy, state, config) { $utils.throwErrByPath('getCookie.invalid_argument', { onFail }) } - validateCookieName('getCookie', name, onFail) - return automateCookies('get:cookie', { name }, options._log, options.timeout) .then((resp) => { options.cookie = resp return resp }) + .catch(handleBackendError('getCookie', 'reading the requested cookie from', onFail)) }, getCookies (options = {}) { @@ -171,6 +180,7 @@ module.exports = function (Commands, Cypress, cy, state, config) { return resp }) + .catch(handleBackendError('getCookies', 'reading cookies from', options._log)) }, setCookie (name, value, userOptions = {}) { @@ -214,26 +224,12 @@ module.exports = function (Commands, Cypress, cy, state, config) { 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 }) - } - return automateCookies('set:cookie', cookie, options._log, options.timeout) .then((resp) => { options.cookie = resp return resp - }).catch((err) => { - return Cypress.utils.throwErrByPath('setCookie.backend_error', { - args: { - browserDisplayName: Cypress.browser.displayName, - errStack: err.stack, - }, - onFail, - }) - }) + }).catch(handleBackendError('setCookie', 'setting the requested cookie in', onFail)) }, clearCookie (name, options = {}) { @@ -271,8 +267,6 @@ module.exports = function (Commands, Cypress, cy, state, config) { $utils.throwErrByPath('clearCookie.invalid_argument', { onFail }) } - validateCookieName('clearCookie', name, onFail) - // TODO: prevent clearing a cypress namespace return automateCookies('clear:cookie', { name }, options._log, options.timeout) .then((resp) => { @@ -281,6 +275,7 @@ module.exports = function (Commands, Cypress, cy, state, config) { // null out the current subject return null }) + .catch(handleBackendError('clearCookie', 'clearing the requested cookie in', onFail)) }, clearCookies (options = {}) { @@ -322,6 +317,7 @@ module.exports = function (Commands, Cypress, cy, state, config) { err.message = err.message.replace('getCookies', 'clearCookies') throw err }) + .catch(handleBackendError('clearCookies', 'clearing cookies in', options._log)) }, }) } diff --git a/packages/driver/src/cypress/error_messages.coffee b/packages/driver/src/cypress/error_messages.coffee index 35a8363610..a82d12c79f 100644 --- a/packages/driver/src/cypress/error_messages.coffee +++ b/packages/driver/src/cypress/error_messages.coffee @@ -150,7 +150,12 @@ 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}}`" + backend_error: """ + #{cmd('{{command}}')} had an unexpected error {{action}} {{browserDisplayName}}. + + {{errMessage}} + {{errStack}} + """ removed_method: """ The Cypress.Cookies.{{method}}() method has been removed. @@ -412,10 +417,10 @@ module.exports = { invalid_prop_name_arg: "#{cmd('{{cmd}}')} only accepts a string or a number as the {{identifier}}Name argument." null_or_undefined_property_name: "#{cmd('{{cmd}}')} expects the {{identifier}}Name argument to have a value." invalid_options_arg: "#{cmd('{{cmd}}')} only accepts an object as the options argument." - invalid_num_of_args: - """ - #{cmd('{{cmd}}')} does not accept additional arguments. - If you want to invoke a function with arguments, use cy.invoke(). + invalid_num_of_args: + """ + #{cmd('{{cmd}}')} does not accept additional arguments. + If you want to invoke a function with arguments, use cy.invoke(). """ timed_out: """ @@ -823,13 +828,7 @@ 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." diff --git a/packages/driver/test/cypress/integration/commands/cookies_spec.coffee b/packages/driver/test/cypress/integration/commands/cookies_spec.coffee index 98eebfe709..2622bb21de 100644 --- a/packages/driver/test/cypress/integration/commands/cookies_spec.coffee +++ b/packages/driver/test/cypress/integration/commands/cookies_spec.coffee @@ -116,9 +116,9 @@ describe "src/cy/commands/cookies", -> 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.contain "cy.getCookies() had an unexpected error reading cookies from #{Cypress.browser.displayName}." + expect(lastLog.get("error").message).to.contain "some err message" + expect(lastLog.get("error").message).to.contain error.stack done() cy.getCookies() @@ -258,9 +258,9 @@ describe "src/cy/commands/cookies", -> 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.contain "cy.getCookie() had an unexpected error reading the requested cookie from #{Cypress.browser.displayName}." + expect(lastLog.get("error").message).to.contain "some err message" + expect(lastLog.get("error").message).to.contain error.stack done() cy.getCookie("foo") @@ -292,15 +292,6 @@ 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) => @@ -498,42 +489,15 @@ 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(errStub).to.be.calledOnce 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") @@ -647,9 +611,9 @@ describe "src/cy/commands/cookies", -> 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.contain "cy.clearCookie() had an unexpected error clearing the requested cookie in #{Cypress.browser.displayName}." + expect(lastLog.get("error").message).to.contain "some err message" + expect(lastLog.get("error").message).to.contain error.stack done() cy.clearCookie("foo") @@ -681,15 +645,6 @@ 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) => @@ -882,9 +837,9 @@ describe "src/cy/commands/cookies", -> 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 err.stack + expect(lastLog.get("error").message).to.contain "cy.clearCookies() had an unexpected error clearing cookies in #{Cypress.browser.displayName}." + expect(lastLog.get("error").message).to.contain "some err message" + expect(lastLog.get("error").message).to.contain error.stack expect(lastLog.get("error")).to.eq(err) done() @@ -921,9 +876,9 @@ describe "src/cy/commands/cookies", -> 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.contain "cy.clearCookies() had an unexpected error clearing cookies in #{Cypress.browser.displayName}." + expect(lastLog.get("error").message).to.contain "some err message" + expect(lastLog.get("error").message).to.contain error.stack expect(lastLog.get("error")).to.eq(err) done()