Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deleted cookies are not removed but shown with value=deleted #25205

Closed
IdoAdler2 opened this issue Dec 18, 2022 · 19 comments · Fixed by #25761
Closed

Deleted cookies are not removed but shown with value=deleted #25205

IdoAdler2 opened this issue Dec 18, 2022 · 19 comments · Fixed by #25761
Assignees
Labels
topic: cookies 🍪 type: bug type: regression A bug that didn't appear until a specific Cy version release

Comments

@IdoAdler2
Copy link

IdoAdler2 commented Dec 18, 2022

Current behavior

After upgrading from version 10.7.0 to newer versions we have problems with some cookies.
Cookies deleted by our web app, stay in the session and are given 'deleted' values.
If we disable experimentalSessionAndOrigin” and do not use cy.session everything is ok.
But we must use cy.session!!! (we love it)

See hows the deleted cookies look in the image attached.
Screen Shot 2022-12-18 at 14 29 04
do

Desired behavior

The Cookies should disappear from the browser and not stay there with a 'deleted' value.
Our app acts weird because it acts like 'deleted' is a real value for this cookies keys.

Test code to reproduce

logIn(email: string, pwd: string) {
        cy.session([email,pwd], () => {
            cy.visit(/);
            cy.get(#email’).type(email);
            cy.get(#password’).type(pwd);
            cy.get(.button._loginBtn.signSubmit’).click();
            cy.url().should(‘contain’,/root/home’)
        });
    }

describe(‘Jobs tests’, () => {
    let pageRouter: PageRouter;
    let logInPage: LogInPage;
    let generalPage: GeneralPage;
    let mainActions: MainActions;
    beforeEach(() => {
        pageRouter = new PageRouter;
        logInPage = new LogInPage;
        generalPage = new GeneralPage;
        mainActions = new MainActions;
        logInPage.loginWithAccount(7);
    });
    it(‘When creating new job from jobs it appears in the jobs report’, () => {
        cy.visit(/root/jobs’);
        let createJob = jobsPage.clickCreateNewJob();
        createJob.fillJobDetails();
        let job = createJob.submitToJob();
        cy.get(job.alias).then((jobId) => {
            let jobsPage = pageRouter.goToJobsPage()
            cy.sortTableByColumn(.rt-table .rt-th:nth-child(2), false);
            jobsPage.isJobsTableContainsJobId(jobId);
        });
    });

Screen Shot 2022-12-18 at 14 29 04

Cypress Version

12.0.0

Node version

v16.17.0

Operating System

MacOS 13.1

Debug Logs

No response

Other

No response

@emilyrohrbough
Copy link
Member

@IdoAdler2 Glad to hear you are loving sessions! This is an interesting issue though. Can you provide a small reproducible example that I can run myself? Your code is using page objects and an app that I don't have access to run myself to observe what is happening. I understand your application code is likely sensitive and cannot be shared but a "dummy" example would be totally fine.

Also can you explain at what point cookies are being deleted from the page? Is this during the session setup or during the test?

@emilyrohrbough emilyrohrbough added stage: needs information Not enough info to reproduce the issue topic: cookies 🍪 topic: session Issues when using session command labels Dec 20, 2022
@emilyrohrbough emilyrohrbough self-assigned this Dec 20, 2022
@emilyrohrbough emilyrohrbough added Needs Reproduction and removed stage: needs information Not enough info to reproduce the issue labels Dec 20, 2022
@IdoAdler2
Copy link
Author

@emilyrohrbough I sent you the invitation to demo project so can run it yourself, it's private, you need to accept the invitation. Thank you, I'll wait for your response https://github.com/IdoAdler2/cypress-example

@emilyrohrbough
Copy link
Member

@IdoAdler2 thank you for giving me access to the example to dig in. This is an interesting issue. I removed use of cy.session() in your test example to only run the setup code standalone like:

beforeEach(() => {
-  cy.session(() => {
    // your setup logic
-  })
})

it('test 1, () => {
  cy.visit(...)
  ...
})

I am seeing the same issue where it looks like the session-expired cookies are persisting with value=deleted, expiry=session like you showed in your screenshot.

I ran the same login-flow manually in my browser and it looks like these cookies are being removed like you expected so something wonky is happening with Cypress's cookie management. Since this issue is happening without the use of cy.session(), so I don't think it's an issue here, just that cy.session() is just showcasing the issue.

Looks like a bug to me. I'm going to route this to my team. Thank you!

@emilyrohrbough emilyrohrbough added type: bug and removed topic: session Issues when using session command labels Dec 22, 2022
@emilyrohrbough emilyrohrbough changed the title Cookies with deleted values are in browser session while using cy.session Deleted cookies are not removed but shown with value=deleted Dec 22, 2022
@IdoAdler2
Copy link
Author

@emilyrohrbough Just it's important to add. You still see the issue when you are not using cy.session. Because you didn't added to the configuration:
experimentalSessionAndOrigin: False.

@darrinmn9
Copy link

I am experiencing the same issue when upgrading to v12 and my codebase does not use cy.session or any experimental flags. I did notice an interesting pattern, as this only caused test failures in 6 of my ~35 tests. All the failing tests had logic to "logout" one user, and then "login" a different user - which results in my server destroying the first session upon logout.

Oddly though, the server is written in php, and apparently from their documentation:

Cookies must be deleted with the same parameters as they were set with. If the value argument is an empty string, and all other arguments match a previous call to setcookie, then the cookie with the specified name will be deleted from the remote client. This is internally achieved by setting value to 'deleted' and expiration time in the past.

(ive also read a few php tracker issues where users are complaining this is a bug since 2005, but no movement on the issues from php core) @IdoAdler2 is your webserver also in php?

From my investigation, its not cypress that is setting the cookie value = 'deleted', but the php backend. I wonder though if cypress is somehow persisting the cookie if the value is not empty / 'deleted' when it should just be deleting the expired cookie since the expires timestamp value is set in the past?

@emilyrohrbough
Copy link
Member

@IdoAdler2 I verified this behavior with Cypress 12.2.0 where this flag was removed and cy.session() is available by default. I am seeing this behavior without using cy.session().

@darrinmn9 There were some minor cookies changes in v12 which is why this is likely observed when you upgraded. I am curious about your findings though! Super insightful. Thank you for adding this! I do wonder if we are correctly cleaning up expired cookies...

@IdoAdler2
Copy link
Author

IdoAdler2 commented Dec 29, 2022

@darrinmn9 Very very interesting!!! I guess you are right. Because our legacy web server code is also written in PHP.
So something changed in the way Cypress responds to this PHP behavior.
From our tries to debug it, the problem is not caused by using Expertimal flags by purpose.
We thought the problem may occur because this flag is not an Expertimal anymore. Since version 12 it is set to true by default. But we are not 100% sure.
Screen Shot 2022-12-29 at 22 42 24

@emilyrohrbough
Copy link
Member

@IdoAdler2 what as the last version this worked correctly for you when using cy.session()? v11?

@IdoAdler2
Copy link
Author

@emilyrohrbough For us, it starts not to work on 10.0.0.

@darrinmn9
Copy link

For me, tests were passing on v11.2.0. started failing in v12

@IdoAdler2
Copy link
Author

@emilyrohrbough Do you have any updates?

@AtofStryker
Copy link
Contributor

@IdoAdler2 would you be able to invite me the git repository?

@AtofStryker AtofStryker assigned mschile and unassigned AtofStryker Jan 31, 2023
@IdoAdler2
Copy link
Author

@AtofStryker @mschile Invited both of you to the repo.

@mschile
Copy link
Contributor

mschile commented Feb 4, 2023

As a temporary workaround, you could try manually removing the deleted cookies:

// HACK: clear deleted cookies
cy.getAllCookies().then((cookies) => {
  cookies.forEach((cookie) => {
    if (cookie.value === 'deleted') {
      cookie.expiry = 1;
      cy.setCookie(cookie.name, cookie.value, cookie)
    }
  });
});

@mschile mschile added the type: regression A bug that didn't appear until a specific Cy version release label Feb 6, 2023
@AtofStryker
Copy link
Contributor

@IdoAdler2 would you be able to try this binary to see if it resolves your issue?

@ilonalink
Copy link

@AtofStryker yes, it's resolved our issue. It's working locally, when this update will be released approximately? Thank you very much for your help.

@AtofStryker
Copy link
Contributor

@ilonalink that's great to hear! My guess would be next week if we need to patch, or the following with 12.7.0

@AtofStryker
Copy link
Contributor

@ilonalink This should be making it in today or tomorrow with 12.6.1 since we need to do a patch and it was ready to go!

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Feb 25, 2023

Released in 12.7.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.7.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic: cookies 🍪 type: bug type: regression A bug that didn't appear until a specific Cy version release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants