Issue 5656 Fix validation of cookie domain

[ghost]

• Closes Cypress incorrectly validates domain of cookies in 3.5.0 #5656

User facing changelog

• Fixed a regression in 3.5.0 where cookies set in a redirect on a cy.visit or in a cy.request with a custom Domain that is not a subdomain of the current domain would fail to set.

Additional details

Defect introduced in 3.5.0 was fixed by correct usage of 3rd party library (tough-cookie) method.

How has the user experience changed?

N/A

PR Tasks

Not sure what is the right place for the tests. I suggest cypress team itself to decide if tests for these are needed and add them if required.

• Have tests been added/updated?

NOTE: looks like 2 lines change didn't work (though it doesn't seems to me that failures are related to my change). Feel free to close this PR and work on #5656 in a separate one or update this pull request.

[cypress-bot]

Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.

• Please write [WIP] in the title of your Pull Request if your PR is not ready for review - someone will review your PR as soon as the [WIP] is removed.
• Please familiarize yourself with the PR Review Checklist and feel free to make updates on your PR based on these guidelines.

PR Review Checklist

If any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'.

User Experience

• The feature/bugfix is self-documenting from within the product.
• The change provides the end user with a way to fix their problem (no dead ends).

Functionality

• The code works and performs its intended function with the correct logic.
• Performance has been factored in (for example, the code cleans up after itself to not cause memory leaks).
• The code guards against edge cases and invalid input and has tests to cover it.

Maintainability

• The code is readable (too many nested 'if's are a bad sign).
• Names used for variables, methods, etc, clearly describe their function.
• The code is easy to understood and there are relevant comments explaining.
• New algorithms are documented in the code with link(s) to external docs (flowcharts, w3c, chrome, firefox).
• There are comments containing link(s) to the addressed issue (in tests and code).

Quality

• The change does not reimplement code.
• There's not a module from the ecosystem that should be used instead.
• There is no redundant or duplicate code.
• There are no irrelevant comments left in the code.
• Tests are testing the code’s intended functionality in the best way possible.

Internal

• The original issue has been tagged with a release in ZenHub.

[CLAassistant]

All committers have signed the CLA.

[flotwig]

Thanks for the PR. tough-cookie's documentation is kinda confusing when it comes to the order of parameters here.

The tests that are failing in server-e2e-tests-2 are relevant to cookies, it seems like these changes break sending the correct cookies when a FQDN is requested. You can check 2_cookies_spec locally to see what's going on:

cd packages/server
npm run test-e2e -- --spec 2_cookies_spec --browser electron

[jennifer-shehane]

Hey @Donotello, any updates on the requests made by @flotwig? These changes broke some of our other tests. Will you have time to look at this?

[flotwig]

I think the failures in the tests are just because we don't clear all cookies of all domains after each test (see #408 ), a cy.clearCookies({ domain: null }) should fix these up... I will look at this some more tomorrow.

[ghost]

Hey @Donotello, any updates on the requests made by @flotwig? These changes broke some of our other tests. Will you have time to look at this?

Hi, sorry I was away.

I see that tests are passing now. Are we ready to merge?